[vim/vim] Add `complete_mode()` (#3866)

151 views
Skip to first unread message

Shougo

unread,
Jan 26, 2019, 6:35:04 AM1/26/19
to vim/vim, Subscribed

I have added complete_mode() API.
It returns current ctrl_x mode as string.

It is useful for auto completion plugin.


You can view, comment on, or merge this pull request online at:

  https://github.com/vim/vim/pull/3866

Commit Summary

  • Add complete_mode()
  • Fix tests

File Changes

Patch Links:


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub

Codecov

unread,
Jan 26, 2019, 6:54:00 AM1/26/19
to vim/vim, Subscribed

Codecov Report

Merging #3866 into master will increase coverage by 0.02%.
The diff coverage is 73.91%.

Impacted file tree graph

@@            Coverage Diff             @@

##           master    #3866      +/-   ##

==========================================

+ Coverage   78.73%   78.75%   +0.02%     

==========================================

  Files         103      103              

  Lines      141914   141952      +38     

==========================================

+ Hits       111730   111791      +61     

+ Misses      30184    30161      -23
Impacted Files Coverage Δ
src/evalfunc.c 87.83% <0%> (ø) ⬆️
src/edit.c 85.52% <85%> (+0.11%) ⬆️
src/ui.c 48.11% <0%> (-0.08%) ⬇️
src/terminal.c 75.2% <0%> (-0.05%) ⬇️
src/ex_docmd.c 80.53% <0%> (+0.01%) ⬆️
src/ex_cmds2.c 84.98% <0%> (+0.09%) ⬆️
src/window.c 83.49% <0%> (+0.12%) ⬆️
src/sign.c 92.65% <0%> (+0.13%) ⬆️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cb574f4...789b231. Read the comment docs.

Dominique Pellé

unread,
Jan 26, 2019, 6:56:55 AM1/26/19
to vim/vim, Subscribed

@dpelle commented on this pull request.


In src/edit.c:

> @@ -3003,6 +3003,63 @@ set_completion(colnr_T startcol, list_T *list)

     out_flush();

 }

 

+// complete_mode() implementation.

+char *ins_compl_mode(void)

+{

+    char *mode = NULL;

+    switch (ctrl_x_mode)

+    {

+	case 0:

+	    // Keyword completion

+	    mode = strdup("keyword");

Vim does not use strdup() which is not portable. It uses vim_strsave() instead.


In src/testdir/test_popup.vim:

> +        \ ["\<C-X>\<C-D>", 'path_defines'],

+        \ ["\<C-X>\<C-I>", 'path_patterns'],

+        \ ["\<C-X>\<C-K>", 'dictionary'],

+        \ ["\<C-X>\<C-T>", 'thesaurus'],

+        \ ["\<C-X>\<C-V>", 'cmdline'],

+        \ ["\<C-X>\<C-U>", 'function'],

+        \ ["\<C-X>\<C-O>", 'omni'],

+        \ ["\<C-X>s", 'spell'],

+        \]

+    call feedkeys("i" . key . "\<f5>\<ESC>", 'tx')

+    call assert_equal(expected, getline('.'))

+    %d

+  endfor

+  call delete('dummy.txt')

+

+  function! s:complTestEval() abort

Vim tests no longer uses the exclamation mark in "function!" but uses "func" … "endfunc".


In src/testdir/test_popup.vim:

> +        \ ["\<C-X>\<C-V>", 'cmdline'],

+        \ ["\<C-X>\<C-U>", 'function'],

+        \ ["\<C-X>\<C-O>", 'omni'],

+        \ ["\<C-X>s", 'spell'],

+        \]

+    call feedkeys("i" . key . "\<f5>\<ESC>", 'tx')

+    call assert_equal(expected, getline('.'))

+    %d

+  endfor

+  call delete('dummy.txt')

+

+  function! s:complTestEval() abort

+    call complete(1, ['source', 'soundfold'])

+    return ''

+  endfunction

+  inoremap <F6>  <c-r>=s:complTestEval()<CR>

The test has a side-effect: it leaks this map. We should do iunmap before exiting the test.


In src/testdir/test_popup.vim:

> @@ -896,4 +896,48 @@ func Test_menu_only_exists_in_terminal()

   endtry

 endfunc

 

+func Test_popup_complete_mode()

+  new

+  inoremap <f5> <c-r>=complete_mode()<cr>

The test has a side-effect: it leaks this map. We should do iunmap before exiting the test.


In src/testdir/test_popup.vim:

