Mercurial > vim
changeset 34596:5a8340e044f4 v9.1.0190
patch 9.1.0190: complete_info() returns wrong order of items
Commit: https://github.com/vim/vim/commit/8950bf7f8b85c1287d4e696965d88091fcc60594
Author: Girish Palya <girishji@gmail.com>
Date: Wed Mar 20 20:07:29 2024 +0100
patch 9.1.0190: complete_info() returns wrong order of items
Problem: complete_info() returns wrong order of items
(after v9.0.2018)
Solution: Revert Patch v9.0.2018
(Girish Palya)
bug fix: complete_info() gives wrong results
1) complete_info() reverses list of items during <c-p>
2) 'selected' item index is wrong during <c-p>
3) number of items returnd can be wrong
Solution:
- Decouple 'cp_number' from 'selected' index since they need not be
correlated
- Do not iterate the list backwards
- Add targeted tests
Regression introduced by https://github.com/vim/vim/commit/69fb5afb3bc9da24c2fb0eafb0027ba9c6502fc2
Following are unnecessary commits to patch problems from above:
https://github.com/vim/vim/commit/fef66301665027f1801a18d796f74584666f41ef
https://github.com/vim/vim/commit/daef8c74375141974d61b85199b383017644978c
All the tests from above commits are retained though.
fixes: #14204
closes: #14241
Signed-off-by: Girish Palya <girishji@gmail.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
| author | Christian Brabandt <cb@256bit.org> |
|---|---|
| date | Wed, 20 Mar 2024 20:15:03 +0100 |
| parents | 6261385b0892 |
| children | fd1a4ec55650 |
| files | src/insexpand.c src/testdir/test_ins_complete.vim src/version.c |
| diffstat | 3 files changed, 111 insertions(+), 85 deletions(-) [+] |
line wrap: on
line diff
--- a/src/insexpand.c +++ b/src/insexpand.c @@ -3045,74 +3045,6 @@ ins_compl_update_sequence_numbers(void) } } - static int -info_add_completion_info(list_T *li) -{ - compl_T *match; - int forward = compl_dir_forward(); - - if (compl_first_match == NULL) - return OK; - - match = compl_first_match; - // There are four cases to consider here: - // 1) when just going forward through the menu, - // compl_first_match should point to the initial entry with - // number zero and CP_ORIGINAL_TEXT flag set - // 2) when just going backwards, - // compl-first_match should point to the last entry before - // the entry with the CP_ORIGINAL_TEXT flag set - // 3) when first going forwards and then backwards, e.g. - // pressing C-N, C-P, compl_first_match points to the - // last entry before the entry with the CP_ORIGINAL_TEXT - // flag set and next-entry moves opposite through the list - // compared to case 2, so pretend the direction is forward again - // 4) when first going backwards and then forwards, e.g. - // pressing C-P, C-N, compl_first_match points to the - // first entry with the CP_ORIGINAL_TEXT - // flag set and next-entry moves in opposite direction through the list - // compared to case 1, so pretend the direction is backwards again - // - // But only do this when the 'noselect' option is not active! - - if (!compl_no_select) - { - if (forward && !match_at_original_text(match)) - forward = FALSE; - else if (!forward && match_at_original_text(match)) - forward = TRUE; - } - - // Skip the element with the CP_ORIGINAL_TEXT flag at the beginning, in case of - // forward completion, or at the end, in case of backward completion. - match = forward || match->cp_prev == NULL ? match->cp_next : - (compl_no_select && match_at_original_text(match) ? match->cp_prev : match->cp_prev->cp_prev); - - while (match != NULL && !match_at_original_text(match)) - { - dict_T *di = dict_alloc(); - - if (di == NULL) - return FAIL; - if (list_append_dict(li, di) == FAIL) - return FAIL; - dict_add_string(di, "word", match->cp_str); - dict_add_string(di, "abbr", match->cp_text[CPT_ABBR]); - dict_add_string(di, "menu", match->cp_text[CPT_MENU]); - dict_add_string(di, "kind", match->cp_text[CPT_KIND]); - dict_add_string(di, "info", match->cp_text[CPT_INFO]); - if (match->cp_user_data.v_type == VAR_UNKNOWN) - // Add an empty string for backwards compatibility - dict_add_string(di, "user_data", (char_u *)""); - else - dict_add_tv(di, "user_data", &match->cp_user_data); - - match = forward ? match->cp_next : match->cp_prev; - } - - return OK; -} - /* * Get complete information */ @@ -3158,24 +3090,60 @@ get_complete_info(list_T *what_list, dic if (ret == OK && (what_flag & CI_WHAT_PUM_VISIBLE)) ret = dict_add_number(retdict, "pum_visible", pum_visible()); - if (ret == OK && (what_flag & CI_WHAT_ITEMS)) + if (ret == OK && (what_flag & CI_WHAT_ITEMS || what_flag & CI_WHAT_SELECTED)) { list_T *li; - - li = list_alloc(); - if (li == NULL) - return; - ret = dict_add_list(retdict, "items", li); - if (ret == OK) - ret = info_add_completion_info(li); - } - - if (ret == OK && (what_flag & CI_WHAT_SELECTED)) - { - if (compl_curr_match != NULL && compl_curr_match->cp_number == -1) - ins_compl_update_sequence_numbers(); - ret = dict_add_number(retdict, "selected", compl_curr_match != NULL - ? compl_curr_match->cp_number - 1 : -1); + dict_T *di; + compl_T *match; + int selected_idx = -1; + + if (what_flag & CI_WHAT_ITEMS) + { + li = list_alloc(); + if (li == NULL) + return; + ret = dict_add_list(retdict, "items", li); + } + if (ret == OK && what_flag & CI_WHAT_SELECTED) + if (compl_curr_match != NULL && compl_curr_match->cp_number == -1) + ins_compl_update_sequence_numbers(); + if (ret == OK && compl_first_match != NULL) + { + int list_idx = 0; + match = compl_first_match; + do + { + if (!match_at_original_text(match)) + { + if (what_flag & CI_WHAT_ITEMS) + { + di = dict_alloc(); + if (di == NULL) + return; + ret = list_append_dict(li, di); + if (ret != OK) + return; + dict_add_string(di, "word", match->cp_str); + dict_add_string(di, "abbr", match->cp_text[CPT_ABBR]); + dict_add_string(di, "menu", match->cp_text[CPT_MENU]); + dict_add_string(di, "kind", match->cp_text[CPT_KIND]); + dict_add_string(di, "info", match->cp_text[CPT_INFO]); + if (match->cp_user_data.v_type == VAR_UNKNOWN) + // Add an empty string for backwards compatibility + dict_add_string(di, "user_data", (char_u *)""); + else + dict_add_tv(di, "user_data", &match->cp_user_data); + } + if (compl_curr_match != NULL && compl_curr_match->cp_number == match->cp_number) + selected_idx = list_idx; + list_idx += 1; + } + match = match->cp_next; + } + while (match != NULL && !is_first_match(match)); + } + if (ret == OK && (what_flag & CI_WHAT_SELECTED)) + ret = dict_add_number(retdict, "selected", selected_idx); } if (ret == OK && (what_flag & CI_WHAT_INSERTED))
--- a/src/testdir/test_ins_complete.vim +++ b/src/testdir/test_ins_complete.vim @@ -412,6 +412,62 @@ func Test_completefunc_info() set completefunc& endfunc +func CompleteInfoUserDefinedFn(findstart, query) + " User defined function (i_CTRL-X_CTRL-U) + if a:findstart + return col('.') + endif + return [{'word': 'foo'}, {'word': 'bar'}, {'word': 'baz'}, {'word': 'qux'}] +endfunc + +func CompleteInfoTestUserDefinedFn(mvmt, idx, noselect) + new + if a:noselect + set completeopt=menuone,popup,noinsert,noselect + else + set completeopt=menu,preview + endif + set completefunc=CompleteInfoUserDefinedFn + call feedkeys("i\<C-X>\<C-U>" . a:mvmt . "\<C-R>\<C-R>=string(complete_info())\<CR>\<ESC>", "tx") + let completed = a:idx != -1 ? ['foo', 'bar', 'baz', 'qux']->get(a:idx) : '' + call assert_equal(completed. "{'pum_visible': 1, 'mode': 'function', 'selected': " . a:idx . ", 'items': [" . + \ "{'word': 'foo', 'menu': '', 'user_data': '', 'info': '', 'kind': '', 'abbr': ''}, " . + \ "{'word': 'bar', 'menu': '', 'user_data': '', 'info': '', 'kind': '', 'abbr': ''}, " . + \ "{'word': 'baz', 'menu': '', 'user_data': '', 'info': '', 'kind': '', 'abbr': ''}, " . + \ "{'word': 'qux', 'menu': '', 'user_data': '', 'info': '', 'kind': '', 'abbr': ''}" . + \ "]}", getline(1)) + bwipe! + set completeopt& + set completefunc& +endfunc + +func Test_complete_info_user_defined_fn() + " forward + call CompleteInfoTestUserDefinedFn("\<C-N>\<C-N>", 1, v:true) + call CompleteInfoTestUserDefinedFn("\<C-N>\<C-N>\<C-N>", 2, v:true) + call CompleteInfoTestUserDefinedFn("\<C-N>\<C-N>", 2, v:false) + call CompleteInfoTestUserDefinedFn("\<C-N>\<C-N>\<C-N>", 3, v:false) + call CompleteInfoTestUserDefinedFn("\<C-N>\<C-N>\<C-N>\<C-N>", -1, v:false) + " backward + call CompleteInfoTestUserDefinedFn("\<C-P>\<C-P>", 2, v:true) + call CompleteInfoTestUserDefinedFn("\<C-P>\<C-P>\<C-P>", 1, v:true) + call CompleteInfoTestUserDefinedFn("\<C-P>\<C-P>\<C-P>\<C-P>\<C-P>", -1, v:true) + call CompleteInfoTestUserDefinedFn("\<C-P>\<C-P>", 3, v:false) + call CompleteInfoTestUserDefinedFn("\<C-P>\<C-P>\<C-P>", 2, v:false) + " forward backward + call CompleteInfoTestUserDefinedFn("\<C-N>\<C-N>\<C-N>\<C-P>", 1, v:true) + call CompleteInfoTestUserDefinedFn("\<C-N>\<C-N>\<C-P>", 0, v:true) + call CompleteInfoTestUserDefinedFn("\<C-N>\<C-N>\<C-N>\<C-P>", 2, v:false) + call CompleteInfoTestUserDefinedFn("\<C-N>\<C-N>\<C-N>\<C-N>\<C-P>", 3, v:false) + call CompleteInfoTestUserDefinedFn("\<C-N>\<C-N>\<C-P>", 1, v:false) + " backward forward + call CompleteInfoTestUserDefinedFn("\<C-P>\<C-P>\<C-P>\<C-P>\<C-P>\<C-N>", 0, v:true) + call CompleteInfoTestUserDefinedFn("\<C-P>\<C-P>\<C-P>\<C-N>", 2, v:true) + call CompleteInfoTestUserDefinedFn("\<C-P>\<C-P>\<C-P>\<C-P>\<C-P>\<C-N>", 1, v:false) + call CompleteInfoTestUserDefinedFn("\<C-P>\<C-P>\<C-P>\<C-N>", 3, v:false) + call CompleteInfoTestUserDefinedFn("\<C-P>\<C-N>\<C-N>", 1, v:false) +endfunc + " Test that mouse scrolling/movement should not interrupt completion. func Test_mouse_scroll_move_during_completion() new @@ -2345,7 +2401,7 @@ func Test_complete_info_index() call assert_equal(-1, g:compl_info['selected']) call feedkeys("Go\<C-X>\<C-N>\<C-P>\<F5>\<Esc>_dd", 'tx') - call assert_equal(0, g:compl_info['selected']) + call assert_equal(5, g:compl_info['selected']) call assert_equal(6 , len(g:compl_info['items'])) call assert_equal("fff", g:compl_info['items'][g:compl_info['selected']]['word']) call feedkeys("Go\<C-X>\<C-N>\<C-N>\<C-N>\<C-N>\<C-N>\<C-N>\<C-N>\<C-N>\<C-N>\<F5>\<Esc>_dd", 'tx')