New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from Oct 2, 2018

Conversation

Projects
None yet
1 participant
@savetheclocktower
Collaborator

savetheclocktower commented Sep 21, 2018

… 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:

  • Say I use a tab stop several times within a snippet. Some may be “plain” tab stops (e.g., $1 or ${1:some placeholder}) and others may be “transformed” tab stops (e.g., ${1/foo/bar/}).
  • Plain tab stops get handled through the creation of new cursors. If there are 4 $1 references in a snippet, you’ll have four cursors when the active tab stop is 1.
  • If you have several cursors on a tab stop, and want to move them with the arrow keys, it’s impossible to move them in any way that will cause at least one of the cursors to leave its original marker (but move directly to another valid marker) without also causing at least one other cursor to move outside of a tab stop marker altogether. And as long as one cursor does so, the entire snippet expansion will be terminated.
  • If you have several cursors on a tab stop, and want to move them via the mouse, clicking anywhere in the document will reduce you to one cursor; when the additional cursors are destroyed, the snippet is terminated.
  • Therefore this bug can only manifest on transformed tab stops, which don’t have any cursors of their own.

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 added some commits Sep 21, 2018

Always terminate a snippet when the cursor leaves a tab stop, even if…
… it moves into a transformed tab stop of the same index.
@savetheclocktower

This comment has been minimized.

Collaborator

savetheclocktower commented Sep 21, 2018

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

@savetheclocktower savetheclocktower merged commit 3df058f into atom:master Oct 2, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@savetheclocktower savetheclocktower deleted the savetheclocktower:moving-between-tab-stop-markers branch Oct 2, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment