Add command property that registers a command name for a snippet

[savetheclocktower]

Description of the Change

Allows a user or a package author to map a snippet to a command as described in #8.

Let’s use this as an example:

'.text.html':
  'Wrap in HTML tag':
    'command': 'wrap-in-html-tag'
    'body': '<${1:div}>$0</${1/[ ]+.*$//}>'

Putting this in my personal snippets.cson creates a command called Snippets: Wrap In Html Tag. I can map that command to a keystroke and invoke that snippet as follows:

The behavior is just as it would’ve been had I invoked the same snippet via a prefix.

(This technique will become more powerful when support for variables is added later on.)

Packages can also map snippets to command names. If the snippet above had been defined in the language-html package, we’d register language-html:wrap-in-html-tag instead.

Since this is an equally valid way of invoking a snippet, both prefix and command are now optional properties, but a snippet must define at least one of them. (Defining both is also fine.)

The snippets package will not allow you to register two snippets with the same command name, nor will it allow you to register a snippet whose command name tramples on a command that’s already been registered.

Alternate Designs

The whole purpose of this is to map a snippet to a key shortcut, but I didn’t want to do so in a way that would introduce a separate system for mapping keys. Allowing snippets to register commands is the best way to keep all key shortcut information within the places the user expects: their own keymap.cson and the keymaps of each individual package.

Benefits

Here are some ways this can be used.

Braces inside template strings

When I’m typing inside of a JavaScript template string, I want ${ and } to be treated as a typing pair. In other words, much like typing ( also inserts a paired ) and places the cursor between them, I want to make it so that typing $ automatically inserts ${ and }, with the cursor between them.

Right now I implement this as a command in my init.js, but it could be implemented more simply as a snippet:

'.source.js .string.quoted.template':
  'insert interpolation pair':
    'body': '\${$0}'

This is how TextMate implements string interpolation delimiters in Ruby — not as a command, but as a snippet.

(There’s another behavior that needs to be implemented to emulate typing pairs: if I absent-mindedly type the ending delimiter when my cursor is right in front of the one that was automatically inserted, it simply moves the cursor over instead of adding a new one. This is outside the scope of the snippets package, but is implemented pretty easily with a command that does a scope check and calls editor.moveRight() if it passes.)

Inserting snippets programmatically

Other commands in my init.js file are too complex to be implemented solely as snippets, but still insert their output in snippet form because it’s a great way to insert text and position the cursor in one step.

Right now, the only reasonable way to do this is to grab a reference to the snippets package via atom.packages.activePackages.snippets.mainModule, then call insert with a snippet body as the first argument. This is what I do for my “insert padding between paired characters” command:

snippetsModule.insert(' $0 ', editor, cursor);

But this is wasteful, because it forces snippets to parse the snippet body and create a new “anonymous” snippet every time it’s invoked, even though the actual snippet body is the same each time.

It would be good to keep a cache to skip re-parsing for usages like these, but it would also be good for a command to be able to trigger a snippet that has already been defined:

atom.commands.dispatch('custom:pad-inside-pairs');

Future: incorporating selected text

Our original example snippet, wrap-in-html-tag, will become more useful once we support the $TM_SELECTED_TEXT variable. It will let us select text, invoke a hotkey, and include the selection in the snippet’s output.

This isn’t possible with prefixes and Tab triggers. For $TM_SELECTED_TEXT to be useful, we first need a system for allowing defined snippets to be activated while text is selected in the editor.

Possible Drawbacks

I can’t think of many.

When the package first parses and registers a snippet with a command property, it must register a command for that snippet, and that’s an extra code path over a prefix-only snippet. But the performance impact of simply registering that command is negligible, and the performance impact of invoking the command is similar to what happens when the user expands a snippet via Tab.

Applicable Issues

I originally proposed this feature in #8.

[Spiker985]

@pulsar-edit/core-admin Can core/maintain get permissions for this repo?

I'd like to allow the checks, but alas, cannot manage the repo

I would say the alternative is to change my perm level, but I think having someone in the lower permissions allows us to find access issues such as this

[confused-Techie]

@Spiker985 I've approved the CI, and added this to the core team and subteams with proper permissions so you can manage it in the future

[savetheclocktower]

CI failed. Should action-pulsar-dependency be updated to a newer version?

[confused-Techie]

CI failed. Should action-pulsar-dependency be updated to a newer version?

Good catch. It absolutely should be updated. It's using an older bad version, the issues have been fixed in the newest release

[savetheclocktower]

OK, I can rebase off of master when that happens.

[Spiker985]

Actually, if you wouldn't mind, @savetheclocktower, would you try targeting my s985- branch on action-pulsar-dependency to see if it is working appropriately on this?

I have it working on x-terminal-reloaded, minus macOS because of a potential "helper" misnaming problem

And I want to see if that's a problem I've inherited from x-terminal, or something that is actually intrinsic because of the way the macOS install was implemented

[savetheclocktower]

@Spiker985 Made the change — just needs a reapproval.

[Spiker985]

CI Approved! Let's see what happens

[Spiker985]

Oh, duh, I forgot to mention the format changed

You can look at Spiker985/x-terminal-reloaded#14 to see

I have to finish fleshing out the documentation updates

Edit: Looking at this stuff on mobile is a right pain btw

[savetheclocktower]

@Spiker985 OK, let's see if I did this right. Needs another approval

[savetheclocktower]

Legit spec failure on Windows, though this PR doesn't touch that functionality, and the test passes for me on macOS locally. 🤷️

Not sure what went wrong on macOS CI.

[Spiker985]

Alright, it looks like it went through on this one - there's 1 failure on Windows, and it the error has changed for mac - so I'll take a look at my PR to see what I can do about that

[Spiker985]

Alright, removing the xattr command has it failing as before on macOS.

When I looked, I found this which I think is our problem.

[savetheclocktower]

As for the Windows failure: when the macOS side of things is solved, I'll make these changes on #7 and see if that spec still fails. That one definitely has nothing to do with the failing spec.

[savetheclocktower]

@Spiker985 The spec failure on Windows was legit and was probably broken for a while now. Latest commit fixes it.

I don't know why it's making someone approve CI every time. I understand why re-approval is needed after I touch the workflow YAML itself, but this most recent commit didn't do that.

[Spiker985]

It's due to the settings on the repo - once we merge this PR, you won't be restricted like this, as you'll be a collaborator

I assume we may run into this on other repos, though maybe it'll count you as an org contributor instead

[Spiker985]

Alright, so Ubuntu and Windows are functioning as anticipated (and you said that you're developing on mac, so it should be fine)

But now I need to find out why we have a mismatch on the naming between our mac app name - and the helper's name

[Spiker985]

Tada! I'm going to merge pulsar-edit/action-pulsar-dependency#6, and then we can update this to the appropriate version, and then this should also be good to merge

[Spiker985]

Preemptively request changes to match new action versions

[Spiker985]

Forgot the v - my mistake

[savetheclocktower]

Thanks for the approval. I'd love to get this one and #7 landed in anticipation of the PR that adds support for variables. That one is almost ready, but it relies on functionality from #7 and #10.