atom / snippets

Atom snippets package
MIT License
205 stars 100 forks source link

Add support for simple regex transformations in tab stops. #260

Closed savetheclocktower closed 6 years ago

savetheclocktower commented 6 years ago

Note: this is my first deep-dive into adding an Atom feature and I’m happy to get pushback on code formatting, architectural choices, and the like.

Description of the Change

This implements a slimmed-down version of snippet transformations. TextMate’s version of this feature is documented here.

For example, this snippet:

<${1:p}></${1/[ ]+.*$//}>

can be used like this:

snippet-transformation-demo

Design

I went through a bunch of possible ways to design this. Here’s what I landed on:

Worth noting

Mirrored tab stops have always been treated like multiple selections…

snippet-mirror mov

…and that doesn’t change. But transformed tab stops are not treated like selections, because I think a user wouldn’t reasonably expect the same input to produce different output when applied to multiple selections.

Transformed tab stops cannot have placeholders; the first transformation gets applied as soon as the snippet is expanded, so any placeholder value would likely be lost immediately.

What is and isn't included

As explained in this comment, I wanted to go minimal here. The find-and-replace syntax roughly corresponds to the two arguments you'd pass to String#replace (the regex is automatically given the global flag). There's one extra feature that I brought over from TextMate format strings: the ability to control case through the \u, \l, \U, \L, and \E flags:

There are lots of other features of TextMate format strings, but a lot of them are obscure. I went with this small extension because it's quite useful and is also supported by Sublime Text's snippet transformation syntax. Other features, like conditional insertions, could be added rather easily in the future just by updating the parser and the Insertion class.

Alternate Designs

I don’t like how a snippet expansion needs to listen to TextBuffer changes in order to do transformations, but I didn’t see another way. An asynchronous listener doesn’t act quickly enough. But we do listen at the transaction level (TextBuffer#onDidChangeText) rather than the keystroke level (TextEditor#onDidChange).

If there is a simpler way to do this, I’m all for it.

Benefits

Makes snippets far more useful. There are a couple more examples in the (duplicate) ticket that I opened.

Possible Drawbacks

Not sure. Increased code complexity, perhaps?

This PR doesn't change any existing behavior — barely any of the existing tests even needed to be touched — and the theoretical performance penalty of observing TextBuffer transactions should only be encountered when using a snippet with a transformation; for existing use cases there should be no performance regression.

Applicable Issues

Ticket #40 contains the original request.

nathansobo commented 6 years ago

Published as snippets@1.2.0. Thanks for the great work adding this. You combined a bunch of APIs in a pretty complex piece of code to add a really cool feature. I've gone ahead and given you commit bit on this repository ⚡️.

savetheclocktower commented 6 years ago

Thanks! I'll revisit this when 1.25 is out, and the undo/redo thing will be on my radar as well.

nobleach commented 6 years ago

Forgive my misunderstanding but, is this actually available in non-beta releases? I see it's published in snippets@1.2.0 (current is 1.3.3).

savetheclocktower commented 6 years ago

Yeah, it went out with Atom 1.25.0.

nobleach commented 6 years ago

That's great news! Looks fantastic. I'll dig around in the source a bit to see how to use it. Thanks!