joaotavora / yasnippet

A template system for Emacs
http://joaotavora.github.io/yasnippet/
2.76k stars 314 forks source link

Adding support for regex as keys when expanding snippets #988

Open Zetagon opened 5 years ago

Zetagon commented 5 years ago

I read https://castel.dev/post/lecture-notes-1/ and wanted to replicate at least the simpler parts of the fraction snippet. Is it possible to easily write such snippets in yasnippet where the key is a regex?

In case it is not currently possible I have hacked together some elisp code using the interface provided that can do this but it is not very integrated into yasnippet. I have basically made a new keybinding that checks if a regex has matched. If it matched a corresponding snippet is expanded using yas-lookup-snippet and yas-expand-snippet. The configuration still happens in elisp code and not in a snippet buffer which is suboptimal.

Is anyone else interested in using regex as keys? If so I could with some help integrate this more into yasnippet and merge it.

npostavs commented 5 years ago

Is it possible to easily write such snippets in yasnippet where the key is a regex?

It's not currently possible.

Sounds like an interesting feature, though I think it breaks the current decision of looking up the snippet key in a hashtable, so implementation might be somewhat involved. I guess the check against regexps could just happen first, and then fallback to hashtable lookup otherwise.

114 (Multiple keys) is sort of related.

Zetagon commented 5 years ago

My current approach is to do a regex match on from the beginning of the line to the cursor so that the search space is small. The regex is appended with a '$' so that the matched part is guaranteed to end at the cursor. The matched string is then deleted and placed in a global variable so that the snippet can use it. yas-expand-snippet is then used to find a snippet in the current mode.

I could submit it as a pull-request but I feel like it would be nicer if the regex could be defined in the snippet file instead of in elisp code.

npostavs commented 5 years ago

The regex is appended with a '$' so that the matched part is guaranteed to end at the cursor.

You can use \= instead. (elisp) Regexp Backslash

\= matches the empty string, but only at point. (This construct is not defined when matching against a string.)


it would be nicer if the regex could be defined in the snippet file instead of in elisp code.

Agreed.

npostavs commented 5 years ago

I could submit it as a pull-request but I feel like it would be nicer if the regex could be defined in the snippet file instead of in elisp code.

How would you feel about submitting a pull-request implementing the regex-in-snippet-file approach?

Zetagon commented 5 years ago

I would love to! If you could give me a few pointers to bits of code that are relevant I think I could do it.

npostavs commented 5 years ago

Okay, first off I should mention that since yasnippet is in GNU ELPA, you need to assign copyright before I can install subsantial (roughly defined as more than 15 lines) patches from you. To start the copyright assignment process, fill in this form, with "Emacs" as the program name, and send it to the mentioned email address.


As for code pointers, I'm thinking you'll want to allow the key field in the yas--template struct to be a list, with elements being a string (plain key) or (:regexp REGEXP) where REGEXP is the regexp key as a string.

