occivink / kakoune-snippets

Snippet support for kakoune
The Unlicense
46 stars 6 forks source link

Matching algorithm based on backwards search #14

Closed JJK96 closed 5 years ago

JJK96 commented 5 years ago

Do a backwards search to select a possible snippet trigger. This has the benefit of being able to expand snippets that are not neccessarily a word as selected by b and being able to select a trigger even when it is attached to another word.

For example, suppose my trigger is called "test" and I have the following buffer:

footest[]

Then test will be expanded as a snippet.

occivink commented 5 years ago

The usecase is for expanding on a keypress and not as-you-type, right? I think this approach is too greedy for as-you-type, since it will execute a <a-?> on every key, which might go through the entire buffer. But even for the on-keypress case, it seems a bit much that a trigger way up in the buffer can be expanded. Regardless, I think we would separate the logic for the two cases

Note that snippets_expand_filter is only really useful for the as-you-type case since we want to exit as quickly as possible. In the on-keypress case, we can definitely afford the shell scope.

I'm also not sure about removing the implicit \b that are added to the triggers. On one hand you could add them to each snippet triggers, e.g. \btest\b but that's rather annoying

JJK96 commented 5 years ago

My usecase is indeed for expanding on keypress.

For me it didn't try to expand a snippet further up in the file, but looking at the code I am confused as to why not. I adapted the code to prevent this possible issue anyway.

About the performance, I understand that searching in the whole file every keypress may be slow. I wonder how this alternative is for performance?

        exec -draft %{hGgs%opt{snippets_expand_filter}<a-!>\z<ret><a-;>"aZ}

Will this start searching before the cursor since it needs to match \z? Or will it simply search for the regex in the whole buffer.

Another option would be:

        exec -draft %{hGhs%opt{snippets_expand_filter}<a-!>\z<ret><a-;>"aZ}

This pull request fits my usecases better than the current implementation, for example, now I can use /* as a trigger for a block comment while with the current implementation that was not possible.

occivink commented 5 years ago

Ok, I guess we can remove the implicit \b to allow for triggers such as /*. However, remember that the trigger is interpreted as a regex (so that \b or ^ can work) so you'll have to use /\*. Regarding the ability to expand a trigger anywhere, I think my preferred solution would be to do

# select the text you want
reg / %opt{snippets_expand_filter}
exec <a-?><ret>
# don't modify the selection, only expand it if it matches
snippets-expand-trigger
# somehow restore the original selection if it wasn't a proper trigger

So that we don't have hardcode the logic for selecting the trigger, and you'll be able to do it for the same line, or the entire buffer, or just the previous word.

JJK96 commented 5 years ago

I don't exactly see how you want that snippet to work, since snippets-expand-trigger still uses the b argument to snippets-expand-trigger-internal. Or do you mean a new implementation of that command?