atom / snippets

Atom snippets package
MIT License
205 stars 100 forks source link

Define default ending tab stop at the end of the snippet #239

Closed Ingramz closed 5 years ago

Ingramz commented 7 years ago

TextMate manual specifies that if no ending tab stop has been defined explicitly, it should be added to the end of snippet:

If you do not explicitly set $0, the caret will be at the end of the snippet.

For tab stops that do not contain a placeholder, this is behavior wise fine (but will not follow spec), but if the last defined tab stop happens to be a placeholder that is not being replaced, this happens (does not have to be necessarily at the end of the snippet):

without

This pull request checks after processing the tab stops whether the end tab stop is defined ($0) or the last tab stop is already at the end. Otherwise it will include a zero-width end stop to the end of the snippet. TextMate only checks the first condition, second condition was added as means of maintaining compatibility with incorrectly-written end-stops for now. Perhaps this should be presented as warning now and removed after a couple of releases.

with

This however does not fix the case where the explicitly defined end tab stop is a placeholder (${0:placeholder}), which will behave as the first image. TextMate in this case simply overwrites it with a tab, as that is the default behavior of selected text + tab. Atom's behavior of indenting the whole line when any character is selected, does not work very well together with this. Split into atom/atom#15096

Attempt to fix this issue revealed a regression in a test that does not sufficiently cover the regression it was supposed to prevent, which I do not know how to fix. Namely test for t6 fails, which expands the snippet and then calls undo, which deletes all expanded text up to the zero-width end tab stop, which still exists. Calling tab again does not expand t6, but jumps to the position of that end tab stop. Without this pull request, but explicitly defining $# (can be any number) at the beginning/end of that snippet will fail the test as well. Split into #261

It should be obvious that I am not expecting this pull request to be merged before these issues have been solved.

Ingramz commented 5 years ago

Looks like this has been mostly solved by other contributors, however I am disappointed by lack of discussion here regarding that. The single line tabbing behavior is still something worth pursuing, which is why I will only close this pull request.