atom / snippets

Atom snippets package
MIT License
205 stars 101 forks source link

Implement $0 as a snippet exit point (à la TextMate or Sublime Text) #278

Closed valkalon closed 5 years ago

valkalon commented 6 years ago

Prerequisites

Description

Hi! Since this is my first contribution here, I'm happy to receive feedback on how to improve.

Take the following snippet:

'.source.ts':
  'console log':
    'prefix': 'cl'
    'body': 'console.log($0)'

Current behavior If you ype "cl" then "Tab" your snippet expands.

If you push "tab" again, nothing happens.

If you push "tab" again, a tab character is added

Expected behavior If you type "cl" then "Tab" your snippet expands.

If you push "tab" again, a tab character is added (no "useless" tab)

Versions

Atom : 1.31.1 Electron: 2.0.7 Chrome : 61.0.3163.100 Node : 8.9.3 apm 2.1.1 npm 6.2.0 node 8.9.3 x64 atom 1.31.1 python 2.7.12 git 2.7.4 Ubuntu 16.04.4, 4.4.0-137-generic, Unity, 64bits

Why this enhancement would be useful The current behavior feels strange when you push tab and nothing happens.

The expected behavior saves a keystroke. It gives a meaning to $0 which becomes the "snippet exit point" variable.

It becomes more consistent with other text editor.

This enhancement is also related to the following issue:

Should snippets use $0? https://github.com/atom/language-c/issues/262

Other text editors or applications where this enhancement exists Expected behavior is the behavior as described in TextMate documentation, https://macromates.com/manual/en/snippets#tab_stops

it's also Sublime Text behavior. http://sublimetext.info/docs/en/extensibility/snippets.html#fields

Additional Information

Would you like to help implementing this feature? Yes, I've actually already implemented it locally.

To try it: in file "lib/snippet-expansion.coffee"

replace your existing "goToNextTabStop" function by

goToNextTabStop: ->
  nextIndex = @tabStopIndex + 1
  if nextIndex < @tabStopMarkers.length
    if @setTabStopIndex(nextIndex)
      true
    else
      @goToNextTabStop()
  else
    if @snippet.tabStopList.list.Infinity? # If snippet uses $0 value, exit snippet
      succeeded = false
      @destroy()
    else
      succeeded = @goToEndOfLastTabStop()
      @destroy()
      succeeded

Does your modification break an existing feature? Yes for those who use placeholders with $0

Ex. ${0:myplaceholder}

The placeholder won't work since $0 is the point where the snippet exists.

savetheclocktower commented 6 years ago

Thanks so much for doing this research.

This behavior was introduced as part of #262, ostensibly by design. I think that the purpose of that PR seems to have been to introduce an implied $0 at the end of any snippet. But snippets with an explicit $0 get some of the same behavior applied, and whether this was intentional or not, I’d argue that they should be treated differently than #262 treats them.

I think if you’ve got this snippet…

foo.bar(${1:'baz'}, $2, $3);

…that it’s sensible, once you reach tab stop 3, for the next Tab to take you to the end of the snippet.

But if you’ve got…

foo.bar(${1:'baz'}, $2, $0);

…then you’re using $0 to explicitly indicate where the cursor should go at the end of the snippet. Once you hit tab stop 0, I don’t think that Tab should bring you to the end of the snippet, and I definitely don’t think that it should do nothing at all, as is the current behavior.

I think your fix brings us back to an intuitive behavior, @valkalon, but it breaks the unit test that got touched in #262, so I just want to get clarity on how this should behave before I proceed. @nathansobo, want to weigh in?

savetheclocktower commented 6 years ago

If another maintainer doesn’t weigh in, BTW, I’m inclined to move forward despite the behavior change, since you’re right about how Sublime Text behaves. A stated goal of #262 was to make Atom behave more like Sublime Text, which I’d take as further evidence that the current behavior is unintentional. I’ll just give it a few days to make sure.

valkalon commented 6 years ago

Your description is exactly what I have in mind as a natural snippet behavior. Thanks a lot for the feedback @savetheclocktower. I'm happy to provide further help if needed

lee-dohm commented 5 years ago

This sounds fine :+1: