options: set 'scrollback' to -1 by default #9563

Makes the 'scrollback' option more consistent (same default for all buffers) and future-proof.

- Default to -1 for all buffers, but treat it as an implementation detail.
- Document range of 1 - 100_000.
- New terminal buffer by default sets scrollback=10_000 if the global default is -1.
- Existing terminal buffer: On entering terminal-mode or on refresh, if the user explicitly did `:set[local] scbk=-1`, the local value goes to 100_000 (max). (This is undocumented on purpose. Users should work with explicit values in the range of 1-100_000.)

runtime/doc/options.txt

@@ -4761,14 +4761,12 @@ A jump table for the options with a short description can be found at |Q_op|.
 	height with ":set scroll=0".
 
 						*'scrollback'* *'scbk'*
-'scrollback' 'scbk'	number	(default: 10000
-				 in normal buffers: -1)
+'scrollback' 'scbk'	number	(default: 10000)
 			local to buffer
 	Maximum number of lines kept beyond the visible screen. Lines at the
 	top are deleted if new lines exceed this limit.
+	Minimum is 1, maximum is 100000.
 	Only in |terminal| buffers.
-	-1 means "unlimited" for normal buffers, 100000 otherwise.
-	Minimum is 1.
 
 			*'scrollbind'* *'scb'* *'noscrollbind'* *'noscb'*
 'scrollbind' 'scb'	boolean  (default off)

src/nvim/option.c

@@ -4240,8 +4240,7 @@ static char *set_num_option(int opt_idx, char_u *varp, long value,
   } else if (pp == &curbuf->b_p_channel || pp == &p_channel) {
     errmsg = e_invarg;
   } else if (pp == &curbuf->b_p_scbk || pp == &p_scbk) {
-    if (value < -1 || value > SB_MAX
-        || (value != -1 && opt_flags == OPT_LOCAL && !curbuf->terminal)) {
+    if (value < -1 || value > SB_MAX) {
       errmsg = e_invarg;
     }
   } else if (pp == &curbuf->b_p_sw || pp == &p_sw) {
@@ -4435,11 +4434,6 @@ static char *set_num_option(int opt_idx, char_u *varp, long value,
     *(long *)get_varp_scope(&(options[opt_idx]), OPT_GLOBAL) = *pp;
   }
 
-  if (pp == &curbuf->b_p_scbk && !curbuf->terminal) {
-    // Normal buffer: reset local 'scrollback' after updating the global value.
-    curbuf->b_p_scbk = -1;
-  }
-
   options[opt_idx].flags |= P_WAS_SET;
 
   // Don't do this while starting up, failure or recursively.
@@ -5862,7 +5856,7 @@ void buf_copy_options(buf_T *buf, int flags)
       buf->b_p_ai = p_ai;
       buf->b_p_ai_nopaste = p_ai_nopaste;
       buf->b_p_sw = p_sw;
-      buf->b_p_scbk = -1;
+      buf->b_p_scbk = p_scbk;
       buf->b_p_tw = p_tw;
       buf->b_p_tw_nopaste = p_tw_nopaste;
       buf->b_p_tw_nobin = p_tw_nobin;

src/nvim/options.lua

@@ -1930,7 +1930,7 @@ return {
       vi_def=true,
       varname='p_scbk',
       redraw={'current_buffer'},
-      defaults={if_true={vi=10000}}
+      defaults={if_true={vi=-1}}
     },
     {
       full_name='scrollbind', abbreviation='scb',

src/nvim/terminal.c

@@ -236,7 +236,7 @@ Terminal *terminal_open(TerminalOptions opts)
   // Default settings for terminal buffers
   curbuf->b_p_ma = false;     // 'nomodifiable'
   curbuf->b_p_ul = -1;        // 'undolevels'
-  curbuf->b_p_scbk = p_scbk;  // 'scrollback'
+  curbuf->b_p_scbk = (p_scbk == -1) ? 10000 : MAX(1, p_scbk);  // 'scrollback'
   curbuf->b_p_tw = 0;         // 'textwidth'
   set_option_value("wrap", false, NULL, OPT_LOCAL);
   set_option_value("list", false, NULL, OPT_LOCAL);
@@ -249,8 +249,7 @@ Terminal *terminal_open(TerminalOptions opts)
   apply_autocmds(EVENT_TERMOPEN, NULL, NULL, false, curbuf);
 
   // Configure the scrollback buffer.
-  rv->sb_size = curbuf->b_p_scbk < 0
-                ? SB_MAX : (size_t)MAX(1, curbuf->b_p_scbk);
+  rv->sb_size = (size_t)curbuf->b_p_scbk;
   rv->sb_buffer = xmalloc(sizeof(ScrollbackLine *) * rv->sb_size);
 
   if (!true_color) {
@@ -1162,8 +1161,10 @@ static void refresh_size(Terminal *term, buf_T *buf)
 /// Adjusts scrollback storage after 'scrollback' option changed.
 static void on_scrollback_option_changed(Terminal *term, buf_T *buf)
 {
-  const size_t scbk = curbuf->b_p_scbk < 0
-                      ? SB_MAX : (size_t)MAX(1, curbuf->b_p_scbk);
+  if (buf->b_p_scbk < 1) {
+    buf->b_p_scbk = SB_MAX;
+  }
+  const size_t scbk = (size_t)buf->b_p_scbk;
   assert(term->sb_current < SIZE_MAX);
   if (term->sb_pending > 0) {  // Pending rows must be processed first.
     abort();

test/functional/terminal/scrollback_spec.lua

@@ -481,25 +481,10 @@ describe("'scrollback' option", function()
     eq(-1, curbufmeths.get_option('scrollback'))
   end)
 
-  it(':setlocal in a normal buffer is an error', function()
-    command('new')
-
-    -- :setlocal to -1 is NOT an error.
-    feed_command('setlocal scrollback=-1')
-    eq(nil, string.match(eval("v:errmsg"), "E%d*:"))
-    feed('<CR>')
-
-    -- :setlocal to anything except -1 is an error.
-    feed_command('setlocal scrollback=42')
-    feed('<CR>')
-    eq('E474:', string.match(eval("v:errmsg"), "E%d*:"))
-    eq(-1, curbufmeths.get_option('scrollback'))
-  end)
-
   it(':set updates local value and global default', function()
     set_fake_shell()
-    command('set scrollback=42')                  -- set global and (attempt) local
-    eq(-1, curbufmeths.get_option('scrollback'))  -- normal buffer: -1
+    command('set scrollback=42')                  -- set global value
+    eq(42, curbufmeths.get_option('scrollback'))
     command('terminal')
     eq(42, curbufmeths.get_option('scrollback'))  -- inherits global default
     command('setlocal scrollback=99')