yas-define-snippets and yas--add-template should be updated to handle this. For the latter, I think it should maintain a :regexp entry with an alist of (REGEXP . NAME) in the snippet key table (since we can't use hash lookup for regexps anyway). Then yas--fetch will have to iterate over the regexps and return the matching entries.

And finally, update yas--parse-template to accept a # regexp-key: header, or similar.

Those are my thoughts about it without trying to implement anything, so it's possible I've made some design bug in the above: if something doesn't make sense, don't hesitate to ask questions.

netjune commented 5 years ago

I read https://castel.dev/post/lecture-notes-1/

So many cool features of Ul­tiSnips. Hope they will be implemented in yasnippet.

Zetagon commented 5 years ago

So I have realized that the process for expanding regular snippets and regexp snippets will probably be different. I am thinking that regexp snippets should delete the text that matched the text and put that text into a variable like yas-matched-text that the snippet could use.

I think that it might be better to modify yas-expand or some other function that deals with expanding a snippet.

npostavs commented 5 years ago

I am thinking that regexp snippets should delete the text that matched the text

I think this part is already covered in yas-expand-snippet according to the BEG and END arguments. This is passed down from yas-expand-from-trigger-key, see the comment ;; Delete snippet key and active region when expanding.

put that text into a variable like yas-matched-text that the snippet could use.

This part is missing though.

Zetagon commented 5 years ago

I noticed that yas-expand-from-trigger-key expects one position and multiple templates which is not compatible with regexps because the text matched by the regexps don't have to be of the same length. The easiest solution seems to be to make a list of positions for each template.

npostavs commented 5 years ago

The easiest solution seems to be to make a list of positions for each template.

It doesn't look like any callers are interested in any template apart from the first, maybe it's easier to always return a single template, with a single position.

Zetagon commented 5 years ago

Sorry if this is off topic but how do I debug tests in yasnippet? If I run ert as is described in the test file I just get the message that there are unexpected errors. I tested running ert manually so that I can get a backtrace but the backtrace is so large that I can't even copy the backtrace without Emacs hanging or lagging immensly.

npostavs commented 5 years ago

If I run ert as is described in the test file

Oh, maybe it's because ert-run-tests-batch-and-exit doesn't exist in the old ert that works with Emacs 22 (I'm not actually sure if yasnippet works in such an old Emacs), but normally it should look like this: emacs --batch -Q -L . -l yasnippet-tests.el -f ert-run-tests-batch-and-exit

the backtrace is so large that I can't even copy the backtrace without Emacs hanging or lagging immensly.

Which Emacs version are you using? I think there was a bug in 26.1 that caused it to ignore print-length when making backtraces (should be fixed in 26.2, if I recall correctly). You can probably work around this by setting debugger-print-function to prin1 (as it was in earlier Emacs versions).

Alternatively, use emacs -Q -L . -l yasnippet-tests.el -f ert to test interactively. Then you can use . on the failing tests to jump their definitions, instrument them with edebug and step through (note that the snippet loading tests pass only once in an Emacs session because they rely on global state, it's something I've been meaning to fix, but never reaches high enough priority).


If you have rake installed, you can use rake tests and rake itests to run those command lines.

Zetagon commented 5 years ago

First off it seems like my Emacs version was the issue, I had Emacs 26.

I tried a different approach which you can find here here. It is by no means finished but it outlines the approach.

First we parse a new directive called "regexp-key". yas--table has a new entry called regexp-templates which is an alist with entries on the form (regexp . template). A new interactive function called yas-expand-regexp is supposed to be bound to say tab. It tries to expand a snippet with a regexp key and if it failed calls yas-expand. IMO it feels like a simpler solution.

The reason that I went for the approach above was because I got type errors because key wasn't a string. It felt error prone to change such a basic assumption.

I noticed that was that snippets with a direct key-binding don't have to have a key. Can we add to the mechanism that is used there in some way to solve the problem above?

npostavs commented 5 years ago

First we parse a new directive called "regexp-key". yas--table has a new entry called regexp-templates which is an alist with entries on the form (regexp . template).

That sounds fine.

A new interactive function called yas-expand-regexp is supposed to be bound to say tab. It tries to expand a snippet with a regexp key and if it failed calls yas-expand. IMO it feels like a simpler solution.

Hmm, I'm not sure about this. Looks like you somewhat arbitrarily decide to match the regexp against the buffer text from the beginning of line.

(text (buffer-substring-no-properties (line-beginning-position) (point)))
(matched-index (string-match regexp text))

I think it's better if you put the regexp matching in yas--templates-for-key-at-point so that it respects yas-key-syntaxes.

The reason that I went for the approach above was because I got type errors because key wasn't a string. It felt error prone to change such a basic assumption.

Could we perhaps have yas-define-snippets accept the KEY element as a list, while keeping the internal yas--template stuff with an extra field like you have done.

In retrospect, perhaps I should have suggested starting by implementing #114 (multiple keys per-snippet). Then this issue could have been tackled in isolation from the regexp matching stuff.

Zetagon commented 5 years ago

Hmm, I'm not sure about this. Looks like you somewhat arbitrarily decide to match the regexp against the buffer text from the beginning of line.

The reason I chose a limit is because it is expensive to match backwards as regexps really only can search forwards. One solution is therefore to search forwards in a substring of the buffer, the limit could be the beginning of the line or the buffer. Another could be to do (reverse (buffer-substring-no-properties (point-min) (point))) to make regexps search backwards. It feels hacky though. Edit: Code is structured in lines so I felt that it would be natural to limit the regexp matching to the current line.

I think it's better if you put the regexp matching in yas--templates-for-key-at-point so that it respects yas-key-syntaxes.

I am not sure I understand this. Is it not the point to have keys that do not respect yas-key-syntaxes? I would want to have snippets that expand on surrounding parentheses(ex. (23 + 4) ) but that does not seem to be possible if yas-key-syntaxes was respected.

In retrospect, perhaps I should have suggested starting by implementing #114 (multiple keys per-snippet). Then this issue could have been tackled in isolation from the regexp matching stuff.

That issue could be fixed with regexp though. key1\\|key2 would match either key1 or key2.

I noticed that was that snippets with a direct key-binding don't have to have a key. Can we add to the mechanism that is used there in some way to solve the problem above?

I dug around a bit more and found that perhaps only two changes were actually necessary. You can view them here.

Edit2: I also tried moving the regexp-matching to yas--templates-for-key-at-point but without respecting yas-key-syntaxes here.

npostavs commented 5 years ago

The reason I chose a limit is because it is expensive to match backwards as regexps really only can search forwards.

Well yes, there has to be some limit. The question is whether it should be configurable or not.

Code is structured in lines so I felt that it would be natural to limit the regexp matching to the current line.

Might be okay. It's true that using word boundaries and such in the regexp might subsume some of the things that yas-key-syntaxes allows. Plus regexp are already sort biased towards lines (e.g., "." corresponds to any character but newline).

By the way, what about using (re-search-backwards (concat "\\(?:" REGEXP "\\)\\=") (line-beginning-position) t) instead of buffer-substring etc?

I am not sure I understand this. Is it not the point to have keys that do not respect yas-key-syntaxes?

I don't know, you opened the feature request, so I guess you have a better of what the point is than I do :)

I would want to have snippets that expand on surrounding parentheses(ex. (23 + 4) )

Can you elaborate on this a bit, I'm not quite following what you mean here. Where would point/cursor be in your example? What's the sequence of input?

Zetagon commented 5 years ago

By the way, what about using (re-search-backwards (concat "\(?:" REGEXP "\)\=") (line-beginning-position) t) instead of buffer-substring etc?

I don't know a lot of elisp at all but maybe that has better performance? Is there a way to limit the search space with that option? In any case it should be easy to swap between the two.

I am not sure I understand this. Is it not the point to have keys that do not respect yas-key-syntaxes?

I don't know, you opened the feature request, so I guess you have a better of what the point is than I do :)

Well then I don't want them to follow yas-key-syntaxes.

I would want to have snippets that expand on surrounding parentheses(ex. (23 + 4) )

Can you elaborate on this a bit, I'm not quite following what you mean here. Where would point/cursor be in your example? What's the sequence of input?

Ok. Let | be the point and (23 + 4)//| be the buffer. I want a snippet that expands that buffer to \frac{23 + 4}{ | }.

npostavs commented 5 years ago

By the way, what about using (re-search-backwards (concat "\(?:" REGEXP "\)\=") (line-beginning-position) t) instead of buffer-substring etc?

Is there a way to limit the search space with that option?

Yes, that's what the (line-beginning-position) is there for.

I don't know a lot of elisp at all but maybe that has better performance?

Perhaps marginally better, it probably doesn't make that much difference. More importantly, it requires the match to end at point (that's the \\=); if I read your current code correctly, it would find matches even if they don't end at point? The re-search-backwards function does have the potential problem that it's not entirely symmetric with re-search-forwards, so this way could have some unexpected results (see (elisp) Regexp Search).

Ok. Let | be the point and (23 + 4)//| be the buffer. I want a snippet that expands that buffer to \frac{23 + 4}{ | }.

What would the regexp key be in this case? ([^)]*)//?

