From 8c24e1462c567c30410d3a45e552ecffc95bbd1d Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Wed, 17 Nov 2021 07:07:15 +0800 Subject: vim-patch:8.2.2518: 'listchars' should be window-local Problem: 'listchars' should be window-local. Solution: Make 'listchars' global-local. (Yegappan Lakshmanan, Marco Hinz, closes vim/vim#5206, closes vim/vim#7850) https://github.com/vim/vim/commit/eed9d46293f0842aad0d50ff3a526f9a48b12421 Nvim already has this feature, but it implements :set listchars the same as :setglobal listchars, which is incorrect. Vim's implementation of :set listchars is correct: using :set listchars clears local value. --- src/nvim/option.c | 11 +++- src/nvim/testdir/test_listchars.vim | 115 ++++++++++++++++++++++++++++++++- src/nvim/testdir/test_listlbr.vim | 2 + test/functional/options/chars_spec.lua | 16 ++++- 4 files changed, 137 insertions(+), 7 deletions(-) diff --git a/src/nvim/option.c b/src/nvim/option.c index 5dd7d708f9..d301b17265 100644 --- a/src/nvim/option.c +++ b/src/nvim/option.c @@ -2650,14 +2650,19 @@ ambw_end: } s = skip_to_option_part(s); } - } else if (varp == &p_lcs) { // 'listchars' + } else if (varp == &p_lcs) { // global 'listchars' errmsg = set_chars_option(curwin, varp, false); - if (!errmsg) { + if (errmsg == NULL) { + // The current window is set to use the global 'listchars' value. + // So clear the window-local value. + if (!(opt_flags & OPT_GLOBAL)) { + clear_string_option(&curwin->w_p_lcs); + } FOR_ALL_TAB_WINDOWS(tp, wp) { set_chars_option(wp, &wp->w_p_lcs, true); } + redraw_all_later(NOT_VALID); } - redraw_all_later(NOT_VALID); } else if (varp == &curwin->w_p_lcs) { // local 'listchars' errmsg = set_chars_option(curwin, varp, true); } else if (varp == &p_fcs) { // 'fillchars' diff --git a/src/nvim/testdir/test_listchars.vim b/src/nvim/testdir/test_listchars.vim index 8a1393a45d..f57d4f9ab9 100644 --- a/src/nvim/testdir/test_listchars.vim +++ b/src/nvim/testdir/test_listchars.vim @@ -112,7 +112,7 @@ func Test_listchars() " Test lead and trail normal ggdG - set listchars=eol:$ + set listchars=eol:$ " Accommodate Nvim default set listchars+=lead:>,trail:<,space:x set list @@ -142,7 +142,7 @@ func Test_listchars() " Test multispace normal ggdG - set listchars=eol:$ + set listchars=eol:$ " Accommodate Nvim default set listchars+=multispace:yYzZ set list @@ -301,7 +301,7 @@ func Test_listchars_invalid() enew! set ff=unix - set listchars=eol:$ + set listchars=eol:$ " Accommodate Nvim default set list set ambiwidth=double @@ -365,3 +365,112 @@ func Test_listchars_composing() enew! set listchars& ff& endfunction + +" Check for the value of the 'listchars' option +func s:CheckListCharsValue(expected) + call assert_equal(a:expected, &listchars) + call assert_equal(a:expected, getwinvar(0, '&listchars')) +endfunc + +" Test for using a window local value for 'listchars' +func Test_listchars_window_local() + %bw! + set list listchars& + let l:default_listchars = &listchars " Accommodate Nvim default + new + " set a local value for 'listchars' + setlocal listchars=tab:+-,eol:# + call s:CheckListCharsValue('tab:+-,eol:#') + " When local value is reset, global value should be used + setlocal listchars= + call s:CheckListCharsValue(l:default_listchars) " Accommodate Nvim default + " Use 'setlocal <' to copy global value + setlocal listchars=space:.,extends:> + setlocal listchars< + call s:CheckListCharsValue(l:default_listchars) " Accommodate Nvim default + " Use 'set <' to copy global value + setlocal listchars=space:.,extends:> + set listchars< + call s:CheckListCharsValue(l:default_listchars) " Accommodate Nvim default + " Changing global setting should not change the local setting + setlocal listchars=space:.,extends:> + setglobal listchars=tab:+-,eol:# + call s:CheckListCharsValue('space:.,extends:>') + " when split opening a new window, local value should be copied + split + call s:CheckListCharsValue('space:.,extends:>') + " clearing local value in one window should not change the other window + set listchars& + call s:CheckListCharsValue(l:default_listchars) " Accommodate Nvim default + close + call s:CheckListCharsValue('space:.,extends:>') + + " use different values for 'listchars' items in two different windows + call setline(1, ["\t one two "]) + setlocal listchars=tab:<->,lead:_,space:.,trail:@,eol:# + split + setlocal listchars=tab:[.],lead:#,space:_,trail:.,eol:& + split + set listchars=tab:+-+,lead:^,space:>,trail:<,eol:% + call assert_equal(['+------+^^one>>two<<%'], ScreenLines(1, virtcol('$'))) + close + call assert_equal(['[......]##one__two..&'], ScreenLines(1, virtcol('$'))) + close + call assert_equal(['<------>__one..two@@#'], ScreenLines(1, virtcol('$'))) + " changing the global setting should not change the local value + setglobal listchars=tab:[.],lead:#,space:_,trail:.,eol:& + call assert_equal(['<------>__one..two@@#'], ScreenLines(1, virtcol('$'))) + set listchars< + call assert_equal(['[......]##one__two..&'], ScreenLines(1, virtcol('$'))) + + " Using setglobal in a window with local setting should not affect the + " window. But should impact other windows using the global setting. + enew! | only + call setline(1, ["\t one two "]) + set listchars=tab:[.],lead:#,space:_,trail:.,eol:& + split + setlocal listchars=tab:+-+,lead:^,space:>,trail:<,eol:% + split + setlocal listchars=tab:<->,lead:_,space:.,trail:@,eol:# + setglobal listchars=tab:{.},lead:-,space:=,trail:#,eol:$ + call assert_equal(['<------>__one..two@@#'], ScreenLines(1, virtcol('$'))) + close + call assert_equal(['+------+^^one>>two<<%'], ScreenLines(1, virtcol('$'))) + close + call assert_equal(['{......}--one==two##$'], ScreenLines(1, virtcol('$'))) + + " Setting the global setting to the default value should not impact a window + " using a local setting + split + setlocal listchars=tab:<->,lead:_,space:.,trail:@,eol:# + setglobal listchars=eol:$ " Accommodate Nvim default + call assert_equal(['<------>__one..two@@#'], ScreenLines(1, virtcol('$'))) + close + call assert_equal(['^I one two $'], ScreenLines(1, virtcol('$'))) + + " Setting the local setting to the default value should not impact a window + " using a global setting + set listchars=tab:{.},lead:-,space:=,trail:#,eol:$ + split + setlocal listchars=tab:<->,lead:_,space:.,trail:@,eol:# + call assert_equal(['<------>__one..two@@#'], ScreenLines(1, virtcol('$'))) + setlocal listchars=eol:$ " Accommodate Nvim default + call assert_equal(['^I one two $'], ScreenLines(1, virtcol('$'))) + close + call assert_equal(['{......}--one==two##$'], ScreenLines(1, virtcol('$'))) + + " Using set in a window with a local setting should change it to use the + " global setting and also impact other windows using the global setting + split + setlocal listchars=tab:<->,lead:_,space:.,trail:@,eol:# + call assert_equal(['<------>__one..two@@#'], ScreenLines(1, virtcol('$'))) + set listchars=tab:+-+,lead:^,space:>,trail:<,eol:% + call assert_equal(['+------+^^one>>two<<%'], ScreenLines(1, virtcol('$'))) + close + call assert_equal(['+------+^^one>>two<<%'], ScreenLines(1, virtcol('$'))) + + %bw! + set list& listchars& +endfunc + +" vim: shiftwidth=2 sts=2 expandtab diff --git a/src/nvim/testdir/test_listlbr.vim b/src/nvim/testdir/test_listlbr.vim index e0518de3c2..2fda12d8b4 100644 --- a/src/nvim/testdir/test_listlbr.vim +++ b/src/nvim/testdir/test_listlbr.vim @@ -43,6 +43,7 @@ endfunc func Test_linebreak_with_list() throw 'skipped: Nvim does not support enc=latin1' + set listchars= call s:test_windows('setl ts=4 sbr=+ list listchars=') call setline(1, "\tabcdef hijklmn\tpqrstuvwxyz_1060ABCDEFGHIJKLMNOP ") let lines = s:screen_lines([1, 4], winwidth(0)) @@ -54,6 +55,7 @@ func Test_linebreak_with_list() \ ] call s:compare_lines(expect, lines) call s:close_windows() + set listchars&vim endfunc func Test_linebreak_with_nolist() diff --git a/test/functional/options/chars_spec.lua b/test/functional/options/chars_spec.lua index 5439ca3dba..aa54b8a04a 100644 --- a/test/functional/options/chars_spec.lua +++ b/test/functional/options/chars_spec.lua @@ -122,7 +122,7 @@ describe("'listchars'", function() | ]]) end) - it('has value local to window', function() + it('has window-local value', function() feed('i') command('set list laststatus=0') command('setl listchars=tab:<->') @@ -136,4 +136,18 @@ describe("'listchars'", function() | ]]) end) + it('using :set clears window-local value', function() + feed('i') + command('set list laststatus=0') + command('setl listchars=tab:<->') + command('vsplit') + command('set listchars=tab:>-,eol:$') + screen:expect([[ + >------->-------^>-------$│<------><------><------>| + ~ │~ | + ~ │~ | + ~ │~ | + | + ]]) + end) end) -- cgit From 7528bcec42bddfb644561adde42b9ea764faffda Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Wed, 17 Nov 2021 07:07:15 +0800 Subject: fix(options): using :set fillchars should clear local value --- src/nvim/option.c | 11 +++-- test/functional/options/chars_spec.lua | 76 ++++++++++++++++++++-------------- 2 files changed, 54 insertions(+), 33 deletions(-) diff --git a/src/nvim/option.c b/src/nvim/option.c index d301b17265..ec5d4919ba 100644 --- a/src/nvim/option.c +++ b/src/nvim/option.c @@ -2665,14 +2665,19 @@ ambw_end: } } else if (varp == &curwin->w_p_lcs) { // local 'listchars' errmsg = set_chars_option(curwin, varp, true); - } else if (varp == &p_fcs) { // 'fillchars' + } else if (varp == &p_fcs) { // global 'fillchars' errmsg = set_chars_option(curwin, varp, false); - if (!errmsg) { + if (errmsg == NULL) { + // The current window is set to use the global 'fillchars' value. + // So clear the window-local value. + if (!(opt_flags & OPT_GLOBAL)) { + clear_string_option(&curwin->w_p_fcs); + } FOR_ALL_TAB_WINDOWS(tp, wp) { set_chars_option(wp, &wp->w_p_fcs, true); } + redraw_all_later(NOT_VALID); } - redraw_all_later(NOT_VALID); } else if (varp == &curwin->w_p_fcs) { // local 'fillchars' errmsg = set_chars_option(curwin, varp, true); } else if (varp == &p_cedit) { // 'cedit' diff --git a/test/functional/options/chars_spec.lua b/test/functional/options/chars_spec.lua index aa54b8a04a..a082204980 100644 --- a/test/functional/options/chars_spec.lua +++ b/test/functional/options/chars_spec.lua @@ -67,36 +67,52 @@ describe("'fillchars'", function() shouldfail('eob:xy') -- two ascii chars shouldfail('eob:\255', 'eob:') -- invalid UTF-8 end) - it('has global value', function() - screen:try_resize(50, 5) - insert("foo\nbar") - command('set laststatus=0') - command('1,2fold') - command('vsplit') - command('set fillchars=fold:x') - screen:expect([[ - ^+-- 2 lines: fooxxxxxxxx│+-- 2 lines: fooxxxxxxx| - ~ │~ | - ~ │~ | - ~ │~ | - | - ]]) - end) - it('has local window value', function() - screen:try_resize(50, 5) - insert("foo\nbar") - command('set laststatus=0') - command('1,2fold') - command('vsplit') - command('setl fillchars=fold:x') - screen:expect([[ - ^+-- 2 lines: fooxxxxxxxx│+-- 2 lines: foo·······| - ~ │~ | - ~ │~ | - ~ │~ | - | - ]]) - end) + end) + it('has global value', function() + screen:try_resize(50, 5) + insert("foo\nbar") + command('set laststatus=0') + command('1,2fold') + command('vsplit') + command('set fillchars=fold:x') + screen:expect([[ + ^+-- 2 lines: fooxxxxxxxx│+-- 2 lines: fooxxxxxxx| + ~ │~ | + ~ │~ | + ~ │~ | + | + ]]) + end) + it('has window-local value', function() + screen:try_resize(50, 5) + insert("foo\nbar") + command('set laststatus=0') + command('1,2fold') + command('vsplit') + command('setl fillchars=fold:x') + screen:expect([[ + ^+-- 2 lines: fooxxxxxxxx│+-- 2 lines: foo·······| + ~ │~ | + ~ │~ | + ~ │~ | + | + ]]) + end) + it('using :set clears window-local value', function() + screen:try_resize(50, 5) + insert("foo\nbar") + command('set laststatus=0') + command('setl fillchars=fold:x') + command('1,2fold') + command('vsplit') + command('set fillchars&') + screen:expect([[ + ^+-- 2 lines: foo········│+-- 2 lines: fooxxxxxxx| + ~ │~ | + ~ │~ | + ~ │~ | + | + ]]) end) end) -- cgit From 5ed2a5cf9cf6a5f90e2b5bf3ef26ffea02f923f1 Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Wed, 17 Nov 2021 07:07:15 +0800 Subject: vim-patch:8.2.2520: missing tests for 'listchars' Problem: Missing tests for 'listchars'. Solution: Add a few more checks. (Yegappan Lakshmanan, closes vim/vim#7854) https://github.com/vim/vim/commit/04ea7e9049706788179945e2a91922c0b7cb9ed0 --- src/nvim/testdir/test_listchars.vim | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/nvim/testdir/test_listchars.vim b/src/nvim/testdir/test_listchars.vim index f57d4f9ab9..d55237c202 100644 --- a/src/nvim/testdir/test_listchars.vim +++ b/src/nvim/testdir/test_listchars.vim @@ -469,6 +469,26 @@ func Test_listchars_window_local() close call assert_equal(['+------+^^one>>two<<%'], ScreenLines(1, virtcol('$'))) + " Setting invalid value for a local setting should not impact the local and + " global settings + split + setlocal listchars=tab:<->,lead:_,space:.,trail:@,eol:# + let cmd = 'setlocal listchars=tab:{.},lead:-,space:=,trail:#,eol:$,x' + call assert_fails(cmd, 'E474:') + call assert_equal(['<------>__one..two@@#'], ScreenLines(1, virtcol('$'))) + close + call assert_equal(['+------+^^one>>two<<%'], ScreenLines(1, virtcol('$'))) + + " Setting invalid value for a global setting should not impact the local and + " global settings + split + setlocal listchars=tab:<->,lead:_,space:.,trail:@,eol:# + let cmd = 'setglobal listchars=tab:{.},lead:-,space:=,trail:#,eol:$,x' + call assert_fails(cmd, 'E474:') + call assert_equal(['<------>__one..two@@#'], ScreenLines(1, virtcol('$'))) + close + call assert_equal(['+------+^^one>>two<<%'], ScreenLines(1, virtcol('$'))) + %bw! set list& listchars& endfunc -- cgit From 8dbe47a4bc823e05afed8048e548a5ff9cc1819f Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Wed, 17 Nov 2021 07:07:15 +0800 Subject: vim-patch:8.2.3572: memory leak when closing window and using "multispace" Problem: Memory leak when closing window and using "multispace" in 'listchars'. Solution: Free the memory. (closes vim/vim#9071) https://github.com/vim/vim/commit/7a33ebfc5b04353aa7674972087d581def8fdcc1 --- src/nvim/testdir/test_listchars.vim | 16 +++++++++++----- src/nvim/window.c | 2 ++ 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/nvim/testdir/test_listchars.vim b/src/nvim/testdir/test_listchars.vim index d55237c202..7596b34433 100644 --- a/src/nvim/testdir/test_listchars.vim +++ b/src/nvim/testdir/test_listchars.vim @@ -440,7 +440,7 @@ func Test_listchars_window_local() call assert_equal(['{......}--one==two##$'], ScreenLines(1, virtcol('$'))) " Setting the global setting to the default value should not impact a window - " using a local setting + " using a local setting. split setlocal listchars=tab:<->,lead:_,space:.,trail:@,eol:# setglobal listchars=eol:$ " Accommodate Nvim default @@ -449,7 +449,7 @@ func Test_listchars_window_local() call assert_equal(['^I one two $'], ScreenLines(1, virtcol('$'))) " Setting the local setting to the default value should not impact a window - " using a global setting + " using a global setting. set listchars=tab:{.},lead:-,space:=,trail:#,eol:$ split setlocal listchars=tab:<->,lead:_,space:.,trail:@,eol:# @@ -460,7 +460,7 @@ func Test_listchars_window_local() call assert_equal(['{......}--one==two##$'], ScreenLines(1, virtcol('$'))) " Using set in a window with a local setting should change it to use the - " global setting and also impact other windows using the global setting + " global setting and also impact other windows using the global setting. split setlocal listchars=tab:<->,lead:_,space:.,trail:@,eol:# call assert_equal(['<------>__one..two@@#'], ScreenLines(1, virtcol('$'))) @@ -470,7 +470,7 @@ func Test_listchars_window_local() call assert_equal(['+------+^^one>>two<<%'], ScreenLines(1, virtcol('$'))) " Setting invalid value for a local setting should not impact the local and - " global settings + " global settings. split setlocal listchars=tab:<->,lead:_,space:.,trail:@,eol:# let cmd = 'setlocal listchars=tab:{.},lead:-,space:=,trail:#,eol:$,x' @@ -480,7 +480,7 @@ func Test_listchars_window_local() call assert_equal(['+------+^^one>>two<<%'], ScreenLines(1, virtcol('$'))) " Setting invalid value for a global setting should not impact the local and - " global settings + " global settings. split setlocal listchars=tab:<->,lead:_,space:.,trail:@,eol:# let cmd = 'setglobal listchars=tab:{.},lead:-,space:=,trail:#,eol:$,x' @@ -489,6 +489,12 @@ func Test_listchars_window_local() close call assert_equal(['+------+^^one>>two<<%'], ScreenLines(1, virtcol('$'))) + " Closing window with local lcs-multispace should not cause a memory leak. + setlocal listchars=multispace:---+ + split + call s:CheckListCharsValue('multispace:---+') + close + %bw! set list& listchars& endfunc diff --git a/src/nvim/window.c b/src/nvim/window.c index 59079584ca..26f483c842 100644 --- a/src/nvim/window.c +++ b/src/nvim/window.c @@ -4750,6 +4750,8 @@ static void win_free(win_T *wp, tabpage_T *tp) clear_winopt(&wp->w_onebuf_opt); clear_winopt(&wp->w_allbuf_opt); + xfree(wp->w_p_lcs_chars.multispace); + vars_clear(&wp->w_vars->dv_hashtab); // free all w: variables hash_init(&wp->w_vars->dv_hashtab); unref_var_dict(wp->w_vars); -- cgit From 145fc69df9f173717e3138c0a07a8de1a243519a Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Wed, 17 Nov 2021 07:07:15 +0800 Subject: vim-patch:8.2.3588: break statement is never reached Problem: Break statement is never reached. Solution: Rely on return value of set_chars_option() not changing. (closes vim/vim#9103) https://github.com/vim/vim/commit/606efc7df4c94104bbd24248106dd0e4ee6f7cfa --- src/nvim/option.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/nvim/option.c b/src/nvim/option.c index ec5d4919ba..278cf7c608 100644 --- a/src/nvim/option.c +++ b/src/nvim/option.c @@ -2659,7 +2659,9 @@ ambw_end: clear_string_option(&curwin->w_p_lcs); } FOR_ALL_TAB_WINDOWS(tp, wp) { - set_chars_option(wp, &wp->w_p_lcs, true); + // If no error was returned above, we don't expect an error + // here, so ignore the return value. + (void)set_chars_option(wp, &wp->w_p_lcs, true); } redraw_all_later(NOT_VALID); } @@ -2674,7 +2676,9 @@ ambw_end: clear_string_option(&curwin->w_p_fcs); } FOR_ALL_TAB_WINDOWS(tp, wp) { - set_chars_option(wp, &wp->w_p_fcs, true); + // If no error was returned above, we don't expect an error + // here, so ignore the return value. + (void)set_chars_option(wp, &wp->w_p_fcs, true); } redraw_all_later(NOT_VALID); } -- cgit