[snippets] Fix incorrect range traversal when resolving variables

[savetheclocktower]

Identify the Bug

This has been biting me for a while. Given the following snippet:

$LINE_COMMENT $1\n$LINE_COMMENT ${1/./=/g}

This is what I expect:

• Two lines, each beginning with the correct line comment delimiter for the language
• Each character I type on the line above is mirrored on the line below, except that it's converted to =

This produces a lovely banner comment… until I try it again with leading whitespace.

snippets-before.mov

When we expand a snippet, we know where the variables are within the snippet; they're described using Points, but the origin of the coordinate system is the beginning of the snippet. To translate them into actual buffer Points during snippet expansion, we “add” each point to the Point that marks the insertion point of the snippet: e.g., “start at (1, 2), then go right two columns and down one row.“

When doing so, we need to remember that, when a snippet contains a newline and content after that newline, that second line of content will be indented by the same amount as the initial line. We know how much leading whitespace (if any) there was before the snippet trigger text. Folks tend to want a snippet to be indented an equal amount on each line unless they explicitly add/subtract space on subsequent lines; so if a snippet creates a new line of text, we make sure it starts with an equal amount of whitespace.

Description of the Change

For this reason, we were adding the two points incorrectly. Since the column position doesn't reset to 0 each time a row is advanced, we should've been using Point#translate, not Point#traverse.

I'd have caught this earlier if I had thought to test the combination of variable expansion and leading whitespace.

Does it work?

snippets-after.mov

(I see other usages of Point#traverse in the same file, but those contexts are different, so I'll leave them be until they can be proven to be the source of a bug.)

Alternate Designs

Since this was an incredibly simple change, I considered no alternate designs.

Possible Drawbacks

None.

Verification Process

Added a new spec for this behavior; all the existing specs pass. If the button's green, click it!

Release Notes

• [snippets] Fixed an issue with expanding snippet variables in certain scenarios if the snippet inserted new lines into the buffer.

[savetheclocktower]

Despite my long-term goal to make all of you realize that you need snippets in your life, I understand that some people may not feel they have enough expertise to review this one or understand what's happening. This isn't the first time this has happened.

Since this change has a spec to support it and does not regress any existing tests, I view this as exceedingly safe to merge, so I'll set a reminder on my calendar to merge this PR on Thursday. Feel free to take a look before then, but feel no obligation.