ultisnips icon indicating copy to clipboard operation
ultisnips copied to clipboard

Tab navigation discussion

Open hl037 opened this issue 3 years ago • 1 comments

To fix #1405 , I need to make some changes about the tabstops.

However, the current tab navigation is... well to be honest quite messed up... ...If you nest a certain amount of tab it doesn't even work : Try expand the following snippet and navigate accross the tabstops...

class TabStop_Attack(_VimTest):
    snippets = ("test", "${1:foo${4:${2:zzz}bar$3fo}}")

I started to work on that too, but I see no reference for how it should work (except the test suite, with some quite convoluted test cases !)

...I propose this :

  1. As long as the user makes no changes to the document, no matter the cursor movements, all the snippets are kept alive.

  2. If the user makes a change in a tabstop, all snippets that are not child of the tabstop remain a live

  3. When the user make a change outside of a tabstop, this happens : 3.1) The text objects with a span containing the change are all removed (from change tracking only) up to (but excluding) the first SnippetInstance ancestor. 3.2) All SnippetInstance descendent of the first SnippetInstance are removed (from change tracking only) too

  4. If there the change is outside the root SnipetInstance, Ultisnips Tears down (All snippets are ended)

  5. Tabstop navigation follows a DFS, parent first then children, order in the alive snippets on the tabstop number, with tabstop 0 last.

...And optionally :

  1. If the cursor is outside a tabstop, pressing the forward or backward jump select the current tab and pressing again does the actual jump.

  2. When UltiSnips select a tabstop, an user option permits to un select if the cursor is moved

First, even if this algorithm is not adopted, adopting a reference algorithm seems key to me to write pertinent test cases. Some part could also be toggle by options to match more with the current behavior (especially the possibility to kill the snippet instance if the cursor moves outside of it) Then this algorithm in particular has several benefits :

  1. for non recursive snippet expansion, it folllows exactly the current behavior (this means that 80% of snippet working today are unaffected).

  2. It keeps alive the snippets longer, allowing one to scroll and go back without the snippet killed (as long as no changes are made elsewhere)

  3. The code to implement it is much simpler than the one currently used (I already made some work that do that except the snippet cancellation)

  4. With the upcoming store feature, it allows much more powerful logic, for example, automatically adding function arguments based on nested template etc.

  5. Less bug-prone. This algorithm is described in 7 points, that is 10 lines, which is in my (subjective I admit) opinion much simpler to describe than what is currently done

hl037 avatar Nov 06 '21 12:11 hl037

I fully agree with your reasoning. I do think your current outlined algo is close enough to intuition and to what is happening currently that adopting it would not disrupt too many users use case. Go for it, I'd say!

SirVer avatar Nov 07 '21 20:11 SirVer