Allow callbacks via node-options.

[L3MON4D3]

Currently the event-callbacks are only settable by providing the jump-index to the parent-node, which is pretty roundabout, and annoying for usage in eg. fmt.

s("qwer", {t'  ', i(1, "asdf", {node_callbacks = {[events.enter] = function()
	print("enter!!!")
end}})})

Calling it node_callbacks for now, since the option also applies to snippetNode, where the key callbacks is already taken :/

Also, as of now the per-node-callbacks disable/override the parent-callbacks, maybe both should be executed?

[L3MON4D3]

@weilbith You were interested in events on textNode/all kinds of node (see #195), would you motivate that again here so not all of the context of #195 is necessary? :D

[weilbith]

I'll give it a try. 😄

The current API decouples callback functions from their associated nodes. Nodes are references by some indexing mechanism. This is "hard" to get right, is prone error wen changing the snippet and is harder to read because the relation is not obvious. Being able to define callbacks directly at the node declaration solves all these issues. I think it is also fair to say that this tends to be an approach that more and more "frameworks" follow. So it seem to be a good idea.

The second part is about the ability to define callbacks for any kind of node, also for non interactive ones. The stupid counter question is: why should there be an exception? It seems like there are valid use-cases to trigger callback functionality also for non-interactive nodes.
One practical example comes from the linked ticket which discusses an approach to apply code actions from a language server on snippet nodes to do things like auto-importing a new symbol. Such functionality needs to know where in the buffer this text (node) is for the language server request. And also the correct timing after/before interactive nodes.
The current workaround to do this somewhat similar is by using a snippet "global" callback and find these nodes by specifically assigned keys. Having callbacks associated directly with any node resolves this issue very elegantly with a simple API and trivial usage with direct node access in the callback function. It also addresses the feature of triggering the callbacks at the "correct time".

---

@L3MON4D3 I must admit I'm unable to tell from the code. But in which order by which "event" do these node callbacks trigger now? 🤔
Taking my very specific example order as I described in this comment at the very bottom, would this work?

Example that works for this PR:

snippet(
  'foo',
  format(
    '{foo} - {bar} - {baz}',
    {
      foo = text_node('foo', {
        node_callbacks = { [events.enter] = function() print(1) end }
      }),
      bar = insert_node(1, 'bar', {
        node_callbacks = { [events.leave] = function() print(2) end }
      }),
      baz = text_node('baz', {
        node_callbacks = { [events.enter] = function() print(3) end }
      }),
    }
  )
)

The (for me) expected process would be:

• snippet expands
• foo text node its enter callback triggers and prints 1
• user gets put into the bar insert node
• user jumps forward
• bar insert node its leave callback triggers and prints 2
• baz text node its enter callback triggers and prints 3

[L3MON4D3]

ty!!

The current API decouples callback functions from their associated nodes. Nodes are references by some indexing mechanism. This is "hard" to get right, is prone error wen changing the snippet and is harder to read because the relation is not obvious. Being able to define callbacks directly at the node declaration solves all these issues. I think it is also fair to say that this tends to be an approach that more and more "frameworks" follow. So it seem to be a good idea

Well put, I'd add that conceptually, one should be able to decouple the content of a snippet, the nodes, from the expansion-parameters, which are the other parameters passed to s(...).
If callbacks have to be defined in the snippet-call, and not with the nodes, that distinction is weakened.

Regarding the textNode-callbacks: I have not yet implemented them😅
If I understand you correctly, they should trigger if they are "jumped over", so in {i(1), t"foo", i(2)}, the enter/leave-callbacks of foo should be executed when the jump from i1 to i2 is performed? That might work, but I think that's hard to define generally, for example if insertNodes are not jumped through in textual order ({i(2), t"foo", i(1)}).
I think in general having to get nodes via key is "fine", (though a bit better if it's avoidable).

Another idea for callbacks: you could make use of the existing infrastructure for functionNode, and just not return text, but do something else, eg. lsp-actions. That does not alleviate the issue with having to manually retrieve keyed nodes (right now, the only difficulty with exposing them to the callback is backward-compatibility) and you can get multiple nodes in the same callback, which is an inherent limitations of the event-callbacks.

[weilbith]

one should be able to decouple the content of a snippet,

That is a really good point too! I used to do this and for some snippets I still do, also for re-usability sometimes. 👍

That might work, but I think that's hard to define generally, for example if insertNodes are not jumped through in textual order ({i(2), t"foo", i(1)}).

Yeah, that's what I was afraid of. So it can only work for nodes that have an explicit index. And we can't simply add such to text nodes or similar because that would be a huge breaking change to the API.
I still like the node callback improvement/feature. Though I currently have no use case for them. All my callbacks currently act on snippet level. But maybe one day I'll be happy to use it.

Another idea for callbacks: you could make use of the existing infrastructure for functionNode,

Thanks for the inspiration. I'm just not sure how this can work. But maybe you teach me.
First issue is the buffer location. I need to know were this piece of text is. Sticking to the original example of the related issue, if I have this piece of code (shortened into a single line) useEffect(() => /* ... */, []), I need to know where useEffect is to get the import action for it. Similar with the second function parameter, the empty array. This gets the dependencies of the effect depending on the callback function I wrote as first parameter of the effect. I currently don't know how to get this buffer location information from within the function node itself. Furthermore, the function node would then return for example just useEffect? But I need this text already in the buffer before requesting the language server.
Second issue I have is the order. In the above snippet I can trigger the first action as soon as the useEffect call is in my buffer (ignoring the syncing delay to the server discussed in the issue). But the second action for the dependencies can only be triggered after I finished writing the callback function within the insert node. For this simple snippet this works because I can use the enter and leave event of the snippet ([-1]). And it might be very unlikely that I wanna trigger like three actions with two inserts in-between or similar.

I had the idea to use an insert node for this. That would give me a proper index and with the new node snippets, that would be great. The intention was to then just programatically jump to the next node within the callback of the node itself. But that doesn't work according to my experiments. Would be not really a "clean" way in terms of how the API is designed, but would do exactly what is needed. To some degree even more simple because the cursor would be at the right location and I can heavily simplify the code action request logic as I can use the native vim.lsp.buf.code_action function which works on the current cursor location.
But as I said, I tried to call require('luasnip').jump(1) within a callback function and it did nothing somehow. 🤷🏾
Thinking about it even more I like the idea really much. I simply hate how much logic I had to write to do the stupid code action request stuff. And it gets even worse because the code in the snippet does not work for all cases. And I have to "maintain" it across NeoVim core runtime changes. So maybe I'll look a bit more how I can trick LuaSnip here 😉

[L3MON4D3]

I currently don't know how to get this buffer location information from within the function node itself.

Yeah, you'd still have to resort to get_keyed_node for that, but we would be able to modify the behaviour of functionNode such that the passed callback is called with the nodes themselves, and not with their content.

Furthermore, the function node would then return for example just useEffect?

Oh, no, I'd suggest to just use the functionNode as a callback, so it would not insert any text

s("foo", {i(1, {key=1}), i(2, {key=2}), t"bar", f(function(args) ... return "" end, {1,2})})

Second issue I have is the order. In the above snippet I can trigger the first action as soon as the useEffect call is in my buffer (ignoring the syncing delay to the server discussed in the issue). But the second action for the dependencies can only be triggered after I finished writing the callback function within the insert node. For this simple snippet this works because I can use the enter and leave event of the snippet ([-1]). And it might be very unlikely that I wanna trigger like three actions with two inserts in-between or similar.

Ahh of course.. yes, for that the leave-callback is more appropriate 👍

I had the idea to use an insert node for this. That would give me a proper index and with the new node snippets, that would be great. The intention was to then just programatically jump to the next node within the callback of the node itself.

Ah, so replace the textNode with an insertNode, so you can get the correct node in the callback, and then jump once more immediately?
Yeah, maybe, but afaict this would just avoid the get_keyed_node, right (ah, and simplify the lsp-callback)? Since events.leave on the insertNode beforeuseEffect-insertNode is during the same interaction as events.enter on the useEffect-insertNode

The intention was to then just programatically jump to the next node within the callback of the node itself

I think this may work if you vim.defer_fn it. Calling jump while another jump is being executed is not a good idea, but with defer it should work just as if you called it manually immediately after the initial jump was done.

And I have to "maintain" it across NeoVim core runtime changes

Feel ya xD

[weilbith]

I think this may work if you vim.defer_fn it. Calling jump while another jump is being executed is not a good idea, but with defer it should work just as if you called it manually immediately after the initial jump was done.

Thank you so much! That is amazing.
Using it plus this branch (using it locally), my code shrinks enormously and becomes very simple. Which I highly appreciate.
There is one little downside and that is that the defer delay also for the language server request (you remember the sync issue) affects the user experience. Though most servers I tried so far work quite well. It is not blazing fast, but pretty great.
I'll post my new solution soon.

In regards of this branch, I'm satisfied for the moment. 🙃

[L3MON4D3]

It is not blazing fast

We're not writing Rust here after all :P

Glad it works better!

[weilbith]

We're not writing Rust here after all :P

I'm afraid Rust can not solve this problem. 😉

---

Thank you very much to implement this feature so fast. Looking forward to get it merged. 😊

[L3MON4D3]

(I've decided to change this a bit from the initial version: now all callbacks that are associated with some node are executed, not just one of them)