bug#12259: Add delete-trailing-whitespace to list of safe eval forms

[Mathieu Boespflug]

>> I have attached a patch at the end of this email that considers eval
>> forms that add 'delete-trailing-whitespace to various hooks safe by
>> default.
>
> Actually, I wonder whether we want to accept/encourage those uses
> instead of (add-hook 'before-save-hook 'delete-trailing-whitespace).

The problem with the method above is that before-save-hook isn't made
a buffer-local variable by hack-local-variables. Therefore, using
(add-hook 'before-save-hook 'delete-trailing-whitespace) causes
delete-trailing-whitespace to be run even for buffers that are not in
the directory hierarchy of .dir-locals.el.

This is undesirable because .dir-locals.el is often used by free
software projects to enforce a common set of guidelines and style for
editing code. Any changes to hooks should therefore ideally be directory
local, so as to apply only to those files that are part of some
particular free software repository.

> IOW I think we should only add the before-save-hook version but not
> the others (and I guess the same holds for time-stamp, tho we'll
> probably keep the other ones for time-stamp for backward-compatibility
> reasons).

(See above.)

>> But ideally this patch would be superseded by adding a mechanism that
>> allows .dir-locals.el to add predefined functions to hooks (at least
>> buffer local ones) without having to use eval.
>
> Why?

Because using eval for the purposes of adding new functions to hooks
feels overkill, and causes several problems. The
affecting-hooks-that-are-not-buffer-local problem is one of them.
Another problem is that there are many equivalent ways of modifying
a hook (using add-hook, using setq, etc), so adding new entries to
safe-local-eval-forms would never catch them all.

>> That way we wouldn't have to write patches such as this one for every
>> new sensible stock function that people want to have executed on
>> file saves.
>
> You don't have to write patches like this one.  You can just customize
> safe-local-eval-forms.  There is a problem, indeed, tho: if you
> customize this var and we later add things to it, you'll keep using your
> customized version and won't benefit from the expanded list.
> So we should keep the default value of safe-local-eval-forms as nil, and
> allow things like those add-hook some other way (e.g. a new var).

... and that's the third problem caused by using eval to set hooks.

Besides, customizing safe-local-eval-forms isn't a great solution in the
scenario discussed above: the whole point for a free software project to
have a .dir-locals.el at the root of the repo is so that none of the
(potentially hundreds of) developers of that project need to fiddle with
customize manually.

-- Mathieu