feat(:source, nvim_exec): anonymous script improvements #16071
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
seandewar
commented
Oct 18, 2021
justinmk
reviewed
Oct 18, 2021
seandewar
force-pushed
the
anon-sid2
branch
3 times, most recently
from
17b5c68
to
1795930
Compare
This comment was marked as outdated.
This comment was marked as outdated.
seandewar
force-pushed
the
anon-sid2
branch
2 times, most recently
from
f83ca29
to
2d1de09
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
seandewar
force-pushed
the
anon-sid2
branch
2 times, most recently
from
3c7c41e
to
7360116
Compare
seandewar
changed the title
seandewar
force-pushed
the
anon-sid2
branch
3 times, most recently
from
832c28b
to
201dc29
Compare
justinmk
added a commit
that referenced
this pull request
Mar 27, 2022
helpers.source() was a hack to work around the lack of anonymous :source. Its "create tempfile" behavior is not a required part of most tests that use it. Some tests still need the old "create tempfile" behavior either because they test SID behavior, or because of missing nvim_exec features: #16071
seandewar
force-pushed
the
anon-sid2
branch
2 times, most recently
from
2055059
to
43e2c1d
Compare
seandewar
force-pushed
the
anon-sid2
branch
3 times, most recently
from
4426f48
to
94aeb56
Compare
seandewar
force-pushed
the
anon-sid2
branch
2 times, most recently
from
659246e
to
7201cf4
Compare
dmitmel
pushed a commit
to dmitmel/neovim
that referenced
this pull request
Apr 16, 2022
helpers.source() was a hack to work around the lack of anonymous :source. Its "create tempfile" behavior is not a required part of most tests that use it. Some tests still need the old "create tempfile" behavior either because they test SID behavior, or because of missing nvim_exec features: neovim#16071
This reverts the change made in neovim#16064 (neovim@72652cb), as it's not needed. The reason why this fails when helpers.exec is used directly, but not when using helpers.source, is due to source calling write_file, which applies a second dedent to remove the indent before the heredoc END marker (required to make heredocs work). Though helpers.source is now deprecated, and uses exec under the hood, it still calls dedent to mimic the old behaviour, so we can still use it.
Followup to da9b0ab neovim#15994. `scriptitem_T` was lifted out of ex_cmds2.c but it's not needed, so keep it private. Also clean up some comments from neovim#15079 and rename a few functions. Add a test for ensuring different anon execs in the same Lua file still have unique SIDs with tracing enabled.
Always assign unique SID for anonymous sources, but defer allocation of script items and vars until script var access. Because all anonymous scripts always have SIDs now, <SID> and s: scope (also defined in functions, autocmds, commands, etc.) will work. Because a SID may not yet (or ever) map to an item, use a hash map for script_items and ga_scripts (now script_vars). Add file_sids; a sorted growarray of SIDs associated with a script file (used for :scriptnames, profiling, do_source, etc.). Replace SCRIPT macros with script_item and script_sv functions. Remove SCRIPT_VARS as it isn't very useful now. Use PRIdSCID to format scid_T in places that were missing it. This also fixes an issue since neovim#15079 that resulted in duplicate file name entries in :scriptnames output; test for this. Adjust more tests to use anonymous :source. Note that as prior s: or <SID> access is no longer needed for scripts to be given a SID, <SNR>s in tests that already use anonymous :sources such as echo_spec.lua need to be adjusted. Also note that we can potentially support adding profiling support for anonymous scripts now. :)
This removes the need for script_ensure_anon_item and applies to all scripts. Also no longer alloc a scriptitem_T for anon sources; they're not needed for anything (at least not right now).
Associate a unique SID for each anonymously :sourced Vim script buffer. This means sourcing the same buffer will retain the same script-local variables. Use anonymous :source for memory_usage_spec.lua's "function captures lvars" test. Use a buffer :source, as the test requires the same SID to be used. Remove "s:defined_func" logic; it isn't needed, and removing it matches Vim.
Unfortunately, even after our changes that allow for <SID> to be always usable in anonymous Vim scripts, the changes in neovim#15079 break this for Lua scripts due to the use of SID_LUA over a unique SID in places such as nvim_exec. Instead, use the special SID offsets to identify scripts from Lua contexts in which nlua_set_sctx can be used. Encoding it this way avoids the need to store such information in an allocated script item or elsewhere. However, this approach produces ugly script IDs, so it's only used when :verbose logging for Lua is enabled. As a result, it's now even more strongly recommended to start nvim as `nvim -V1` rather than to set verbose=1 in the middle of a session, otherwise tracing for previously sourced Lua scripts before verbose was set will not work. Remove the previous LastSet sctx dancing logic when accessing "s:" scope. It's no longer needed as anonymous Lua scripts now get a unique SID. Add some tests that would previously fail due to bugs with the old verbose Lua approach. Also, remove find_sid (unused leftover from the autocmd API PR) and change sid_T's type to a fixed-width type (int64_t).
Support showing line numbers for Vim script sourced anonymously via :source (no
args) and nvim_exec. For ranged buffer :sources, offset the reported number by
the line number of the range's start.
Don't offset the line number shown from anonymous scripts with the line number
of an enclosing script. For example, this could produce confusing and incorrect
numbers in situations where we have escaped NLs:
```vim
echo "hi"
call nvim_exec("\n\n\nset cul", 1)
verbose set cul?
```
Would print "line 6" (preferably "line 5"), but it's actually on line 2 of the
enclosing script...
As :verbose will only report the context of nvim_exec's anonymous script here,
just show the effective line number inside of it: "last set from anonymous
:source line 4".
Also, fix the order of nlua_set_sctx in set_option_sctx_idx; the current order
will erroneously offset the line number from the Lua script... Added a test.
|
@seandewar Do you plan to continue this PR? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Needs a big rebase; will get around to it
Summary of changes:
<SID>works always; no priors:access required. Every anonymous Vim script has a unique SID.nvim_exec, which may be used many times)s:is used in a script.s:defined in nested contexts such as commands defined in anonymous scripts may get lost.:sourceing the same buffer (like Vim, but it always allocates a named script item:source buffer=<buffer id>; we could emulate this if we want, but I imagine it's done as they don't support anonymous scripts).nvim_execoverhelpers.sourcein more tests. (possible after the bug fixes)Things to explore in the future:
nvim_execthat returns the SID of the anonymous script used. Can take anoptsdict allowing it to run in the context of a given SID.