> @@ -896,4 +896,48 @@ func Test_menu_only_exists_in_terminal()

   endtry

 endfunc

 

+func Test_popup_complete_mode()

+  new

+  inoremap <f5> <c-r>=complete_mode()<cr>

+  call writefile([

+        \ '!rm	src/testdir/amiga.vim	/^cmap !rm !Delete all$/;"	m',

+        \], 'dummy.txt')

In general, temporary files created by Vim tests start with X presumably because it makes it easier to spot temporary files which were not removed.


In runtime/doc/eval.txt:

> @@ -3522,6 +3523,29 @@ complete_check()				*complete_check()*

 		Only to be used by the function specified with the

 		'completefunc' option.

 

+							*complete_mode()*

+complete_mode()

+		Return a string that indicates the current completion mode.

+		Note: It does not check popup menu is visible.

It does not check whether popup menu is visible


You are receiving this because you are subscribed to this thread.

Reply to this email directly, view it on GitHub, or mute the thread.

Shougo

unread,
Jan 26, 2019, 7:00:34 AM1/26/19
to vim/vim, Push

@Shougo pushed 1 commit.


You are receiving this because you are subscribed to this thread.

View it on GitHub

Shougo

unread,
Jan 26, 2019, 7:03:21 AM1/26/19
to vim/vim, Push

@Shougo pushed 1 commit.


You are receiving this because you are subscribed to this thread.

View it on GitHub

Shougo

unread,
Jan 26, 2019, 7:05:53 AM1/26/19
to vim/vim, Subscribed

I have fixed for reviews.

mattn

unread,
Jan 26, 2019, 7:25:32 AM1/26/19
to vim/vim, Subscribed

Totally, char_u* should be used instead of char* ?

Shougo

unread,
Jan 26, 2019, 7:31:50 AM1/26/19
to vim/vim, Push

@Shougo pushed 1 commit.


You are receiving this because you are subscribed to this thread.

View it on GitHub

Shougo

unread,
Jan 26, 2019, 7:32:04 AM1/26/19
to vim/vim, Subscribed

OK. I have changed.

mattn

unread,
Jan 26, 2019, 7:56:00 AM1/26/19
to vim/vim, Subscribed

@mattn commented on this pull request.


In src/testdir/test_popup.vim:

> @@ -896,4 +896,48 @@ func Test_menu_only_exists_in_terminal()
   endtry
 endfunc
 
+func Test_popup_complete_mode()
+  new
+  inoremap <f5> <c-r>=complete_mode()<cr>

Or put <buffer>.

Shougo

unread,
Jan 26, 2019, 8:09:29 AM1/26/19
to vim/vim, Subscribed

@Shougo commented on this pull request.


In src/testdir/test_popup.vim:

> @@ -896,4 +896,48 @@ func Test_menu_only_exists_in_terminal()
   endtry
 endfunc
 
+func Test_popup_complete_mode()
+  new
+  inoremap <f5> <c-r>=complete_mode()<cr>

Oh, it seems better.

Shougo

unread,
Jan 26, 2019, 8:10:36 AM1/26/19
to vim/vim, Push

@Shougo pushed 1 commit.


You are receiving this because you are subscribed to this thread.

View it on GitHub

Bram Moolenaar

unread,
Jan 26, 2019, 9:13:33 AM1/26/19
to vim/vim, Subscribed

> I have added `complete_mode()` API.
> It returns current ctrl_x mode as string.
>
> It is useful for auto completion plugin.

I wonder if other completion state would be useful. I rather have one
function to get the whole state, than a bunch of functions for several
pieces of state.

Unfortunately, the completion state is very complex, it's not easy to
even explain what each state variable means. I've been meaning to
refactor this, but it's a complex thing.


About the implementation: Instead of using a big switch, it would be
much more compact to use a table with the completion types and string.
Something like:

struct {
int mode;
char *name;
} mode_table = {
{0, "keyword"},
{CTRL_X_FILES, "files"},
};


