[vim/vim] feat: make 'preinsert' work with 'autocomplete' (PR #18213)
girish
This change extends Insert mode autocompletion so that "preinsert" behavior of 'completeopt' also
works when 'autocomplete' is enabled.
Try: :set autocomplete completeopt=preinsert. It's quite useful.
See :help 'cot' for more details.
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/18213
Commit Summary
- 5edc60c feat: make 'preinsert' work with 'autocomplete'
- 673bfef Fix test failure
- 40ab5b7 Merge branch 'vim:master' into preinsert
File Changes
(7 files)
- M runtime/doc/options.txt (37)
- M src/edit.c (9)
- M src/highlight.c (1)
- M src/insexpand.c (207)
- M src/proto/insexpand.pro (4)
- M src/testdir/test_cmdline.vim (1)
- M src/testdir/test_ins_complete.vim (107)
Patch Links:
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
girish
@girishji pushed 1 commit.
- 1d95c30 Fix build failure
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Maxim Kim
habamax left a comment (vim/vim#18213)Looks like hl-ComplMatchIns is not defined in vim/colorschemes. Is it a new one or I completely overlooked its existence?
—
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#18213)It was already there. I just noticed it was undefined, so I linked it to the Added highlight group.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#18213)Thanks, but this seems to break a few screen-dump tests
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
girish
@girishji pushed 2 commits.
- b3b918f Revert hl-ComplMatchIns changes, fix test failure
- 5df7dde Merge branch 'preinsert' of https://github.com/girishji/vim into preinsert
You are receiving this because you are subscribed to this thread.
girish
girishji left a comment (vim/vim#18213)Thanks, but this seems to break a few screen-dump tests
Thanks, just noticed this. The failures are caused by the highlight group ComplMatchIns, which I had linked incorrectly to another group. That was a mistake. ComplMatchIns isn’t limited to preinsert -- it also highlights the postfix part of the text when selecting from the popup menu.
I am not sure if it would make sense to have a separate highlight group from preinsert. For now, I reverted the changes so as not to hold up this PR.
@habamax I think hl-ComplMatchIns can’t really be used in any colorscheme, because it highlights every match you pick from the menu. That feels quite unusual.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
girish
girishji left a comment (vim/vim#18213)Added a new highlight group hl-PreInsert. Using hl-ComplMatchIns for both purposes prevented setting distinct default colors for pre-inserted text. Currently applies only to 'autocomplete'.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#18213)thanks
—
Reply to this email directly, view it on GitHub.
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.
girish
girishji left a comment (vim/vim#18213)Thanks
—
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#18213)@lifepillar, there is new PreInsert hl, you may want to add it to coloremplate.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Justin M. Keyes
justinmk left a comment (vim/vim#18213)Added a new highlight group
hl-PreInsert. Usinghl-ComplMatchIns
Why is the new hl group named PreInsert, instead of something like ComplPreIns ? Can Vim please pick a convention and stick with it?
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#18213)It should be ComplMatchPreIns no? We can rename it
—
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#18213)I wasn’t aware there was a convention. I chose what felt the most intuitive and readable. When I first saw hl-ComplMatchIns, it looked like a jumble of letters I couldn’t immediately connect to anything. If a convention is already established, I don’t mind following it, but it feels odd to replace a meaningful tag with something that looks cryptic.
—
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 left a comment (vim/vim#18213)There seems to be no convention. I personally like PreInsert more.
—
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#18213)@chrisbra do you go to neovim github questioning what neovim does?
The message could have been a tiny bit more polite.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#18213)The message could have been a tiny bit more polite.
agree.
if a convention is already established, I don’t mind following it, but it feels odd to replace a meaningful tag with something that looks cryptic.
Well, we only have a single highlight group for completion: ComplMatchIns so it seems we have a precedent for other related highlighting groups. So if there is a chance we will have more additional related highlighting groups related to completion feature, I think this would make sense to have a consistent naming.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
girish
girishji left a comment (vim/vim#18213)The doc is incorrect:
ComplMatchIns Matched text of the currently inserted completion.
This highlight group applies to the unmatched portion of the completion (the suffix). Consider renaming it to something like InsertedSuffix, CompletionSuffix, PostfixText, etc. Current name conveys the opposite meaning of what it is doing.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#18213)So this comes from patch 6a38aff (v9.1.0936). We could consider renaming it. But I think this is too late already
—
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#18213)According to the document you have recently authored, it is not too late :)
FWIW, it is not in any of built-in colorschemes.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#18213)yep I know. in that case what would be a good naming convention? Something that would also apply for PreInsert?
—
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#18213)in that case what would be a good naming convention? Something that would also apply for PreInsert?
Should we have a prefix for the group of 2 (as of now) highlight groups?
- CompletePreInsert, CoPreInsert, CtePreInsert, ...
- CompleteMatchInsert, CoMatchInsert, CteMatchInsert, ... or CompleteSuffix, CompleteMatchSuffix, CoMatchSuffix, CteMatchSuffix
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Shane-XB-Qian
Shane-XB-Qian left a comment (vim/vim#18213)>
> FWIW, it is not in any of built-in colorschemes.
as usually in the past it was allowed maximal 1 week, minimal 1 day, i may think the 'policy' should keep that way.
now this 'CompMatchIns' had been over many weeks, i think better be keep it is now;
as for 'preinsert', how about 'ComplPreIns' or 'ComplMatchPreIns'?
--
shane.xb.qian
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.