From my point of view, yas-key-syntaxes is just a way to make the search space user configurable, so I'm still not quite sure what your objection is to it. I don't see why yas-key-syntaxes would be a problem for this example.

Zetagon commented 5 years ago

if I read your current code correctly, it would find matches even if they don't end at point?

No it won't because I append "$" to the regexp in yas--parse-template. That might be a bit unclear though, I should probably move that to where the matching is done.

From my point of view, yas-key-syntaxes is just a way to make the search space user configurable, so I'm still not quite sure what your objection is to it. I don't see why yas-key-syntaxes would be a problem for this example.

Well I just realized that we could probably skip limiting to the current line and just go with yas-key-syntaxes to achieve a similar but more customizable effect. You were right all along!

npostavs commented 5 years ago

No it won't because I append "$" to the regexp in yas--parse-template. That might be a bit unclear though, I should probably move that to where the matching is done.

Oh, yeah, I missed that. In addition to being unclear, it means that snippets created through yas-define-snippets won't get this extra "$"; that could lead to some confusing bugs I think.

Well I just realized that we could probably skip limiting to the current line and just go with yas-key-syntaxes to achieve a similar but more customizable effect. You were right all along!

I'm not declaring victory yet, it might still turn out that the yas-key-syntaxes users want for their plain key snippets are not suitable for their regexp key snippets. But we'll find out when people start using the new feature I guess.

