Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Hello,
>
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> Subject: [PATCH] org-mime.el: Avoid use of letf/cl-letf
>
> Thank you. Some comments follow.
>
>> +    (let* ((mp (lambda (p)) (org-entry-get nil p org-mime-use-property-inheritance))
>
> It should be
>
>   (mp (lambda (p) (org-entry-get ....)))

Whoops, dammit, I made the same mistake in both places, but somehow only
fixed the second.

>> +  (let ((bhook
>> +	 (lambda (body fmt)
>> +	   (let ((hook (intern (concat "org-mime-pre-"
>> +				       (symbol-name fmt)
>> +				       "-hook"))))
>> +	     (if (> (eval `(length ,hook)) 0)
>> +		 (with-temp-buffer
>> +		   (insert body)
>> +		   (goto-char (point-min))
>> +		   (eval `(run-hooks ',hook))
>> +		   (buffer-string))
>> +	       body))))
>
> Not really related to the patch but the `eval' in the definition above
> looks wrong. Shouldn't it be
>
>   (> (length hook) 0)
>
> and
>
>   (run-hooks hook)

That is weird. What's even weirder is the above doesn't work. I set up a
test like this:

(defun my-org-mime-hook ()
  (message "hook!"))

(add-hook 'org-mime-pre-org-hook 'my-org-mime-hook)

If I remove the two `eval's and treat "hook" like a normal variable, the
call to `length' fails with:

Wrong type argument: sequencep, org-mime-pre-org-hook

So apparently `length' is seeing the symbol name, and not the symbol
value.

I tried changing the `let' to look like:

(let ((hook (symbol-value (intern (....

Now the value of "hook" is '(my-org-mime-hook). That works with the
`length', and also with the `run-hooks', so long "hook" is quoted as in
the original `eval' version:

(run-hooks 'hook)

Unfortunately, that means there are still some fundamental things I
don't understand about how symbols work.

Here's a fixed version of the previous patch. I suppose I could also
alter the "bhook" thing to use `symbol-value' instead of `eval', but
that doesn't seem to be a net gain.

Thanks,
Eric