Added ModePush and ModePop hooks

[laelath]

Implemented ModePush and ModePop as proposed in #2513.

[laelath]

I'm not sure if the behavior in InputHandler::reset_normal_mode is desired, as it could trigger a bunch of hooks, but when using the push/pop syntax it seemed weird to just invisibly remove the stack.

Edit: Github won't let me post a comment on a code location.

[mawww]

Yeah, I am wondering about that as well, and it seems safer to do it so that hooks can rely on PushMode always being matched by a PopMode.

On the other hand, we do not have a lot of tools to manage nested states: say you have a [..., normal, insert, normal] stack to pop, for the two normal modes that are going to be popped, if we want to do something that requires to run on two normal pops instead of one, we would need to somehow distinguish those two pops, which would likely require some kind of stack accessible from the hook to get some state associated with those pops. It seems to me that without those tools, we cannot easily do anything interesting with two pops that we could not do with a single one.

[Screwtapello]

I'm trying to think of a way to emulate a stack by means of one-shot PopMode hooks, but if triggering a PopMode hook triggers all the PopMode hooks, instead of just the ones added in the most recent PushMode, then yeah, not much use.

On the other hand, if there were a PopThisMode hook that fired when a specific mode was popped from the stack (instead of when any mode of a given type was popped), that'd do it.

At the end of the day, though, I think it's just less surprising for everybody (both scripters, and people working on the Kakoune code-base) if every PushMode is always matched by a PopMode.

[laelath]

This might be over complicating it, but we could add a mode stack level to the filtering regex that could be used to identify when the mode being popped matches the add. This could also allow special behavior on modes popping back to that level. This would only work if we strictly enforced that every ModePush was matched by a ModePop.

[mawww]

@laelath That could be an idea, the ModePush/ModePop filter string could be "<prev_mode>:<new_mode>:<stack_level>". Then hook global ModePush \w+:insert:(\d+) %{ hook -once global ModePop "insert:\w+:%val{hook_param_capture_1}" 'echo matching pop' } could do the trick.

[mawww]

I think we should think a bit about the mode in which the hook is run.

In the proposed implementation, ModePush runs in the pushed mode, while ModePop runs in the parent mode of the popped mode (If I am in mode A, and push then pop mode B, I get a ModePush A:B that runs in mode B, and a ModePop B:A that runs in ModeA).

I am not sure this is what we want, because it breaks symmetry, we cannot undo something we did on ModePush in ModePop, because we are in another mode already.

I think running ModePush in the pushed mode context is correct, and hence I think ModePop should run in the pushed mode just before it gets disabled.

This leads me to another question: the filtering string format. Do we want to have the parent mode there ? I am not sure there is much value in knowing that information, It would be nice to have an explicit use case for that. Actually it probably would be nice to have a real use case for the stack level as well. Maybe just putting the pushed/popped mode name is good enough ?

Opinions ?

[Screwtapello]

I think we should think a bit about the mode in which the hook is run.

I talked about this in #2513 (comment). Long story short, it's safer to always run the hook in the "after" mode.

This leads me to another question: the filtering string format. Do we want to have the parent mode there?

I'm not actually sure what the Kakoune mode state diagram looks like. If it's a star with "normal" in the centre, so you always have to go via "normal" to get from one non-normal mode to another... then yeah, there's not much point in knowing the parent mode. On the other hand, if there's multiple ways to get from one mode to another, it might be important to know which route the user took.

Actually it probably would be nice to have a real use case for the stack level as well.

It might be useful for a snippet plugin, where inserting a snippet begins a new insert mode and adds a bunch of hooks and mappings to jump between the snippet's fields... hooks and mappings that need to be removed when that specific insert-mode is popped, not just when any old insert-mode is popped.

[mawww]

Sorry, I had forgotten about that PR, revisited that and in the end went with a push/pop prefix for the ModeChange hook as ModePush/ModePop would have required to duplicate hooks for simply running commands when a mode gets enabled (regardless on how we got to it).