serycjon commented 5 years ago

Well I just realized that we could probably skip limiting to the current line and just go with yas-key-syntaxes to achieve a similar but more customizable effect. You were right all along!

I do not think this will work very well. I have tried tweaking yas-key-syntaxes and playing with regex based condition for snippet expansion before (trying to implement the snippets from the castel.dev blogpost) and it worked, but suddenly I started getting regular snippet expansions where I didn't want them. Basically I have enabled searching till the previous space with yas-longest-key-from-whitespace in yas-key-syntaxes, but regular snippet keys that were suffix of usual words started to cause problems. E.g.: I have "st" to expand into "action='store_true'" in python, but with the extended yas-key-syntaxes it expands even when I type "best", etc.

Don't know what a good solution should look like, but I would vote not to do it this way.

Zetagon commented 5 years ago

@serycjon Well in your example I feel like it is a bad key rather than weirdness from yas-key-syntaxes. In your case it would be better to have a regular key, not a regexp-key.

serycjon commented 5 years ago

@serycjon Well in your example I feel like it is a bad key rather than weirdness from yas-key-syntaxes. In your case it would be better to have a regular key, not a regexp-key.

The "st" key was a regular key, that is the point. I think that it would often make sense to have different yas-key-syntaxes for regular keys and for the regex keys, thats why I have commented. Basically I try to support with my own experience what @npostavs has said about not declaring victory yet.

Zetagon commented 5 years ago

Oh yeah, I get what you mean now. That would become very weird indeed. Thank you for pointing that out.

Zetagon commented 5 years ago

Well I am back with a new report.

Multiple regexp-keys can match the same text. In yasnippet this is solved by prompting the user but I wanted to test another approach. I tested implementing an internal order between regexp-keys so that the user can specify what order they should be considered. The reason I chose to do it this way is because currently only the first matching regexp-key is expanded. What do you think?

I also made regexp-keys respect yas-key-syntaxes. To solve @serycjon 's problem I made a separate variable called yas-regexp-key-syntaxes that behaves the same so that regexp-keys and normal keys are kept separate.

I changed using $ to \\' so that key-syntaxes that span over multiple line work.

Lastly I have added some tests and documentation.

npostavs commented 5 years ago

Multiple regexp-keys can match the same text. In yasnippet this is solved by prompting the user but I wanted to test another approach. I tested implementing an internal order between regexp-keys so that the user can specify what order they should be considered.

Hmm, not sure I want to start adding yet another mechanism for configuring this. Do you think it would be very common to have overlapping regexp keys?


Why do you have a new yas-expand-regexp command which duplicates the matching code from yas--templates-for-key-at-point? Shouldn't yas-expand be expanding regexp keys too?

(defvar yas-matched-regexp-key nil
  "The text that was used as a key for this snippet, if it was expanded using a regexp-key.")

I think it's better to name it yas-matched-key and bind it for non-regexp snippets as well.


PS any progress on the copyright assignment? I don't see any likely matching name in the list

Zetagon commented 5 years ago

Hmm, not sure I want to start adding yet another mechanism for configuring this. Do you think it would be very common to have overlapping regexp keys?

The article in the original post had snippets that depended on order so I figured I would implement it. I don't think the implementation is very complicated as it uses the mechanisms already in place.

Why do you have a new yas-expand-regexp command which duplicates the matching code from yas--templates-for-key-at-point? Shouldn't yas-expand be expanding regexp keys too?

That is something that should be removed. I moved the code from yas-expand-regexp to yas--templates-for-key-at-point and forgot to remove it.

I think it's better to name it yas-matched-key and bind it for non-regexp snippets as well.

Fair enough.

Edit: That would be kinda pointless though as normal keys are constant and are declared in the same snippet as the variable would be used.

PS any progress on the copyright assignment? I don't see any likely matching name in the list

