Add variables

[savetheclocktower]

Description of the Change

At long last, this PR adds support for snippet variables.

Variables have been a part of snippets since TextMate invented them. They’ve also been supported in Sublime Text and VSCode for ages, and probably other editors. And variable support is part of the LSP specification for snippets.

This PR is based partly on @Aerijo's pull request from 2019. I borrowed some tests from that PR, and I decided that it was best to start with its overhauled PEGjs file that could already parse all the new constructs. I felt safe doing this because the overhaul ended up passing all of the existing body-parser-spec.js tests, not to mention other tests that would’ve started failing if there were any subtle parser regressions.

Otherwise, the code is my own. For the variables and other features that VSCode added, I copied their exact implementation where possible, since the LSP spec itself doesn’t provide a sample implementation.

README updates

I added documentation for the new feature to this package's README. While I was at it, I figured I might as well replace all mentions of “Atom” with “Pulsar” and make various other custodial changes.

Not yet implemented

Choice syntax

This gets us practially full compatibility with the LSP spec, but the one main absence is the “choice” syntax where a snippet can define an enum of possible values like ${1|one,two,three|}. The idea is that the IDE’s autocompleter will pop up to invite the user to choose between them.

I’m fine with this in concept, but it’s a bit too much to bite off in this PR, so I did the same thing @Aerijo did: make it so that we grok choice syntax, but translate it to something we can support. For instance, ${1|one,two,three|} for now will be treated exactly like ${1:one} — the first choice is treated as placeholder text. In the future, when we can support this more robustly, this fallback behavior will still be useful in situations where the user has disabled autocomplete-plus.

UUID variable

VSCode supports $UUID in a snippet for generating a new UUID each time it’s invoked. I have no problem with this behavior, but I didn’t want to introduce a new dependency for something so obscure.

Luckily, Node defines randomUUID in the built-in crypto package. Unluckily, it was added in v14.17, and we’re currently on v14.16. But I hear we’ll be able to upgrade to a newer Electron version quite soon, so I decided to make UUID support contingent upon the existence of that method. If it exists, we add a resolver for $UUID. That means we’ll get new functionality automatically when Electron gets upgraded.

Comment-related variables

VSCode defines $BLOCK_COMMENT_START, $BLOCK_COMMENT_END, and $LINE_COMMENT for language-agnostic usage of comments inside snippets. We’re close to being able to support this, but the way this is handled right now is via scoped settings that define only editor.commentStart and sometimes editor.commentEnd. To oversimplify: we usually know only the line-comment syntax for a given scope, not the block-comment syntax.

@Aerijo suggested that this data be tied more closely to the grammar in this proposed RFC. I think we should use this approach, but keep the data within scoped settings files.

But I’ll open a separate issue to explain exactly why.

Alternate Designs

I really do admire the aforementioned PR, but I decided to avoid its pitfalls by making this change as incremental as possible. I’m sure there are other good ideas present in its changes, and if so I think we can adopt them incrementally in future PRs.

Benefits

Here are some examples of snippets that are now possible:

<a href="${1:${CLIPBOARD:https://example.com}}">$TM_SELECTED_TEXT</a>$0

Select some text, invoke this snippet via key shortcut, and have that text wrapped in an anchor tag. The contents of the clipboard are assumed to be the URL.

**${1:${TM_SELECTED_TEXT}}**$0

VSCode defines this one (and the italic equivalent) for Markdown files: select text and make it bold with a keystroke. Or, if no text is selected, it’ll insert ** delimiters on either side of the cursor.

## $CURRENT_YEAR-$CURRENT_MONTH-$CURRENT_DAY

Do you mantain a changelog file in Markdown format? Give that a tab trigger and you’ve got a simple way to generate a heading for today’s date.

Possible Drawbacks

I can’t think of any. All the tests pass and there won’t be any performance regressions.

Applicable Issues

Closes #9.

[Daeraxa]

Not something that needs to be addressed instantly but if this is new functionality we want to make sure we document it in the Pulsar docs as well. Happy to do it myself using the content from the readme, this is really just to make sure we have it on the radar somewhere.
Currently we have https://pulsar-edit.dev/docs/launch-manual/sections/using-pulsar/#snippets which will need to be updated.

[savetheclocktower]

Yeah, also happy to do that. Tabstop transformation never even got documented after I added it way back when, so it'll feel good to cover that as well.

[confused-Techie]

At a glance the changes here look awesome, even if a bit in depth. I'd like to take more time to take a good read through everything here. But anyways this is fantastic! And glad to see new features coming about. The work is very much appreciated! And just to make sure we get enough eyes on this one since it is a bit bigger @pulsar-edit/core @pulsar-edit/packages

[savetheclocktower]

Just found a bug when doing some other work: when a snippet is invoked via tab trigger, the package selects the typed prefix so that it will be replaced when the editor inserts the snippet body. But it wasn't selected when the user pressed Tab, so it shouldn't show up in $TM_SELECTED_TEXT. In fact, $TM_SELECTED_TEXT will always be '' when a snippet is invoked via tab trigger.

Wrote a fix and a new spec to cover this case. The download server is acting up, so we'll have to try CI again when it's fixed.

[DeeDeeG]

Just popping in to say: I don't know much about snippets, and all of the diff in this PR is around stuff that's totally new to me, so I don't think I'd be able to provide much timely and meaningful feedback on this one, sorry.

(Responding to the ping and request for reviews/more eyeballs.)

[savetheclocktower]

Just popping in to say: I don't know much about snippets, and all of the diff in this PR is around stuff that's totally new to me, so I don't think I'd be able to provide much timely and meaningful feedback on this one, sorry.

Yeah, I fear that this will take a while to review just because few who regularly use Pulsar are familiar with this feature, or else they'd have clamored for its inclusion by now. :)

Maybe the acceptance criteria can be as simple as trying some of the snippets in the unit tests in both Pulsar and VSCode and asserting that they work roughly identically.

[confused-Techie]

Just popping in to say: I don't know much about snippets, and all of the diff in this PR is around stuff that's totally new to me, so I don't think I'd be able to provide much timely and meaningful feedback on this one, sorry.

Yeah, I fear that this will take a while to review just because few who regularly use Pulsar are familiar with this feature, or else they'd have clamored for its inclusion by now. :)

Maybe the acceptance criteria can be as simple as trying some of the snippets in the unit tests in both Pulsar in VSCode and asserting that they work roughly identically.

I think this is probably the best thing we can do. Or even seeing that all tests pass is a good sign already, even moreso considering you've added some new specs that ensure the functionality of the new feature.

I'm erroring on the side of a light approval here if I'm not able to find the time to review more in depth.

[savetheclocktower]

It's been almost two months. I haven't been lobbying for landing this because I've been, uh, busy with something else, but I think we should make a decision.

If folks want more assurances before we land it, I'm happy to have that discussion. Otherwise I'll be requesting that “light approval” that @confused-Techie mentioned.

[mauricioszabo]

Well, if it works, I agree on adding them.

Only one thing: these pascalcase for example, can we add some testcases for them too?

[savetheclocktower]

Only one thing: these pascalcase for example, can we add some testcases for them too?

There's a new test for the casing flags.

[savetheclocktower]

OK, I'm going to merge this PR sometime this week. Speak now or forever hold your peace!

[mauricioszabo]

Just one word: go for it :)

[confused-Techie]

lgtm