[vim/vim] runtime(vimscript): add simple vimscript complete function (PR #17871)
Maxim Kim
I have come up with a simple omnicomplete function for vimscript to complete different things (commands, functions, global/buffer variables, expressions, highlight command).
It works nicely with the set autocomplete. While it is not super smart, it might be better than nothing and as usual there is a room for future improvements.
To enable set omnifunc=vimcomplete#Complete:
@chrisbra, @girishji, @lifepillar, @yegappan could you please check if it works for you? And in general if it makes sense to put it into vim's core.
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/17871
Commit Summary
- e194253 add simple vimscript complete function
File Changes
(1 file)
- A runtime/autoload/vimcomplete.vim (122)
Patch Links:
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Maxim Kim
@habamax pushed 1 commit.
- 162e02e better `set option option=value,value2` complete
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
girish
girishji left a comment (vim/vim#17871)You may be missing some entries from 'cmdline'. Take a look at this: https://github.com/girishji/vimcomplete/blob/main/tools/vimdictgen.vim
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Maxim Kim
habamax left a comment (vim/vim#17871)You may be missing some entries from
'cmdline'. Take a look at this: https://github.com/girishji/vimcomplete/blob/main/tools/vimdictgen.vim
Thx, will check it.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Shougo
Shougo left a comment (vim/vim#17871)I think getcompletiontype() is better to parse Vim script.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Maxim Kim
habamax left a comment (vim/vim#17871)I think
getcompletiontype()is better to parse Vim script.
I tried it but it can't do &option as in echo getcompletiontype("&option") is a command which is not what I want to complete. So I went full regex parsing instead.
Maybe it could be used in some parts of the code, let me see.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Lifepillar
lifepillar left a comment (vim/vim#17871)Is this meant to replace (and augment) Vim command completion (CTRL-X CTRL-V)? If so, I'd recommend comparing the respective outputs in as many contexts as possible.
Regardless, these are a couple of examples where things need probably to be improved:
Vim completion:
Normal-vim-cmd.png (view on web) cterm-vim-cmd.png (view on web)Omni-completion (this PR):
Normal-omni.png (view on web) cterm-omni.png (view on web)Omni-completion seems to order matches such that the closest match is not always at the top.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Maxim Kim
habamax left a comment (vim/vim#17871)@lifepillar thx for the feedback -- I didn't properly test it without set autocomlete and missed the part of where I had to filter 'augmented' values as well.
The difference with CTRL-X CTRL-V is that omnicomplete function works with set autocomplete provided you have added setlocal complete+=o to your filetype. And of course the fact that we can improve completion way easier using vimscript instead of getting into C source code.
It can't complete &option or expression/function and probably some more.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Phạm Bình An
brianhuster left a comment (vim/vim#17871)The difference with CTRL-X CTRL-V is that omnicomplete function works with set autocomplete provided you have added setlocal complete+=o to your filetype. And of course the fact that we can improve completion way easier using vimscript instead of getting into C source code.
So do you plan to make Ctrl-x Ctrl-v just call this omnifunc?
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Maxim Kim
habamax left a comment (vim/vim#17871)So do you plan to make
Ctrl-x Ctrl-vjust call this omnifunc?
I don't think this is easily possible tbh, but getcompletion with getcompletiontype does the job for me (with a bit of additional regex handling)
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Maxim Kim
habamax left a comment (vim/vim#17871)should be good to check it again
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
girish
girishji left a comment (vim/vim#17871)Does (Ctrl-X Ctrl-V) provide more relevant completions than this script?
Also, when I tried this on the Vim source (with a ~5 MB tag file), I noticed some slight hiccups—maybe around 0.5 seconds—which felt a bit annoying.
Is it doing a regex search on the tag file every time?
Maybe you could cache the entire tag file into a Vim dictionary and query that instead?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maxim Kim
Phạm Bình An
brianhuster left a comment (vim/vim#17871)oh did you mean to use omnifunc when user presses ctrl-x ctrl-v? No I don't plan it.
Basically I mean to just reimplement Ctrl-x_Ctrl-v in Vim9script (the command could still be defined in C, but its logic is in Vim9script). I can't see a reason why there are 2 versions (one in C, one in Vim9script) of very similar feature.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Maxim Kim
habamax left a comment (vim/vim#17871)oh did you mean to use omnifunc when user presses ctrl-x ctrl-v? No I don't plan it.
Basically I mean to just reimplement
Ctrl-x_Ctrl-vin Vim9script (the command could still be defined in C, but its logic is in Vim9script). That avoids unnecessarily duplicating code and logic.
I see, however it is not related to this PR, it needs another one.
Feature omnifunc exists quite for a while and I don't see a reason why we shouldn't have the one for vimscript.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Shougo
Shougo left a comment (vim/vim#17871)I tried it but it can't do &option as in echo getcompletiontype("&option") is a command which is not what I want to complete. So I went full regex parsing instead.
Yes. But you can use the pattern. getcompletiontype("echo &op") works.
You can use the full command line to parse.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Maxim Kim
habamax left a comment (vim/vim#17871)Yes. But you can use the pattern. getcompletiontype("echo &op") works.
You can use the full command line to parse.
I use exactly this plus additional regex parsing to be able to complete vim9script &com... as in &commentstring = '// %s'
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
dkearns
@dkearns commented on this pull request.
> @@ -116,6 +116,9 @@ setlocal include=\\v^\\s*import\\s*(autoload)? " set 'define' to recognize export commands setlocal define=\\v^\\s*export\\s*(def\|const\|var\|final) +" set omnifunc completion +setlocal omnifunc=vimcomplete#Complete
I suppose this needs some sort of vim9script guard so as not to annoy the Neovimmers.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maxim Kim
@habamax commented on this pull request.
> @@ -116,6 +116,9 @@ setlocal include=\\v^\\s*import\\s*(autoload)? " set 'define' to recognize export commands setlocal define=\\v^\\s*export\\s*(def\|const\|var\|final) +" set omnifunc completion +setlocal omnifunc=vimcomplete#Complete
Can’t they just not get this in the first place?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
dkearns
@dkearns commented on this pull request.
> @@ -116,6 +116,9 @@ setlocal include=\\v^\\s*import\\s*(autoload)? " set 'define' to recognize export commands setlocal define=\\v^\\s*export\\s*(def\|const\|var\|final) +" set omnifunc completion +setlocal omnifunc=vimcomplete#Complete
My understanding is that we are trying to limit the differences in shared runtime files.
$ git grep git grep -r 'has(.vim9script'
runtime/autoload/dist/vim.vim:if !has('vim9script')
runtime/autoload/htmlfold.vim:if !has("vim9script")
runtime/autoload/javaformat.vim:if !has("vim9script")
runtime/doc/version9.txt:Solution: Add has('vim9script').
runtime/doc/vim9.txt: if !has('vim9script')
runtime/ftplugin/html.vim:if !has("vim9script")
runtime/ftplugin/java.vim:if !has("vim9script")
runtime/ftplugin/lua.vim:if !has('vim9script')
runtime/indent/racket.vim:if has('vim9script')
runtime/indent/racket.vim:let b:undo_indent = "setlocal lisp< ai< si< lw<" .. (has('vim9script') ? ' indentexpr< lispoptions<' : '')
runtime/syntax/java.vim:if !has("vim9script")
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maxim Kim
@habamax commented on this pull request.
> @@ -116,6 +116,9 @@ setlocal include=\\v^\\s*import\\s*(autoload)? " set 'define' to recognize export commands setlocal define=\\v^\\s*export\\s*(def\|const\|var\|final) +" set omnifunc completion +setlocal omnifunc=vimcomplete#Complete
I disagree with that, I believe we should target vim in vim project.
But, well...
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maxim Kim
@habamax commented on this pull request.
> @@ -116,6 +116,9 @@ setlocal include=\\v^\\s*import\\s*(autoload)? " set 'define' to recognize export commands setlocal define=\\v^\\s*export\\s*(def\|const\|var\|final) +" set omnifunc completion +setlocal omnifunc=vimcomplete#Complete
And we don’t have the vim9script guard here https://github.com/vim/vim/blob/6a6a44ffb5ad7350a4d313a5faeae17e50bf37a2/runtime/ftplugin/c.vim#L32
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
@chrisbra commented on this pull request.
> @@ -116,6 +116,9 @@ setlocal include=\\v^\\s*import\\s*(autoload)? " set 'define' to recognize export commands setlocal define=\\v^\\s*export\\s*(def\|const\|var\|final) +" set omnifunc completion +setlocal omnifunc=vimcomplete#Complete
Then let's fix it there as well :) (I'll do it when merging this). Thanks all
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Maxim Kim
habamax left a comment (vim/vim#17871)thanks!
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
dkearns
@dkearns commented on this pull request.
> @@ -116,6 +116,9 @@ setlocal include=\\v^\\s*import\\s*(autoload)? " set 'define' to recognize export commands setlocal define=\\v^\\s*export\\s*(def\|const\|var\|final) +" set omnifunc completion +setlocal omnifunc=vimcomplete#Complete
The completion function for C was added a long time ago, before the change in policy. It was actually rewritten so the Neovim folks just kept the legacy script version.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maxim Kim
@habamax commented on this pull request.
> @@ -116,6 +116,9 @@ setlocal include=\\v^\\s*import\\s*(autoload)? " set 'define' to recognize export commands setlocal define=\\v^\\s*export\\s*(def\|const\|var\|final) +" set omnifunc completion +setlocal omnifunc=vimcomplete#Complete
before the change in policy
It would be nice to see the policy stated in the README or anywhere in the repo.
For me this is a surprise that we are to think about neovim compatibility. That is ok here, as it doesn't cost us anything, just a simple guard. But in general this is not very promising.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
@chrisbra commented on this pull request.
> @@ -116,6 +116,9 @@ setlocal include=\\v^\\s*import\\s*(autoload)? " set 'define' to recognize export commands setlocal define=\\v^\\s*export\\s*(def\|const\|var\|final) +" set omnifunc completion +setlocal omnifunc=vimcomplete#Complete
So how about this then:
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8f0947acf..9e187f3d1 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -48,7 +48,7 @@ approvers are happy with that particular change. # Reporting issues -We use GitHub issues, but that is not a requirement. Writing to the Vim +We use GitHub [issues][16], but that is not a requirement. Writing to the Vim mailing list is also fine. Please use the GitHub issues only for actual issues. If you are not 100% sure @@ -72,7 +72,8 @@ Or open [the todo file][todo list] on GitHub to see the latest version. The latest version of these files can be obtained from the repository. They are usually not updated with numbered patches. However, they may -or may not work with older Vim releases (since they may contain new features). +or may not work with older Vim releases (since they may depend on new +features). If you find a problem with one of these files or have a suggestion for improvement, please first try to contact the maintainer directly. @@ -82,7 +83,18 @@ upstream repository. You may also check the [MAINTAINERS][11] file. The maintainer will take care of issues and send updates to the Vim project for distribution with Vim. -If the maintainer does not respond, contact the [vim-dev][0] mailing list. +If the maintainer does not respond, contact the [vim-dev][0] mailing list or +open an [issue][16] here. + +Note: Whether or not to use Vim9 Script is up to the maintainer. For runtime +files maintained here, we aim to preserve compatibility with Neovim if +possible. Please add wrap Vim9 script with a guard like this: +```vim +if has('vim9script') + " use Vim9 script implementation + [...] +endif +``` ## Contributing new runtime files @@ -95,6 +107,9 @@ PR with your changes against this repository here. For new filetypes, do not for - add yourself as Maintainer to the top of file (again, keep the header similar to other runtime files) - add yourself to the [MAINTAINERS][11] file. +- add a guard `if has('vim9script')` if you like to maintain Neovim + compatibility but want to use Vim9 script (or restrict yourself to legacy Vim + script) # Translations @@ -144,3 +159,4 @@ mailing list. For other questions please use the [Vi Stack Exchange][8] website, [14]: https://github.com/vim/vim/blob/master/runtime/doc/syntax.txt [15]: https://en.wikipedia.org/wiki/Developer_Certificate_of_Origin [16]: https://github.com/vim/vim/blob/master/runtime/doc/helphelp.txt +[16]: https://github.com/vim/vim/issues
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
bfrg
@bfrg commented on this pull request.
> @@ -116,6 +116,9 @@ setlocal include=\\v^\\s*import\\s*(autoload)? " set 'define' to recognize export commands setlocal define=\\v^\\s*export\\s*(def\|const\|var\|final) +" set omnifunc completion +setlocal omnifunc=vimcomplete#Complete
vim9script can be disabled when compiling Vim from source. Therefore, the check has('vim9script') is necessary, even in Vim itself. Though, not sure if runtime would work when vim9script is disabled since a few functions in runtime have been ported to vim9script.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
zeertzjq
@zeertzjq commented on this pull request.
> @@ -116,6 +116,9 @@ setlocal include=\\v^\\s*import\\s*(autoload)? " set 'define' to recognize export commands setlocal define=\\v^\\s*export\\s*(def\|const\|var\|final) +" set omnifunc completion +setlocal omnifunc=vimcomplete#Complete
vim9scriptcan be disabled when compiling Vim from source.
No, there is no feature flag for vim9script.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
bfrg
@bfrg commented on this pull request.
> @@ -116,6 +116,9 @@ setlocal include=\\v^\\s*import\\s*(autoload)? " set 'define' to recognize export commands setlocal define=\\v^\\s*export\\s*(def\|const\|var\|final) +" set omnifunc completion +setlocal omnifunc=vimcomplete#Complete
Then why is it listed in :version?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
zeertzjq
@zeertzjq commented on this pull request.
> @@ -116,6 +116,9 @@ setlocal include=\\v^\\s*import\\s*(autoload)? " set 'define' to recognize export commands setlocal define=\\v^\\s*export\\s*(def\|const\|var\|final) +" set omnifunc completion +setlocal omnifunc=vimcomplete#Complete
Look at version.c line 630. It always shows +vim9script (even if FEAT_EVAL is disabled).
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Shane-XB-Qian
Shane-XB-Qian left a comment (vim/vim#17871)// from last time: "let g:vimsyn_vim_features = ['nvim']" ?
// it is weird, not sure Debian Whether setup something for Ubuntu, or firefox setup something for chrome :lol:
--
shane.xb.qian
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
dkearns
@dkearns commented on this pull request.
> @@ -116,6 +116,9 @@ setlocal include=\\v^\\s*import\\s*(autoload)? " set 'define' to recognize export commands setlocal define=\\v^\\s*export\\s*(def\|const\|var\|final) +" set omnifunc completion +setlocal omnifunc=vimcomplete#Complete
I don't really understand where the lines are but it's clearly not that simple.
[doug] vim/git/neovim (git)-[master] % git grep -r 'has(.vim9script' runtime
runtime/autoload/dist/vim.vim:if !has('vim9script')
runtime/autoload/htmlfold.vim:if !has("vim9script")
runtime/autoload/javaformat.vim:if !has("vim9script")
runtime/ftplugin/html.vim:if !has("vim9script")
runtime/ftplugin/java.vim:if !has("vim9script")
runtime/indent/racket.vim:if has('vim9script')
runtime/indent/racket.vim:let b:undo_indent = "setlocal lisp< ai< si< lw<" .. (has('vim9script') ? ' indentexpr< lispoptions<' : '')
runtime/syntax/java.vim:if !has("vim9script")
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
@chrisbra commented on this pull request.
> @@ -116,6 +116,9 @@ setlocal include=\\v^\\s*import\\s*(autoload)? " set 'define' to recognize export commands setlocal define=\\v^\\s*export\\s*(def\|const\|var\|final) +" set omnifunc completion +setlocal omnifunc=vimcomplete#Complete
Well, I don't know if such a change to the vim ftplugin would be marked as "Vim9script", however this is done and then
it also does not hurt, plus it helps when a user with Vim8 simply downloads the update plugin files from here (not sure if this is a valid use case).
In any case, that is the best I can do to maintain compatibility with Neovim.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
zeertzjq
@zeertzjq commented on this pull request.
> @@ -116,6 +116,9 @@ setlocal include=\\v^\\s*import\\s*(autoload)? " set 'define' to recognize export commands setlocal define=\\v^\\s*export\\s*(def\|const\|var\|final) +" set omnifunc completion +setlocal omnifunc=vimcomplete#Complete
Hmm, then mappings added in c0b3c19 should also be guarded with has('vim9script').
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Phạm Bình An
@brianhuster commented on this pull request.
> @@ -116,6 +116,9 @@ setlocal include=\\v^\\s*import\\s*(autoload)? " set 'define' to recognize export commands setlocal define=\\v^\\s*export\\s*(def\|const\|var\|final) +" set omnifunc completion +setlocal omnifunc=vimcomplete#Complete
I don't really understand where the lines are but it's clearly not that simple
Except for dist/vim.vim, other files seem to maintained by some third parties, they also have upstream Github repo. Nvim users could download the plugin from upstream, so it makes sense to have vim9script guard.
But I just say that is not necessary for plugins maintained by Vim directly and don't have another upstream repo.
it helps when a user with Vim8 simply downloads the update plugin files from here (not sure if this is a valid use case)
It's up to you guys to decide, but I hope not, otherwise you guys will have to add a bunch of version check for very old Vim version (what if user of Vim5 install it?), which make the code uglier and harder to maintain
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.