Merge pull request #16341 from zeertzjq/vim-8.2.2518nightly

vim-patch:8.2.{2518,2520,3572,3588}: 'listchars' (and 'fillchars'?) fixes

5 files changed, 225 insertions, 42 deletions

diff --git a/src/nvim/option.c b/src/nvim/option.c
index 44b7d98e88..45e2032b35 100644
--- a/src/nvim/option.c
+++ b/src/nvim/option.c
@@ -2650,24 +2650,38 @@ 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);
+        // 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);
     }
-    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'
+  } 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);
+        // 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);
     }
-    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/src/nvim/testdir/test_listchars.vim b/src/nvim/testdir/test_listchars.vim
index 751be8eff5..9906b00222 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
 
@@ -305,7 +305,7 @@ func Test_listchars_invalid()
   enew!
   set ff=unix
 
-  set listchars=eol:$
+  set listchars=eol:$  " Accommodate Nvim default
   set list
   set ambiwidth=double
 
@@ -369,3 +369,138 @@ 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('$')))
+
+  " 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('$')))
+
+  " 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
+
+" 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/src/nvim/window.c b/src/nvim/window.c
index 4731c2cd41..e328ff5467 100644
--- a/src/nvim/window.c
+++ b/src/nvim/window.c
@@ -4756,6 +4756,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);
diff --git a/test/functional/options/chars_spec.lua b/test/functional/options/chars_spec.lua
index 5439ca3dba..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:<ff>') -- 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)
 
@@ -122,7 +138,7 @@ describe("'listchars'", function()
                                                         |
     ]])
   end)
-  it('has value local to window', function()
+  it('has window-local value', function()
     feed('i<tab><tab><tab><esc>')
     command('set list laststatus=0')
     command('setl listchars=tab:<->')
@@ -136,4 +152,18 @@ describe("'listchars'", function()
                                                         |
     ]])
   end)
+  it('using :set clears window-local value', function()
+    feed('i<tab><tab><tab><esc>')
+    command('set list laststatus=0')
+    command('setl listchars=tab:<->')
+    command('vsplit')
+    command('set listchars=tab:>-,eol:$')
+    screen:expect([[
+      >------->-------^>-------$│<------><------><------>|
+      ~                        │~                       |
+      ~                        │~                       |
+      ~                        │~                       |
+                                                        |
+    ]])
+  end)
 end)