Merge #33542 shada improvements

* feat(shada): don't store jumplist if '0 in 'shada'
* fix(shada): don't store search and sub patterns if /0 in 'shada'
* fix(shada): don't store empty replacement string
* fix(shada): don't add '0' mark if f0 in 'shada'

runtime/doc/options.txt

@@ -5193,8 +5193,8 @@ A jump table for the options with a short description can be found at |Q_op|.
 	'	Maximum number of previously edited files for which the marks
 		are remembered.  This parameter must always be included when
 		'shada' is non-empty.
-		Including this item also means that the |jumplist| and the
+		If non-zero, then the |jumplist| and the |changelist| are also
-		|changelist| are stored in the shada file.
+		stored in the shada file.
 							*shada-/*
 	/	Maximum number of items in the search pattern history to be
 		saved.  If non-zero, then the previous search and substitute

runtime/lua/vim/_meta/options.lua

@@ -5450,8 +5450,8 @@ vim.go.ssop = vim.go.sessionoptions
 --- '	Maximum number of previously edited files for which the marks
 --- 	are remembered.  This parameter must always be included when
 --- 	'shada' is non-empty.
---- 	Including this item also means that the `jumplist` and the
+--- 	If non-zero, then the `jumplist` and the `changelist` are also
---- 	`changelist` are stored in the shada file.
+--- 	stored in the shada file.
 --- 						*shada-/*
 --- /	Maximum number of items in the search pattern history to be
 --- 	saved.  If non-zero, then the previous search and substitute

src/nvim/options.lua

@@ -7302,8 +7302,8 @@ local options = {
         '	Maximum number of previously edited files for which the marks
         	are remembered.  This parameter must always be included when
         	'shada' is non-empty.
-        	Including this item also means that the |jumplist| and the
+        	If non-zero, then the |jumplist| and the |changelist| are also
-        	|changelist| are stored in the shada file.
+        	stored in the shada file.
         						*shada-/*
         /	Maximum number of items in the search pattern history to be
         	saved.  If non-zero, then the previous search and substitute

src/nvim/shada.c

@@ -2396,38 +2396,42 @@ static ShaDaWriteResult shada_write(FileDescriptor *const sd_writer,
     } while (var_iter != NULL);
   }
 
-  // Initialize jump list
+  if (num_marked_files > 0) {  // Skip if '0 in 'shada'
-  wms->jumps_size = shada_init_jumps(wms->jumps, &removable_bufs);
+    // Initialize jump list
+    wms->jumps_size = shada_init_jumps(wms->jumps, &removable_bufs);
+  }
 
-  const bool search_highlighted = !(no_hlsearch
+  if (dump_one_history[HIST_SEARCH] > 0) {  // Skip if /0 in 'shada'
-                                    || find_shada_parameter('h') != NULL);
+    const bool search_highlighted = !(no_hlsearch
-  const bool search_last_used = search_was_last_used();
+                                      || find_shada_parameter('h') != NULL);
+    const bool search_last_used = search_was_last_used();
 
-  // Initialize search pattern
+    // Initialize search pattern
-  add_search_pattern(&wms->search_pattern, &get_search_pattern, false,
+    add_search_pattern(&wms->search_pattern, &get_search_pattern, false,
-                     search_last_used, search_highlighted);
+                       search_last_used, search_highlighted);
 
-  // Initialize substitute search pattern
+    // Initialize substitute search pattern
-  add_search_pattern(&wms->sub_search_pattern, &get_substitute_pattern, true,
+    add_search_pattern(&wms->sub_search_pattern, &get_substitute_pattern, true,
-                     search_last_used, search_highlighted);
+                       search_last_used, search_highlighted);
 
-  // Initialize substitute replacement string
+    // Initialize substitute replacement string
-  {
     SubReplacementString sub;
     sub_get_replacement(&sub);
-    wms->replacement = (PossiblyFreedShadaEntry) {
+    if (sub.sub != NULL) {  // Don't store empty replacement string
-      .can_free_entry = false,
+      wms->replacement = (PossiblyFreedShadaEntry) {
-      .data = {
+        .can_free_entry = false,
-        .type = kSDItemSubString,
-        .timestamp = sub.timestamp,
         .data = {
-          .sub_string = {
+          .type = kSDItemSubString,
-            .sub = sub.sub,
+          .timestamp = sub.timestamp,
-          }
+          .data = {
-        },
+            .sub_string = {
-        .additional_data = sub.additional_data,
+              .sub = sub.sub,
-      }
+            }
-    };
+          },
+          .additional_data = sub.additional_data,
+        }
+      };
+    }
   }
 
   // Initialize global marks
@@ -2555,9 +2559,9 @@ static ShaDaWriteResult shada_write(FileDescriptor *const sd_writer,
     }
   }
 
-  // Update numbered marks: '0' should be replaced with the current position,
+  // Update numbered marks: replace '0 mark with the current position,
-  // '9' should be removed and all other marks shifted.
+  // remove '9 and shift all other marks. Skip if f0 in 'shada'.
-  if (!ignore_buf(curbuf, &removable_bufs) && curwin->w_cursor.lnum != 0) {
+  if (dump_global_marks && !ignore_buf(curbuf, &removable_bufs) && curwin->w_cursor.lnum != 0) {
     replace_numbered_mark(wms, 0, (PossiblyFreedShadaEntry) {
       .can_free_entry = false,
       .data = {

test/functional/shada/history_spec.lua

@@ -6,6 +6,7 @@ local t_shada = require('test.functional.shada.testutil')
 local nvim_command, fn, api, nvim_feed, eq = n.command, n.fn, n.api, n.feed, t.eq
 local assert_alive = n.assert_alive
 local expect_exit = n.expect_exit
+local pcall_err = t.pcall_err
 
 local reset, clear = t_shada.reset, t_shada.clear
 
@@ -104,6 +105,16 @@ describe('ShaDa support code', function()
     eq('c', fn.histget('>', -1))
   end)
 
+  it('does not dump last search pattern if /0 in shada', function()
+    nvim_command('set shada+=/0')
+    nvim_command('silent! /test/')
+    expect_exit(nvim_command, 'qall!')
+    reset()
+    nvim_command('language C')
+    local err = pcall_err(n.exec_capture, 'normal! n')
+    eq('nvim_exec2(), line 1: Vim(normal):E35: No previous regular expression', err)
+  end)
+
   it('dumps and loads last search pattern with offset', function()
     api.nvim_set_option_value('wrapscan', false, {})
     fn.setline('.', { 'foo', 'bar--' })
@@ -182,6 +193,22 @@ describe('ShaDa support code', function()
     eq('goo', fn.getline(1))
   end)
 
+  it('does not dump last substitute pattern or replacement string if /0 in shada', function()
+    nvim_command('set shada+=/0')
+    fn.setline('.', { 'foo', 'bar' })
+    nvim_command('%s/f/g/g')
+    eq('goo', fn.getline(1))
+    expect_exit(nvim_command, 'qall!')
+    reset()
+    fn.setline('.', { 'foo', 'bar' })
+    nvim_command('language C')
+    local err = pcall_err(nvim_command, '&')
+    eq('Vim(&):E33: No previous substitute regular expression', err)
+    nvim_feed('/f\n')
+    err = pcall_err(nvim_command, '~&')
+    eq('Vim(~):E33: No previous substitute regular expression', err)
+  end)
+
   it('dumps and loads history with UTF-8 characters', function()
     reset()
     nvim_feed(':echo "«"\n')
@@ -210,24 +237,24 @@ describe('ShaDa support code', function()
 
   it('dumps and loads search pattern with UTF-8 characters', function()
     nvim_command('silent! /«/')
-    nvim_command('set shada+=/0')
     expect_exit(nvim_command, 'qall!')
     reset()
-    fn.setline('.', { '\171«' })
+    fn.setline('.', { 'foo', '\171«' })
-    nvim_command('~&')
+    nvim_feed('gg0n')
-    eq('\171', fn.getline('.'))
+    eq({ 0, 2, 2, 0 }, fn.getpos('.'))
+    eq('\171«', fn.getline('.'))
     eq('', fn.histget('/', -1))
   end)
 
   it('dumps and loads search pattern with 8-bit single-byte', function()
     -- \171 is U+00AB LEFT-POINTING DOUBLE ANGLE QUOTATION MARK in latin1
     nvim_command('silent! /\171/')
-    nvim_command('set shada+=/0')
     expect_exit(nvim_command, 'qall!')
     reset()
-    fn.setline('.', { '\171«' })
+    fn.setline('.', { 'foo', '\171«' })
-    nvim_command('~&')
+    nvim_feed('gg0n')
-    eq('«', fn.getline('.'))
+    eq({ 0, 2, 1, 0 }, fn.getpos('.'))
+    eq('\171«', fn.getline('.'))
     eq('', fn.histget('/', -1))
   end)
 
@@ -252,4 +279,14 @@ describe('ShaDa support code', function()
     nvim_command('wshada')
     assert_alive()
   end)
+
+  it('does not dump empty replacement string', function()
+    expect_exit(nvim_command, 'qall!')
+    reset()
+    fn.setline('.', { 'foo', 'bar' })
+    nvim_command('language C')
+    nvim_feed('/f\n')
+    local err = pcall_err(nvim_command, '~&')
+    eq('Vim(~):E33: No previous substitute regular expression', err)
+  end)
 end)

test/functional/shada/marks_spec.lua

@@ -51,29 +51,115 @@ describe('ShaDa support code', function()
     eq(2, nvim_current_line())
   end)
 
-  it('does not dump global mark with `f0` in shada', function()
+  it('can dump and read back numbered marks', function()
+    local function move(cmd)
+      feed(cmd)
+      nvim_command('wshada')
+    end
+    nvim_command('edit ' .. testfilename)
+    move('l')
+    move('l')
+    move('j')
+    move('l')
+    move('l')
+    nvim_command('edit ' .. testfilename_2)
+    move('l')
+    move('l')
+    move('j')
+    move('l')
+    move('l')
+    -- we have now populated marks 0 through 9
+    nvim_command('edit ' .. testfilename)
+    feed('gg0')
+    -- during shada save on exit, mark 0 will become the current position,
+    -- 9 will be removed, and all other marks shifted
+    expect_exit(nvim_command, 'qall')
+    reset()
+    nvim_command('edit ' .. testfilename)
+    nvim_command('edit ' .. testfilename_2)
+    local marklist = fn.getmarklist()
+    for _, mark in ipairs(marklist) do
+      mark.file = fn.fnamemodify(mark.file, ':t')
+    end
+    eq({
+      {
+        file = testfilename,
+        mark = "'0",
+        pos = { 1, 1, 1, 0 },
+      },
+      {
+        file = testfilename_2,
+        mark = "'1",
+        pos = { 2, 2, 5, 0 },
+      },
+      {
+        file = testfilename_2,
+        mark = "'2",
+        pos = { 2, 2, 4, 0 },
+      },
+      {
+        file = testfilename_2,
+        mark = "'3",
+        pos = { 2, 2, 3, 0 },
+      },
+      {
+        file = testfilename_2,
+        mark = "'4",
+        pos = { 2, 1, 3, 0 },
+      },
+      {
+        file = testfilename_2,
+        mark = "'5",
+        pos = { 2, 1, 2, 0 },
+      },
+      {
+        file = testfilename,
+        mark = "'6",
+        pos = { 1, 2, 5, 0 },
+      },
+      {
+        file = testfilename,
+        mark = "'7",
+        pos = { 1, 2, 4, 0 },
+      },
+      {
+        file = testfilename,
+        mark = "'8",
+        pos = { 1, 2, 3, 0 },
+      },
+      {
+        file = testfilename,
+        mark = "'9",
+        pos = { 1, 1, 3, 0 },
+      },
+    }, marklist)
+  end)
+
+  it('does not dump global or numbered marks with `f0` in shada', function()
     nvim_command('set shada+=f0')
     nvim_command('edit ' .. testfilename)
     nvim_command('mark A')
     nvim_command('2')
-    nvim_command('kB')
     nvim_command('wshada')
     reset()
     nvim_command('language C')
     eq('Vim(normal):E20: Mark not set', exc_exec('normal! `A'))
+    eq('Vim(normal):E20: Mark not set', exc_exec('normal! `0'))
   end)
 
-  it("does read back global mark even with `'0` and `f0` in shada", function()
+  it("restores global and numbered marks even with `'0` and `f0` in shada", function()
     nvim_command('edit ' .. testfilename)
     nvim_command('mark A')
     nvim_command('2')
-    nvim_command('kB')
     nvim_command('wshada')
     reset("set shada='0,f0")
     nvim_command('language C')
     nvim_command('normal! `A')
     eq(testfilename, fn.fnamemodify(api.nvim_buf_get_name(0), ':t'))
     eq(1, nvim_current_line())
+    nvim_command('normal! `0')
+    eq(testfilename, fn.fnamemodify(api.nvim_buf_get_name(0), ':t'))
+    eq(2, nvim_current_line())
   end)
 
   it('is able to dump and read back local mark', function()
@@ -152,6 +238,37 @@ describe('ShaDa support code', function()
     eq(saved, exec_capture('jumps'))
   end)
 
+  it("does not dump jumplist if `'0` in shada", function()
+    local empty_jumps = exec_capture('jumps')
+    nvim_command("set shada='0")
+    nvim_command('edit ' .. testfilename_2)
+    nvim_command('normal! G')
+    nvim_command('normal! gg')
+    nvim_command('edit ' .. testfilename)
+    nvim_command('normal! G')
+    nvim_command('normal! gg')
+    nvim_command('enew')
+    nvim_command('normal! gg')
+    expect_exit(nvim_command, 'qall')
+    reset()
+    eq(empty_jumps, exec_capture('jumps'))
+  end)
+
+  it("does read back jumplist even with `'0` in shada", function()
+    nvim_command('edit ' .. testfilename_2)
+    nvim_command('normal! G')
+    nvim_command('normal! gg')
+    nvim_command('edit ' .. testfilename)
+    nvim_command('normal! G')
+    nvim_command('normal! gg')
+    nvim_command('enew')
+    nvim_command('normal! gg')
+    local saved = exec_capture('jumps')
+    expect_exit(nvim_command, 'qall')
+    reset("set shada='0")
+    eq(saved, exec_capture('jumps'))
+  end)
+
   it('when dumping jump list also dumps current position', function()
     nvim_command('edit ' .. testfilename)
     nvim_command('normal! G')