+// complete_mode() implementation.
+char_u *ins_compl_mode(void)
+{

This should use Vim style function header.


--
'I generally avoid temptation unless I can't resist it."
-- Mae West

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ an exciting new programming language -- http://www.Zimbu.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

Shougo

unread,
Jan 27, 2019, 5:52:06 AM1/27/19
to vim/vim, Push

@Shougo pushed 1 commit.

  • 83281f7 Use table instead of big switch


You are receiving this because you are subscribed to this thread.

View it on GitHub

Shougo

unread,
Jan 27, 2019, 5:52:29 AM1/27/19
to vim/vim, Subscribed

Thank you for the review.
I have changed the implementation to use table.

Bram Moolenaar

unread,
Jan 27, 2019, 9:01:10 AM1/27/19
to vim/vim, Subscribed

> Thank you for the review.
> I have changed the implementation to use table.

It's a bit simpler this way:

for (i = 0; i < sizeof(table)/ sizeof(struct mode_table); i++)
if (table[i].mode == ctrl_x_mode)
return (char_u *)table[i].name;

return (char_u *)"unknown";

Please use Vim code style.

--
MORTICIAN: What?
CUSTOMER: Nothing -- here's your nine pence.
DEAD PERSON: I'm not dead!
MORTICIAN: Here -- he says he's not dead!
CUSTOMER: Yes, he is.
DEAD PERSON: I'm not!
The Quest for the Holy Grail (Monty Python)


/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ an exciting new programming language -- http://www.Zimbu.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

h_east

unread,
Jan 27, 2019, 10:24:13 AM1/27/19
to vim_dev
Hi,

I think that we would check `compl_started` except `ctrl_x_mode == CTRL_X_NOT_DEFINED_YET`.
Then we can return "" when not in complementary mode.

Patch attached.

--
Best regards,
Hirohito Higashi (h_east)

complete_mode.patch

Shougo

unread,
Jan 27, 2019, 7:10:24 PM1/27/19
to vim/vim, Push

@Shougo pushed 1 commit.


You are receiving this because you are subscribed to this thread.

View it on GitHub

Shougo

unread,
Jan 27, 2019, 7:10:42 PM1/27/19
to vim/vim, Subscribed

OK. I have fixed it.

Shougo

unread,
Jan 27, 2019, 7:11:23 PM1/27/19
to vim/vim, Push

@Shougo pushed 1 commit.


You are receiving this because you are subscribed to this thread.

View it on GitHub

h_east

unread,
Jan 27, 2019, 7:20:38 PM1/27/19
to vim/vim, Subscribed

Shougo

unread,
Jan 28, 2019, 5:57:01 PM1/28/19
to vim/vim, Push

@Shougo pushed 1 commit.


You are receiving this because you are subscribed to this thread.

View it on GitHub

Shougo

unread,
Jan 28, 2019, 5:57:38 PM1/28/19
to vim/vim, Subscribed

Oh, I have not read your response.

I have included (and some changed) your patch.

mattn

unread,
Jan 28, 2019, 7:27:40 PM1/28/19
to vim/vim, Subscribed

@mattn commented on this pull request.


In src/edit.c:

>  
-    return (char_u *)"unknown";
+    return (char_u *)mode;
 }
  if (ctrl_x_mode == CTRL_X_NOT_DEFINED_YET || compl_started)
        return mode_names[ctrl_x_mode & ~CTRL_X_WANT_IDENT];
  return (char_u *)"";

h_east

unread,
Jan 28, 2019, 7:34:12 PM1/28/19
to vim/vim, Subscribed

It is better to overwrite by my patch as is.
Check the contents of my patch more.

I add to the document, add test cases, simplify the test file (Xdummy.txt), add code comment, and move the definition position of the function (ins_compl_mode) to the appropriate place.

Or it may be better for Bram to include my patch :)

Shougo

unread,
Feb 2, 2019, 2:51:39 AM2/2/19
to vim/vim, Push

@Shougo pushed 2 commits.

  • ded3c78 Fix for mattn's review
  • 7828100 Include other @h_east's patch


You are receiving this because you are subscribed to this thread.

View it on GitHub

h_east

unread,
Feb 2, 2019, 6:36:23 AM2/2/19
to vim/vim, Subscribed

@h-east commented on this pull request.

I think your arrangement is subtle.
So, did you decide that this is unnecessary?

diff --git a/runtime/doc/insert.txt b/runtime/doc/insert.txt
index 0431043c2..2bb357ef4 100644
--- a/runtime/doc/insert.txt
+++ b/runtime/doc/insert.txt
@@ -642,6 +642,7 @@ and one of the CTRL-X commands.  You exit CTRL-X mode by typing a key that is
 not a valid CTRL-X mode command.  Valid keys are the CTRL-X command itself,
 CTRL-N (next), and CTRL-P (previous).
 
+To get the current completion mode, |complete_mode()| can be used.
 Also see the 'infercase' option if you want to adjust the case of the match.
 
 							*complete_CTRL-E*

In src/edit.c:

