Re: [O] replace letf with cl-letf in org-mime

[Eric Abrahamsen]

Nicolas Goaziou <address@hidden> writes:

> Hello,
>
> Eric Abrahamsen <address@hidden> 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

0001-org-mime.el-Don-t-use-letf-or-cl-letf.patch
Description: Text Data