I thought I would do it when I was done. I should start doing that now though as most of the code seems to be complete.

npostavs commented 5 years ago

I thought I would do it when I was done. I should start doing that now though as most of the code seems to be complete.

Oh, I think it's better to start early, as most of the process involves waiting, so it can easily be done in parallel.

npostavs commented 5 years ago

The article in the original post had snippets that depended on order so I figured I would implement it. I don't think the implementation is very complicated as it uses the mechanisms already in place.

It's less about complication on the implementing side, and more about complication for the user trying to configure things. But maybe it's practically a necessity to be able to order regexp keys.

Edit: That would be kinda pointless though as normal keys are constant and are declared in the same snippet as the variable would be used.

Yeah, but it would be nice if a snippet could keep working even if you happen to remove its regexp, and use a plain key instead. Plus it makes the documentation a bit simpler.

Zetagon commented 5 years ago

It's less about complication on the implementing side, and more about complication for the user trying to configure things. But maybe it's practically a necessity to be able to order regexp keys.

I think having #regexp-order as a directive is not in the way of the user as it is optional to add it. I agree that it could clutter the documentation though.

Yeah, but it would be nice if a snippet could keep working even if you happen to remove its regexp, and use a plain key instead. Plus it makes the documentation a bit simpler.

Yeah it would probably be simpler for the user. Another benefit could be that it is easier to change key definitions.

matthuszagh commented 4 years ago

I've started using the fork created by @Zetagon and am really enjoying the functionality. Thanks for working on this! Would love to see merged in when you feel it's ready.

Zetagon commented 4 years ago

Yeah the only thing that I'm waiting for is an approval for assignment of copyright but I have not gotten a response in months. Maybe it is time to send another mail again...

ymarco commented 4 years ago

...Any updates on this?

Zetagon commented 4 years ago

Well I sent another mail. Hopefully they will respond this time.

Zetagon commented 4 years ago

I have sent the request now so hopefully we can start merging now!

tecosaur commented 4 years ago

@Zetagon how's it going?

Zetagon commented 4 years ago

I'm struggling a bit to get a disclaimer from my university. They will not try to claim my work, it's just a little bit of confusion that needs to be cleared up. After that's done I think we can actually begin the work to merge this in!

tecosaur notifications@github.com writes:

@Zetagon how's it going?

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/joaotavora/yasnippet/issues/988#issuecomment-600776785

mohkale commented 4 years ago

I had the same issue with my uni... be prepared to wait a couple months 😭.

maikol-solis commented 4 years ago

Any update on this?

Zetagon commented 4 years ago

Yes! I might finally get the papers signed! There was an issue where the guys at FSF forgot to attach the disclaimer for my school so I had to wait some extra rounds in email-limbo...

Edit: Papers have been signed! Now I just have to get a confirmation from FSF.

Zetagon commented 4 years ago

Papers are signed! @npostavs Are we ready to merge now?

OceanS2000 commented 3 years ago

Any updates here? It will definitely be an interesting feature

Zetagon commented 3 years ago

For those that want to use this for latex: there is a very nice package called https://github.com/tecosaur/Auto-LaTeX-Snippets that works very nicely for inputing latex-snippets. There is no readme but the idea is that you type in "ascii math" (e.g. a/b) which gets converted to latex math on the fly (\fraction{a}{b}).

ymarco commented 3 years ago

https://github.com/tecosaur/Auto-LaTeX-Snippets

A readme is coming, once I finish separating the backend from the actual snippets :)

maikol-solis commented 3 years ago

https://github.com/tecosaur/Auto-LaTeX-Snippets

A readme is coming, once I finish separating the backend from the actual snippets :)

It looks very promising. I'll follow your repo.

ymarco commented 3 years ago

Backend seperation complete! The backend now lives at ymarco/auto-activating-snippets while the pre-typed LaTeX snippets are on tecosaur/LaTeX-auto-activating-snippets. Both complete with READMEs and configuration examples.

maikol-solis commented 3 years ago

Backend seperation complete! The backend now lives at ymarco/auto-activating-snippets while the pre-typed LaTeX snippets are on tecosaur/LaTeX-auto-activating-snippets. Both complete with READMEs and configuration examples.

Awesome work guys! Please submit those packages to MELPA! Thank you!

mohkale commented 3 years ago

Ping.

deshearth commented 2 years ago

Any update on this feature?