atom / snippets

Atom snippets package
MIT License
206 stars 102 forks source link

Always terminate a snippet when the cursor leaves a tab stop… #273

Closed savetheclocktower closed 5 years ago

savetheclocktower commented 5 years ago

… even if it moves into a transformed tab stop of the same index.

Description of the Change

Consider this snippet:

# begin ${1:foo}
$0
# end ${1/./\U/}

You invoke its tab trigger and type bar. The text is mirrored and transformed as BAR two lines below. What’s next? Choose your own adventure:

  1. Pressing Tab will move you to the final tab stop and terminate the snippet.
  2. Pressing will terminate the snippet because the snippets package is keeping track of the markers associated with each tab stop and bails if the cursor goes somewhere that isn’t within one of those markers.
  3. Clicking within BAR to move the cursor inside there should also terminate the snippet. But it doesn’t, because the cursor is still within one of the markers for tab stop 1. This is incorrect.

Scenario 3 should behave identically to scenarios 1 and 2.

This happens because we’re not checking whether the cursor remains inside a specific marker. I was worried that fixing this would require us to do a lot of bookkeeping on cursor/marker relationships, but here’s why that can be avoided:

So the fix is targeted: for the snippet to stay active after the cursor moves, it must remain inside of the marker for an insertion that is not a transformation.

Alternate Designs

Before I realized that I could do this in just a few lines, I’d introduced a whole system for tracking the active marker of each cursor and comparing it to its previous marker when the cursor moved. That also worked. But this is, uh, way simpler.

Benefits

This fixes a bug with one of the snippets I had in mind when I wrote #260.

// ${1:SOME TEXT}
// ${1/./=/}

Expand that snippet, type foo, observe how the bottom line lengthens automatically as you type… then press and hit Enter. Nothing happens. Since the cursor is still in a $1 region, all input will trigger the transformation of foo into === and replace all content inside the region, including the line break you’re trying to insert.

This PR fixes that by ensuring that a marker for a transformation never allows a cursor to move inside of it without terminating the snippet. The new unit test is based on the example above.

Possible Drawbacks

It’s not at all intuitive that this code change produces this outcome, which is why I tried to comment this code exhaustively.

Applicable Issues

None, as far as I can tell.

savetheclocktower commented 5 years ago

As before, I’ll leave this open for a couple days in case anyone wants to suggest a different approach.