the preSave Action isn't triggered when the file is saved during quit dialog

[klauweg]

Description of the problem or steps to reproduce

the preSave Action isn't triggered when the file is saved during quit dialog (answering the question with "yes").

Specifications

Version: 2.0.14-dev.73
Commit hash: 8724709
Compiled on March 17, 2024

I suppose further information about used terminal isn't necessary in this case.

[JoeKar]

Because it was the Quit-action being executed, which furthermore calls the internal save functions, but it wasn't the Save-action. Currently only direct invoked actions cause such plugin callbacks. There are only a few exceptions of it.

[klauweg]

I see.
So i suppose if i want to do something BEFORE saving (backup) i must implement the quit dialog myself.

[Andriamanitra]

Because it was the Quit-action being executed, which furthermore calls the internal save functions, but it wasn't the Save-action. Currently only direct invoked actions cause such plugin callbacks. There are only a few exceptions of it.

Is there a reason behind the Quit action not triggering Save action when it's saving files? It feels more like an oversight to me, but I'm not familiar with how the action system works internally.

[JoeKar]

Is there a reason behind the Quit action not triggering Save action when it's saving files?

I assume it was easier to implement in that way, since an additional callback is used. I assume there are more scenarios/use cases in which micro doesn't trigger the related action, but the internal function behind of it, in case the "parent" action was called. Furthermore autosave shouldn't trigger the plugin callback too.
There is still a lot room for improvements of micro. 😉

It's possible...