> @@ -3589,6 +3590,36 @@ ins_compl_active(void)
     return compl_started;
 }
 
+// Return Insert completion mode name string
/*
 * Return Insert completion mode name string
 */

Why do you get worse?
It has already been pointed out by Bram.
See the document.

 :h style-examples
/function_name

In runtime/doc/eval.txt:

> @@ -3522,6 +3523,30 @@ complete_check()				*complete_check()*
 		Only to be used by the function specified with the
 		'completefunc' option.
 
+							*complete_mode()*
+complete_mode()
+		Return a string that indicates the current completion mode.
+		Note: It does not check whether popup menu is visible.
+
+		   ""		Not in completion mode
+		   "keyword"	Keyword completion |i_CTRL-X_CTRL-N|
+		   "ctrl_x"	Just press |i_CTRL-X|
+		   "whole_line"	Whole lines |i_CTRL-X_CTRL-L|
+		   "files"	File names |i_CTRL-X_CTRL-F|
+		   "tags"	Tags |i_CTRL-X_CTRL-]|
+		   "path_defines"
+				Definition completion |i_CTRL-X_CTRL-D|

Why did you change it? The way it does not break is easier to see.


In runtime/doc/eval.txt:

> @@ -3522,6 +3523,30 @@ complete_check()				*complete_check()*
 		Only to be used by the function specified with the
 		'completefunc' option.
 
+							*complete_mode()*
+complete_mode()
+		Return a string that indicates the current completion mode.
+		Note: It does not check whether popup menu is visible.
+
+		   ""		Not in completion mode
+		   "keyword"	Keyword completion |i_CTRL-X_CTRL-N|
+		   "ctrl_x"	Just press |i_CTRL-X|
+		   "whole_line"	Whole lines |i_CTRL-X_CTRL-L|
+		   "files"	File names |i_CTRL-X_CTRL-F|
+		   "tags"	Tags |i_CTRL-X_CTRL-]|
+		   "path_defines"
+				Definition completion |i_CTRL-X_CTRL-D|
+		   "path_patterns"
+		   		Include completion |i_CTRL-X_CTRL-I|

Same above.


In src/proto/edit.pro:

> @@ -20,6 +20,7 @@ char_u *find_word_end(char_u *ptr);
 int ins_compl_active(void);
 int ins_compl_add_tv(typval_T *tv, int dir);
 void ins_compl_check_keys(int frequency, int in_compl_func);
+char_u *ins_compl_mode(void);

This if after int ins_compl_active(void);


In src/testdir/test_popup.vim:

> +func Test_popup_complete_mode()
+  new
+  inoremap <buffer><f5> <c-r>=complete_mode()<cr>
+  call writefile([
+        \ 'dummy	dummy.txt	1',
+        \], 'Xdummy.txt')
+  setlocal tags=Xdummy.txt
+  setlocal dictionary=Xdummy.txt
+  setlocal thesaurus=Xdummy.txt
+  setlocal omnifunc=syntaxcomplete#Complete
+  setlocal completefunc=syntaxcomplete#Complete
+  setlocal spell
+  for [key, expected] in [
+        \ ["", ''],
+        \ ["\<C-X>", 'ctrl_x'],
+        \ ["\<C-X>\<C-N>", 'keyword'],

Test case is missing.
\ ["\<C-X>\<C-P>", 'keyword'],


In src/edit.c:

> +	"tags",
+	"path_patterns",
+	"path_defines",
+	"unknown",	    // CTRL_X_FINISHED
+	"dictionary",
+	"thesaurus",
+	"cmdline",
+	"function",
+	"omni",
+	"spell",
+	"unknown",	    // CTRL_X_LOCAL_MSG only used in "ctrl_x_msgs"
+	"eval",
+    };
+
+    if (ctrl_x_mode == CTRL_X_NOT_DEFINED_YET || compl_started)
+	return mode_names[ctrl_x_mode & ~CTRL_X_WANT_IDENT];

Need a cast (char_u *).


You are receiving this because you are subscribed to this thread.

Shougo

unread,
Feb 3, 2019, 6:27:48 PM2/3/19
to vim/vim, Subscribed

@Shougo commented on this pull request.


In src/edit.c:

> @@ -3589,6 +3590,36 @@ ins_compl_active(void)
     return compl_started;
 }
 
+// Return Insert completion mode name string

It has already been pointed out by Bram.

Oh, I have not see it in the issue.
It is pointed in ML thread?

Shougo

unread,
Feb 3, 2019, 6:32:45 PM2/3/19
to vim/vim, Push

