bug#35351: 27.0.50; Enable derived modes to run their own very-early 'change-major-mode-hook' code

bug#35351: 27.0.50; Enable derived modes to run their own very-early 'change-major-mode-hook' code

[Phil Sainty]

The library I'm working on (so-long.el) defines a major mode
which needs to remember various buffer-local values as they were
in the original mode, *before* my major mode takes effect.

I'm currently using `change-major-mode-hook' for this, but it has
occurred to me that it would be nicer if this hook code of mine
only ever ran in the case where it is useful (i.e. the major mode
being changed to is in fact my mode).  `change-major-mode-hook'
has no knowledge of the mode which has just been invoked, so it
must necessarily run for *every* mode change -- which isn't
relevant to my library in the vast majority of cases.

I think `change-major-mode-hook' would more commonly be used by
modes to handle any subsequent 'unloading' needs of that same
mode in case it gets replaced later on (i.e. the mode body could
set a buffer-local hook value), so my scenario of the new mode
wanting to know things about the previous mode is doubtless a bit
of a niche case; but I thought I'd raise it for discussion.


`define-derived-mode' creates the mode function like so:

    (defun ,child ()
      ,docstring
      (interactive)
      ; Run the parent.
      (delay-mode-hooks
        (,(or parent 'kill-all-local-variables))

Where PARENT must likewise `kill-all-local-variables' (which runs
`change-major-mode-hook').

What would be nice is a way for the mode definition to provide
code which would be evaluated before that (delay-mode-hooks...)
form, and consequently acted like a `change-major-mode-hook'
which only ever happened if this mode was called.