diff --git a/internal/action/actions.go b/internal/action/actions.go
index c1a96e33..72af9c3a 100644
--- a/internal/action/actions.go
+++ b/internal/action/actions.go
@@ -1885,17 +1885,23 @@ func (h *BufPane) Quit() bool {
        if h.Buf.Modified() {
                if config.GlobalSettings["autosave"].(float64) > 0 {
                        // autosave on means we automatically save when quitting
-                       h.SaveCB("Quit", func() {
-                               h.ForceQuit()
+                       action := BufKeyAction(func(h *BufPane) bool {
+                               return h.Save()
                        })
+                       if h.execAction(action, "Save", nil) {
+                               h.ForceQuit()
+                       }
                } else {
                        InfoBar.YNPrompt("Save changes to "+h.Buf.GetName()+" before closing? (y,n,esc)", func(yes, canceled bool) {
                                if !canceled && !yes {
                                        h.ForceQuit()
                                } else if !canceled && yes {
-                                       h.SaveCB("Quit", func() {
-                                               h.ForceQuit()
+                                       action := BufKeyAction(func(h *BufPane) bool {
+                                               return h.Save()
                                        })
+                                       if h.execAction(action, "Save", nil) {
+                                               h.ForceQuit()
+                                       }
                                }
                        })
                }

...but doesn't look that nice so far, since it may end in this...

preQuit
onQuit
preSave
onSave

....since the InfoBar.YNPrompt() is performed async.

[klauweg]

Furthermore autosave shouldn't trigger the plugin callback too.

The save command does not trigger the preSave/onSave Action either.

reading your comment i suppose this is intentional too.
It's ok but good to know.

[dmaluka]

Micro clearly defines what is an "action". It is something that is bound to a key, i.e. triggered when this key is pressed. There are over 100 actions, they all are listed in help keybindings. Save is just one of them. Micro doesn't document that onSave is triggered whenever a file is saved. It only documents that onAction, where Action is any of those 100+ actions including Save, is triggered when the user presses a key combination bound to this action.

Which doesn't mean that a callback that is triggered whenever micro saves a file for whatever reason, not just when pressing Ctrl-S, wouldn't be useful. But it would probably need to be named differently, e.g. onBufferSave (in line with the existing onBufferOpen), so that it is clear that it is a "core" callback like onBufferOpen, not an action callback, to keep the behavior of action callbacks predictable.

[Andriamanitra]

Micro clearly defines what is an "action". It is something that is bound to a key, i.e. triggered when this key is pressed. There are over 100 actions, they all are listed in help keybindings. Save is just one of them. Micro doesn't document that onSave is triggered whenever a file is saved. It only documents that onAction, where Action is any of those 100+ actions including Save, is triggered when the user presses a key combination bound to this action.

Which doesn't mean that a callback that is triggered whenever micro saves a file for whatever reason, not just when pressing Ctrl-S, wouldn't be useful. But it would probably need to be named differently, e.g. onBufferSave (in line with the existing onBufferOpen), so that it is clear that it is a "core" callback like onBufferOpen, not an action callback, to keep the behavior of action callbacks predictable.

Thanks for the explanation, that clarifies it a lot. I disagree on the "clearly defines" part though – the documentation in plugins.md says:

onAction(bufpane): runs when Action is triggered by the user, where
Action is a bindable action (see > help keybindings). A bufpane
is passed as input and the function should return a boolean defining
whether the view should be relocated after this action is performed.

The save in the quit dialog is clearly triggered by the user by answering the prompt. It's not clear in the documentation that an action can only be triggered directly through a keybinding that they have explicitly set or that is part of the documented defaultkeys. One could reasonably assume (as I did) that in the prompt pressing "y" is bound to Save,Quit.

From a plugin perspective this system is a bit silly – plugins tend to care about what actions are happening in the editor and not so much about what triggered the action. This means that in practice using onSave is almost always a bug. If a plugin wanted to only handle a keybinding, it could already do that directly through the keybinding system.

Adding onBufferSave would be a satisfactory solution to the problem. onSave would remain a source of confusion but plugin writers are a niche enough audience that we can be taught to avoid it.

[dmaluka]

I said "clearly defines", not "clearly documents".

The save in the quit dialog is clearly triggered by the user by answering the prompt. It's not clear in the documentation that an action can only be triggered directly through a keybinding that they have explicitly set or that is part of the documented defaultkeys.

We can trivially fix it, right?

--- a/runtime/help/plugins.md
+++ b/runtime/help/plugins.md
@@ -67,8 +67,8 @@ that micro defines:
 
 * `onSetActive(bufpane)`: runs when changing the currently active bufpane.
 
-* `onAction(bufpane)`: runs when `Action` is triggered by the user, where
-   `Action` is a bindable action (see `> help keybindings`). A bufpane
+* `onAction(bufpane)`: runs when `Action` is triggered by the user, when
+   pressing a key bound to this action (see `> help keybindings`). A bufpane
    is passed as input and the function should return a boolean defining
    whether the view should be relocated after this action is performed.
 

If we take the existing description literally, it wouldn't make much sense, since it would mean that onSave it triggered by Ctrl-S, Ctrl-Q or the save command, but for some reason is not triggered by autosave, by a plugin that e.g. directly calls buf:Save() and so on.

One could reasonably assume (as I did) that in the prompt pressing "y" is bound to Save,Quit.

Doesn't seem like a reasonable assumption to me. A key being "bound" to something means that the user can check what it is bound to with the showkey command, and can rebind it to something else. So "y" in the quit prompt (or in any other yes/no prompt) is not "bound" to anything. The documentation doesn't say that it is, and it isn't. "y" or "n" in a prompt just directly triggers micro's internal logic, not "actions" exposed to the user. (And it triggers that internal logic as a part of performing the Quit action triggered by the user, for that matter.)

From a plugin perspective this system is a bit silly – plugins tend to care about what actions are happening in the editor and not so much about what triggered the action.

Yep, basically this system is a quick smart hack: since we need to define a bunch of "actions" to let the users configure their keybindings anyway, why not also, as a bonus, allow the user to hook custom Lua logic on all those same actions.

IOW, perhaps the primary target audience of these "action callbacks" is not plugin writers but normal users who just hack on their init.lua. I've just checked how many "action callbacks" I happen to have in my messy init.lua. I don't have many: I have onInsertNewline, preFindNext and preFindPrevious. I could achieve the same result by binding the keys to custom Lua functions instead, but why bother and mess with my keybindings if micro already allows me to just add callbacks for the existing default actions instead. (Or in case of onInsertLine, I could probably use onRune instead, but again, why bother.)

IOW, if such "action callbacks" are not very useful or reliable for use by plugins, it just means that plugin authors should rather ignore their existence and instead aim at using, let's call it, "event callbacks", that are triggered by some well-defined events irregardless of keybindings. Micro has a few such callbacks (onBufferOpen, onBufPaneOpen, onRune, onBeforeTextEvent, onSetActive, ...), we could add more. Vim has a plenty: https://vimhelp.org/autocmd.txt.html#autocmd-events

onSave would remain a source of confusion but plugin writers

The documentation doesn't even mention onSave, so what's the confusion? ...Hmm, I stand corrected, just found that onSave luckily happens to be the one of 100+ action callbacks that is explicitly mentioned in the documentation, as an example. Well, we could fix that description as well...

[dmaluka]

BTW also thanks for bringing my attention to this:

onAction(bufpane): [...] and the function should return a boolean defining
whether the view should be relocated after this action is performed.

What the heck is that? Micro doesn't implement anything like that. It uses the return value of onAction for a completely different thing.

[JoeKar]

IOW, if such "action callbacks" are not very useful or reliable for use by plugins, it just means that plugin authors should rather ignore their existence and instead aim at using, let's call it, "event callbacks", that are triggered by some well-defined events irregardless of keybindings. Micro has a few such callbacks (onBufferOpen, onBufPaneOpen, onRune, onBeforeTextEvent, onSetActive, ...), we could add more.

Ok, that's the other way around. We leave the "action callbacks" (execAction()) being related to the key bindings only and don't support nesting of such "action callbacks" as it seems to be intended.
Extending the "event callbacks" resp. plugin hooks means to spread more of these RunPluginFn() over micro. Hopefully we don't loose the control over them.

And I definitely agree to rework the documentation.