@Shougo pushed 1 commit.


You are receiving this because you are subscribed to this thread.

View it on GitHub

Shougo

unread,
Feb 3, 2019, 6:33:25 PM2/3/19
to vim/vim, Subscribed

@Shougo commented on this pull request.


In src/testdir/test_popup.vim:

> +func Test_popup_complete_mode()
+  new
+  inoremap <buffer><f5> <c-r>=complete_mode()<cr>
+  call writefile([
+        \ 'dummy	dummy.txt	1',
+        \], 'Xdummy.txt')
+  setlocal tags=Xdummy.txt
+  setlocal dictionary=Xdummy.txt
+  setlocal thesaurus=Xdummy.txt
+  setlocal omnifunc=syntaxcomplete#Complete
+  setlocal completefunc=syntaxcomplete#Complete
+  setlocal spell
+  for [key, expected] in [
+        \ ["", ''],
+        \ ["\<C-X>", 'ctrl_x'],
+        \ ["\<C-X>\<C-N>", 'keyword'],

I have added it.


You are receiving this because you are subscribed to this thread.

Prabir Shrestha

unread,
Mar 7, 2019, 7:31:22 PM3/7/19
to vim/vim, Subscribed

I would also be interested in this patch.

I do see Bram's comment on

I wonder if other completion state would be useful. I rather have one
function to get the whole state, than a bunch of functions for several
pieces of state.

One option would be to have complete_info() that returns the following. Feel free to suggest better names.

{
    "mode": "omni",
    "items": [],
    "selected": -1, // index of items
    "height": 10,
    "width": 20
}

For start we could only return {"mode": "omni" } and add others as needed. We can add docs that says to check of key before accessing it.

Shougo

unread,
Mar 10, 2019, 1:34:41 AM3/10/19
to vim/vim, Subscribed

I have renamed complete_mode() to complete_info().

Patch to add complete_mode(). Shougo - #3866. Alternate patch by Hirohito
Higashi, 2019 Jan 27, included now?

@brammool I have already included Higashi's patch.

h_east

unread,
Mar 10, 2019, 10:45:14 AM3/10/19
to vim/vim, Subscribed

@Shougo
Do NOT use my patch.
I'll Pull Request for a few days

--
Best regards,
Hirohito Higashi (h_east)

Prabir Shrestha

unread,
Mar 10, 2019, 2:30:34 PM3/10/19
to vim/vim, Subscribed

@prabirshrestha commented on this pull request.


In runtime/doc/eval.txt:

> @@ -2267,6 +2267,7 @@ col({expr})			Number	column nr of cursor or mark
 complete({startcol}, {matches}) none	set Insert mode completion
 complete_add({expr})		Number	add completion match
 complete_check()		Number	check for key typed during completion
+complete_info()			String	get current completion information

Type should be changed to dictionary


In runtime/doc/eval.txt:

> @@ -3536,6 +3537,36 @@ complete_check()				*complete_check()*
 		Only to be used by the function specified with the
 		'completefunc' option.
 
+							*complete_info()*
+complete_info()
+		Return a string that indicates the current completion

dictionary instead of string

Shougo

unread,
Mar 10, 2019, 7:22:19 PM3/10/19
to vim/vim, Push

@Shougo pushed 1 commit.


You are receiving this because you are subscribed to this thread.

View it on GitHub

mattn

unread,
Mar 10, 2019, 8:05:10 PM3/10/19
to vim/vim, Subscribed

I think this is enough to merge.


You are receiving this because you are subscribed to this thread.

Prabir Shrestha

unread,
Mar 12, 2019, 1:01:17 AM3/12/19
to vim/vim, Subscribed

Verified that this patch works for asyncomplete v2 to ignore processing autocomplete if it is open due to vim's builtin completions. :shipit:

Shougo

unread,
Mar 17, 2019, 10:27:03 PM3/17/19
to vim/vim, Subscribed

#4106 (comment)

I sent a patch that was almost completely rewritten on # 3866.
Despite the reviews I have repeatedly made, he kept subtle.
(and he has never said thanks-word)

OK. Thank you for your review.
I know your patch is better.

But you have said in other place "Your patch implementation is very bad. My patch should be included instead".
So I was stubborn sorry.

I will close the PR.

Note: He have blocked me in github. So I cannot comment the issue.

Shougo

unread,
Mar 17, 2019, 10:27:03 PM3/17/19
to vim/vim, Subscribed

Closed #3866.

Reply all
Reply to author
Forward
0 new messages