(A vaguely analogous facility currently in `define-derived-mode'
is the :after-hook keyword, for running code very *late* in the
proceedings.)


I could always define my mode without using the macro, and then
do whatever I wanted before calling `kill-all-local-variables';
but I *want* to have the benefits of using the macro, so I don't
want to resort to that; I just want the equivalent ability,
while still using the macro.

A hack which works in my case (because my mode was not already
derived from another) is:

    (defun my-mode-parent ()
      (do-my-custom-thing)
      (kill-all-local-variables))

    (define-derived-mode my-mode my-mode-parent ...)

This feels a little ugly, and `derived-mode-make-docstring' will
point out the existence of the parent, so it's not ideal; but I'm
still tempted to use this approach, and simply document it
sufficiently such that the docstring reference won't cause undue
confusion for those who follow it.


That hack obviously can't be used in cases where a parent is
already defined, but my suggested new feature could still work in
those cases.  Something like:

    (defun ,child ()
      ,docstring
      (interactive)
      ,@before-hook
      ; Run the parent.
      (delay-mode-hooks
        (,(or parent 'kill-all-local-variables))

If the new child mode is used directly then things pan out almost
identically to the hack version.

More generally, we end up with:

    ,@child-before-hook
    (delay-mode-hooks
      ,@parent-before-hook
      (delay-mode-hooks
        ,@grandparent-before-hook
        (delay-mode-hooks
          (kill-all-local-variables) ;; runs change-major-mode-hook
          ,@grandparent-body)
        (run-mode-hooks 'grandparent-mode-hook)
        ,@parent-body)
      (run-mode-hooks 'parent-mode-hook)
      ,@child-body)
    (run-mode-hooks 'child-mode-hook)

Which creates the following sequence of events:

    ,@child-before-hook
    ,@parent-before-hook
    ,@grandparent-before-hook
    (kill-all-local-variables) ;; runs change-major-mode-hook
    ,@grandparent-body
    ,@parent-body
    ,@child-body
    (run-hooks 'change-major-mode-after-body-hook)
    (run-hooks 'grandparent-mode-hook)
    (run-hooks 'parent-mode-hook)
    (run-hooks 'child-mode-hook)
    (run-hooks 'after-change-major-mode-hook)

Obviously that sequence of 'before-hook' instances is in the
reverse sequence to the 'body' and 'mode-hook' sequences.  That's
possibly a desirable thing, but I'm not absolutely certain.

A simply way of reversing that sequence would be for each
:before-hook to be added as a buffer-local change-major-mode-hook
entry -- (add-hook 'change-major-mode-hook FOO nil :local) --
rather than evaluating them immediately, which would then build
up a buffer-local hook sequence like:

'(grandparent-before-hook parent-before-hook child-before-hook)

And then `kill-all-local-variables' would run them in that order
(and still ahead of any other pre-existing buffer-local values
for `change-major-mode-hook').


What do people think?


-Phil

bug#35351: 27.0.50; Enable derived modes to run their own very-early 'change-major-mode-hook' code

[Phil Sainty]

I forgot to mention the other hack solution which occurred to me,
which is for the library which defines the major mode to add its
own before-advice for that major mode function.

This also seems a bit ugly to me -- but maybe not quite as ugly
as using a 'fake parent' mode.  It's a slightly more flexible
solution, at any rate.


-Phil

bug#35351: 27.0.50; Enable derived modes to run their own very-early 'change-major-mode-hook' code

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > I forgot to mention the other hack solution which occurred to me,
  > which is for the library which defines the major mode to add its
  > own before-advice for that major mode function.

We have adopted the design principle that advice is for users to add,
and nothing else.

This is because developers investigating why a call to the function
foo doesn't work as expected will not think of the possibility that
foo has advice.

Thus, our design rule is that, if you want some code to be run before
or after calls to foo, add code to foo to run a hook.  This way, the
code for foo will show developers that a hook is called.
-- 
Dr Richard Stallman
President, Free Software Foundation (https://gnu.org, https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

bug#35351: 27.0.50; Enable derived modes to run their own very-early 'change-major-mode-hook' code

[Stefan Monnier]

> I'm currently using `change-major-mode-hook' for this, but it has
> occurred to me that it would be nicer if this hook code of mine
> only ever ran in the case where it is useful (i.e. the major mode
> being changed to is in fact my mode).

There's no need for a "hook" for that.  You just need to put the code to
save the old values before calling kill-all-local-variables.

> (A vaguely analogous facility currently in `define-derived-mode'
> is the :after-hook keyword, for running code very *late* in the
> proceedings.)

Indeed what you're asking for is like a :before-hook symmetric (FWIW,
I think the "-hook" part of the name was probably not the best choice).

Usually this is done with something akin to:

    (define-derived-mode my-actual-mode
      ...)

    (defun my-mode ()
      (my-save-local-vars)
      (my-actual-mode))

or something similar (or not using define-derived-mode).
Of course, this comes with its own drawbacks.


        Stefan

bug#35351: 27.0.50; Enable derived modes to run their own very-early 'change-major-mode-hook' code

[Phil Sainty]

On 22/04/19 8:54 AM, Stefan Monnier wrote:
> Usually this is done with something akin to:
>     (define-derived-mode my-actual-mode
>       ...)
>     (defun my-mode ()
>       (my-save-local-vars)
>       (my-actual-mode))
>
> or something similar (or not using define-derived-mode).
> Of course, this comes with its own drawbacks.

Yes, I think I like that least of all -- having the macro set up
everything based on a different name to the mode that people are
expected to be using just seems wrong to me.

I'd say the fact that there's a known "usual" hack for this
suggests that there's good reason to implement it directly in the
macro.


> Indeed what you're asking for is like a :before-hook symmetric

I've pushed an implementation to branch origin/fix/bug-35351 as a
starter.

https://git.savannah.gnu.org/cgit/emacs.git/log?h=fix/bug-35351

Previous Emacs versions will ignore the unrecognised keyword and
its value; so libraries using the new keyword and still wanting
to support older versions of Emacs can safely use the new keyword
and *conditionally* add advice when the Emacs version is < 27, to
run the same code before the mode function and achieve the same
effect.


> (FWIW, I think the "-hook" part of the name was probably not
> the best choice).

I've used the name :before-hook to match the existing
:after-hook, but we can certainly change it to something else.

(We could potentially deprecate the name :after-hook as well, and
add an alias for that with a better name at the same time.)

I'm now unsure whether :after-hook was intended to be interpreted
as "this is a bit like a *hook* which runs *after* everything
else has happened"; or if it meant "do this thing *after* the
mode *hook*" (or indeed after after-change-major-mode-hook).

The name ":before-hook" only really matches the former
interpretation.  ("before the mode hook" would be more
change-major-mode-after-body-hook than change-major-mode-hook).

Would :eval-before and :eval-after be good?


It might also be nice if symbol values for these keywords were
interpreted as the name of a function to call.  I don't think that
change would be *expected* to break anything, as a symbol value
would in effect be a no-op for the current :after-hook, and so
I wouldn't expect any existing modes to be using a symbol value.


In the initial commit I've used an approach which will run the
parent's :before-hook before the child's :before-hook, using the
change-major-mode-hook technique I mentioned at the end of my
original message, although I'm still uncertain about which order
is *best*.

Possibly it should be child-before-parent on the basis that the
author then has more influence over the order in which things
happen?  i.e.: If we implement it like this:

    ,child-before-hook
    (delay-mode-hooks
      (,(or parent 'kill-all-local-variables))

Which gives us the sequence:

    child-before-hook
    parent-before-hook
    change-major-mode-hook

Then child-before-hook could, if it wanted, do this:

    (add-hook 'change-major-mode-hook FOO nil :local)

And then FOO would run *after* the parent-before-hook,
effectively reversing that sequence.

(Whereas the code I've pushed initially is already using that
change-major-mode-hook approach, so there's no scope to
manipulate the order.)

Child-before-parent also means :before-hook *really is* the very
first thing that happens in the child mode, just as :after-hook
is the very last; so perhaps that's still a good/intuitive way
around, even if it's the reverse of the parent->child order used
in all the other phases of the mode's execution.

Child-before-parent would also match the order when 'before'
advice was being used to mimic the :before-hook behaviour in
older versions of Emacs.

I'm not sure I have a strong opinion on the matter, and the cases
in which it would even matter would be limited, but I think I've
now argued myself around to being slightly in favour of child-
before-parent; so if no one else has strong feelings about it,
I'll most likely change it to that.


-Phil

bug#35351: 27.0.50; Enable derived modes to run their own very-early 'change-major-mode-hook' code

[Stefan Monnier]

> Yes, I think I like that least of all -- having the macro set up
> everything based on a different name to the mode that people are
> expected to be using just seems wrong to me.

I tend to view this as the fact that those "modes" should distinguish
between the mode and the command that invokes it, because that command
does more than setup the mode (e.g. it arranges to be able to go back
to the earlier modes).

E.g. in your case the mode could be called `so-long-mode` and the
command to enter it could be `so-long`.

> I'm now unsure whether :after-hook was intended to be interpreted
> as "this is a bit like a *hook* which runs *after* everything
> else has happened"; or if it meant "do this thing *after* the
> mode *hook*" (or indeed after after-change-major-mode-hook).

Oh, you're absolutely right, it's called ":after-hook" because it runs
after the mode-hook.

> In the initial commit I've used an approach which will run the
> parent's :before-hook

That seems backward to me.

> Possibly it should be child-before-parent on the basis that the
> author then has more influence over the order in which things
> happen?

Exactly.


        Stefan

bug#35351: 27.0.50; Enable derived modes to run their own very-early 'change-major-mode-hook' code

[Stefan Monnier]

> I've pushed an implementation to branch origin/fix/bug-35351 as a
> starter.
>
> https://git.savannah.gnu.org/cgit/emacs.git/log?h=fix/bug-35351

BTW, could you try and look around emacs/lisp/.../*.el to see which
other existing major modes could benefit from such a thing (and maybe
also if they almost could but in the end can't for some reason).


        Stefan

bug#35351: 27.0.50; Enable derived modes to run their own very-early 'change-major-mode-hook' code

[Phil Sainty]

On 23/04/19 2:39 AM, Stefan Monnier wrote:
>> I'm now unsure whether :after-hook was intended to be interpreted
>> as "this is a bit like a *hook* which runs *after* everything
>> else has happened"; or if it meant "do this thing *after* the
>> mode *hook*" (or indeed after after-change-major-mode-hook).
> 
> Oh, you're absolutely right, it's called ":after-hook" because it
> runs after the mode-hook.

Which means :before-hook definitely isn't a good name for the new
keyword.

Should I go with :eval-before ?

And should I add :eval-after as an alias for :after-hook, for naming
symmetry?  (Even if :after-hook makes better sense than you initially
thought, it's maybe still not the best name for it.)


>> In the initial commit I've used an approach which will run the
>> parent's :before-hook
> 
> That seems backward to me.
> 
>> Possibly it should be child-before-parent on the basis that the
>> author then has more influence over the order in which things
>> happen?
> 
> Exactly.

Cool.  I'm switching that around.


-Phil

bug#35351: 27.0.50; Enable derived modes to run their own very-early 'change-major-mode-hook' code

[Phil Sainty]

On 23/04/19 11:18 AM, Phil Sainty wrote:
> On 23/04/19 2:39 AM, Stefan Monnier wrote:
>> Oh, you're absolutely right, it's called ":after-hook" because it
>> runs after the mode-hook.
> 
> Which means :before-hook definitely isn't a good name for the new
> keyword.
> 
> Should I go with :eval-before ?

Or maybe this pairing would be reasonable:

:before-change
:after-hook

Other suggestions welcomed.


-Phil

bug#35351: 27.0.50; Enable derived modes to run their own very-early 'change-major-mode-hook' code

[Stefan Monnier]

>> Oh, you're absolutely right, it's called ":after-hook" because it
>> runs after the mode-hook.
> Which means :before-hook definitely isn't a good name for the new
> keyword.

It's run before the before-change-mode-hook, so the name isn't
completely wrong (and the description of :before-hook makes this
connection clear).

> Should I go with :eval-before ?

I'd prefer we avoid "eval" since that would suggest to quote the form.
IOW, I think `:before-hook` is fine.


        Stefan

bug#35351: 27.0.50; Enable derived modes to run their own very-early 'change-major-mode-hook' code

[Phil Sainty]

On 23/04/19 2:08 PM, Stefan Monnier wrote:
>>> Oh, you're absolutely right, it's called ":after-hook" because it
>>> runs after the mode-hook.
>> Which means :before-hook definitely isn't a good name for the new
>> keyword.
> 
> It's run before the before-change-mode-hook, so the name isn't
> completely wrong (and the description of :before-hook makes this
> connection clear).
> 
>> Should I go with :eval-before ?
> 
> I'd prefer we avoid "eval" since that would suggest to quote the form.
> IOW, I think `:before-hook` is fine.

Point taken re: eval (although at minimum we have `with-eval-after-load'
which does not require quoting), but I'm also not terribly happy with
"before-hook", as I now feel it's quite ambiguous.

Running a derived mode involves all of the following hooks:

* change-major-mode-hook
* change-major-mode-after-body-hook
* *-mode-hook(s)
* after-change-major-mode-hook
* delayed-after-hook-functions

So "before hook" (and "after hook") are pretty vague.  Especially when
the "hook" in question is different in each case.

The documentation would indeed clarify, but I think it's sensible to
choose a name which gives a clearer idea from the outset.

My other suggestion of :before-change still seems reasonable to me.


-Phil

bug#35351: 27.0.50; Enable derived modes to run their own very-early 'change-major-mode-hook' code

[Stefan Monnier]

> which does not require quoting), but I'm also not terribly happy with
> "before-hook", as I now feel it's quite ambiguous.

While "before-hook" may not be perfect, it has the very definitive
advantage of matching the existing "after-hook", so we should go
with that.

If/when we find a better naming scheme for those two, we can consider
renaming them, but that itself comes with non-negligible disadvantages,
so we're probably better off living with the existing "suboptimal" names.

More important would be to make sure that the new semantics covers most
of the existing needs.


        Stefan