-
-
Notifications
You must be signed in to change notification settings - Fork 246
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow self-dependent dynamicNode #1137
base: master
Are you sure you want to change the base?
Conversation
326779d
to
595996e
Compare
edf1dd4
to
f513f60
Compare
339fb3f
to
e89d49d
Compare
A quick update: I've somwhat widened the scope of this PR, besides simply allowing self-dependent dynamicNodes, I'd like to make those more practical by addressing this usecase where a snippet that was active inside an insertNode remains active even after it was modified by the dynamicNode-function! A preliminary example: 1730232750.mp4This required some larger changes (and enabled some other nice things, for example a restoreNode also restoring entire snippets that were expanded inside the restoreNode), so I'm happy with any user that is open to test-running this branch in their usual setup for a while, even without using these new features :D |
eb85cac
to
15ebd05
Compare
In that case, there will be a `nil` at the place of the text of the node in the arg-list. They have to be explicitly marked as optional, since we currently have the behaviour of not updating nodes at all if an argnode is missing.
dynamicNode has to return .static_snip instead of .snip
Previously: InsertNodes trigger an update on input_leave. This works, but, if updates invalidate nodes that are currently involved in being jumped over, we'd have to abort and roll back the jump, or do some other recovery. To avoid this, update_dependents is done _before_ any jumping is performed (this is really more elegant now since we can keep track of which nodes are "above" some insertNode, and then just get all of them and their dependents (see node_util.find_node_dependents/collect_dependents)) So, now `ls.jump` does essentially: * store position of cursor relative to active node * collect nodes that depend on the text of this node (so, all dependents of all parents of this node) * update them * try to find an equivalent node (node with the same key, or if a restoreNode is present, the exact same node :D) and perform the jump from it * if an equivalent node could not be found, just enter the first dynamicNode (starting from the previously active node) and enter it Obviously, this only really works well if an equivalent of the current node can be found in the new generated nodes. Similarly, active_update_dependents
put_initial is not called on it, but since it's always visible we can set it when initializing.
This is important to prevent an infinite loop when a snippet is remove_from_jumplist'd in node_util.snippettree_find_undamaged_node: if child_snippets is not modified, we just continue to remove it (or call :r_f_j but immediately nop because the snippet is not visible).
And store generated snippet in .snip, not .snip_stored, which is not reached by subtree_do, which we would like to have apply to the stored snippet.
3a30512
to
79b985e
Compare
Previously, we used a metatable to refer to the choiceNode for some keys that are required, which are only .parent and .pos.
This may improve the accuracy of docstrings generated on an expanded snippet.
Otherwise the jump_into from change_choice may complete after the cursor-movement due to the subsequent update.
Much more appropriate, also get_current_choices will work even if some insertNode is not visible.
8ee6626
to
ae8d95c
Compare
350ba61
to
3d8f8bd
Compare
* handle column-shifted begin-position (only shift cursor-column if it stays in the same line) * correctly enqueue cursor-movement via feedkeys.
31d314e
to
aa4b71d
Compare
Code is essentially the same thing. Also allow passing the current cursor to ls.set/change_choice, which is useful for extras.select_choice, where the cursor-state is "destroyed" due to vim.input, and should be saved before that is even opened.
4af90c0
to
1464b3f
Compare
metadata can store eg. when a snippetString was created, marks are a bit like extmarks, they can mark a position in a snippetString and are shifted by text-insertions.
6df134e
to
a938528
Compare
Cache `static_text` (for insertNodes) and don't query it anew while luasnip is operating. This is valid under the assumption that all changes to the buffer are due to luasnip while an api-function (jump, expand, etc.) is running. This is enabled by session.luasnip_changedtick which is set as soon as an api-function is called, and prevents re-fetching snippetStrings from the buffer (which in turn allows us to set the cursor-position once, and then have it propagate through all updates that are triggered subsequently). This commit also replaces no_region_check_wrap with api_do, which is more general (handles both jump_active, which prevents recursive api-calls and luasnip_changedtick)
See extensive comment in commit.
By inserting the stored snip into the buffer, `get_args` during `update_restore` can find the argnode inside the snippet.
If we don't do this, the content of a choiceNode may not be restored correctly. (or, it will be restored correctly, but it won't be what the user saw before they called change/set/select_choice, which seems suboptimal).
ea82903
to
d066147
Compare
Deal with/Explicitly allow dynamicNodes being updated due to a change inside of them.
This is to enable stuff like #1096 and #1102, where the text of an insertNode should be updated while typing.
One important limitation this has (for now??) is that the updates have to reach some equilibrium (text is unchanged after a second update), otherwise we just keep updating.
One example:
Note the other new addition,
opt_arg
(optional argnodes, we currently don't run the dynamicNode-function if an argnode is missing), for allowing the initial creation of thesn
when the"asd"
-node does not yet exist.This is missing tests and doc for now, and I'm not certain it handles all cases correctly, but I'd like to show this since I think it should work for simple ones.