Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Fix behavior when inserting content between two adjacent tab stops #281

Closed
wants to merge 3 commits into from
Closed

Fix behavior when inserting content between two adjacent tab stops #281

wants to merge 3 commits into from

Conversation

savetheclocktower
Copy link
Contributor

@savetheclocktower savetheclocktower commented Oct 24, 2018

Fixes #15.

Description of the Change

The problem

Here’s a simple example of a snippet that has never quite worked right.

${2:bar}${3:baz}

I’ll quote from my comment in #236 to explain how this goes wrong:

I’ll use parentheses to illustrate the invisible markers. After tab expansion, you’ve got:

(bar)(baz)

You type x and now you’ve got:

(x)(baz)

The editor knows to put x into the first marker because bar was selected, and that removed all ambiguity. But right now the cursor is between x and baz, so when you type text, it could plausibly get added to either marker. We want this…

(xyzz)(baz)

…but we’re getting this instead:

(x)(yzzbaz)

The fix

All snippet markers are getting created with exclusive: false. That means that they’ll expand to claim text that’s typed at their boundaries. But when inserted text is right between two adjacent markers, the one on the right always wins. The only way to force it in the direction you want is to create one marker with exclusive: true and one with exclusive: false.

In our example, $2 is the active tab stop, so we want its marker to claim the characters that we type. But when we hit Tab and make $3 the active tab stop, we want the opposite behavior. So we’ll have to change the settings on the markers as the active tab stop changes — or, rather, copy them to new markers with the settings we want.

There’s one more edge case to consider, and I wasn’t sure how to conceptualize it until I peeked at VScode’s strategy for avoiding this problem: tab stops can reference other tab stops in their placeholder content. If $2 has a placeholder that includes a reference to $1, then $2’s marker needs to be inclusive when either $1 or $2 is the active tab stop, because we need $2’s marker to grow as we type the value for $1. If you find this hard to wrap your brain around, you’re not the only one.

Alternate Designs

The marker API, though robust, is not so rich as to give me lots of options here. The fact that this implementation is nearly identical to VScode’s is validation of the approach.

There were a few different ways I could’ve used to determine the cross-references between tab stops, their placeholders, and any tab stops those placeholders referenced. I decided to capture those references just after parsing, as we consume the syntax tree. Another option would’ve been to inspect the markers after creation and infer the relationships based on which markers were completely surrounded by other markers, but that felt like an indirect approach.

Benefits

A number of snippets that seemed like they should just work haven’t worked (or have worked incorrectly) for a long time, and now they will.

Possible Drawbacks

The fact that we have to destroy and recreate markers as tab stops move is not ideal, but I don’t think it’s a bottleneck. If it turns out to be a problem, we can make the changes necessary in text-buffer to allow the exclusive setting to be changed after creation on an existing DisplayMarker.

Applicable Issues

#236 is a duplicate of #15 but contains a lot of relevant discussion.

@savetheclocktower savetheclocktower mentioned this pull request Jun 17, 2019
15 tasks
Aerijo pushed a commit to Aerijo/snippets that referenced this pull request Jun 18, 2019
@savetheclocktower
Copy link
Contributor Author

Closing in favor of #312.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Snippet placeholder bug
1 participant