patch 8.2.2222: Vim9: cannot keep script variables when reloadingv8.2.2222

Problem:    Vim9: cannot keep script variables when reloading.
Solution:   Add the "noclear" argument to :vim9script.

8 files changed, 161 insertions, 48 deletions

diff --git a/runtime/doc/vim9.txt b/runtime/doc/vim9.txt
index d68d936e5f..dd8cd61f20 100644
--- a/runtime/doc/vim9.txt
+++ b/runtime/doc/vim9.txt
@@ -25,7 +25,7 @@ THIS IS STILL UNDER DEVELOPMENT - ANYTHING CAN BREAK - ANYTHING CAN CHANGE
 
 ==============================================================================
 
-1. What is Vim9 script?					*vim9-script*
+1. What is Vim9 script?					*Vim9-script*
 
 THIS IS STILL UNDER DEVELOPMENT - ANYTHING CAN BREAK - ANYTHING CAN CHANGE
 
@@ -112,7 +112,12 @@ In Vi # is a command to list text with numbers.  In Vim9 script you can use
 	101 number
 
 To improve readability there must be a space between a command and the #
-that starts a comment.
+that starts a comment: >
+	var = value # comment
+	var = value# error!
+
+In legacy script # is also used for the alternate file name.  In Vim9 script
+you need to use %% instead.  Instead of ## use %%% (stands for all arguments).
 
 
 Vim9 functions ~
@@ -193,6 +198,45 @@ You can use an autoload function if needed, or call a legacy function and have
 |FuncUndefined| triggered there.
 
 
+Reloading a Vim9 script clears functions and variables by default ~
+							*vim9-reload*
+When loading a legacy Vim script a second time nothing is removed, the
+commands will replace existing variables and functions and create new ones.
+
+When loading a Vim9 script a second time all existing script-local functions
+and variables are deleted, thus you start with a clean slate.  This is useful
+if you are developing a plugin and want to try a new version.  If you renamed
+something you don't have to worry about the old name still hanging around.
+
+If you do want to keep items, use: >
+	vimscript noclear
+
+You want to use this in scripts that use a `finish` command to bail out at
+some point when loaded again.  E.g. when a buffer local option is set: >
+	vimscript noclear
+	setlocal completefunc=SomeFunc
+	if exists('*SomeFunc') | finish | endif
+	def g:SomeFunc()
+	....
+
+There is one gotcha: If a compiled function is replaced and it is called from
+another compiled function that is not replaced, it will try to call the
+function from before it was replaced, which no longer exists.  This doesn't
+work: >
+	vimscript noclear
+
+	def ReplaceMe()
+	  echo 'function redefined every time'
+	enddef
+
+	if exists('s:loaded') | finish | endif
+	var s:loaded = true
+
+	def NotReplaced()
+	  ReplaceMe()  # Error if ReplaceMe() was redefined
+	enddef
+
+
 Variable declarations with :var, :final and :const ~
 						*vim9-declaration* *:var*
 Local variables need to be declared with `:var`.  Local constants need to be
@@ -340,7 +384,7 @@ When using `function()` the resulting type is "func", a function with any
 number of arguments and any return type.  The function can be defined later.
 
 
-Lamba using => instead of -> ~
+Lambda using => instead of -> ~
 
 In legacy script there can be confusion between using "->" for a method call
 and for a lambda.  Also, when a "{" is found the parser needs to figure out if
@@ -351,7 +395,7 @@ To avoid these problems Vim9 script uses a different syntax for a lambda,
 which is similar to Javascript: >
 	var Lambda = (arg) => expression
 
-No line break is allowed in the arguments of a lambda up to and includeing the
+No line break is allowed in the arguments of a lambda up to and including the
 "=>".  This is OK: >
 	filter(list, (k, v) =>
 			v > 0)
@@ -369,9 +413,9 @@ Additionally, a lambda can contain statements in {}: >
 	    }
 NOT IMPLEMENTED YET
 
-Note that the "{" must be followed by white space, otherwise it is assumed to
-be the start of a dictionary: >
-	var Lambda = (arg) => {key: 42}
+To avoid the "{" of a dictionary literal to be recognized as a statement block
+wrap it in parenthesis: >
+	var Lambda = (arg) => ({key: 42})
 
 
 Automatic line continuation ~
@@ -737,18 +781,24 @@ prefix and they do not need to exist (they can be deleted any time).
 Limitations ~
 
 Local variables will not be visible to string evaluation.  For example: >
-	def EvalString(): list<string>
+	def MapList(): list<string>
 	  var list = ['aa', 'bb', 'cc', 'dd']
 	  return range(1, 2)->map('list[v:val]')
 	enddef
 
 The map argument is a string expression, which is evaluated without the
 function scope.  Instead, use a lambda: >
-	def EvalString(): list<string>
+	def MapList(): list<string>
 	  var list = ['aa', 'bb', 'cc', 'dd']
-	  return range(1, 2)->map({ _, v -> list[v] })
+	  return range(1, 2)->map(( _, v) => list[v])
 	enddef
 
+The same is true for commands that are not compiled, such as `:global`.
+For these the backtick expansion can be used.  Example: >
+	def Replace()
+	  var newText = 'blah'
+	  g/pattern/s/^/`=newText`/
+	enddef
 
 ==============================================================================
 
diff --git a/src/ex_cmds.h b/src/ex_cmds.h
index 5e61c3e7e7..75bb7fb943 100644
--- a/src/ex_cmds.h
+++ b/src/ex_cmds.h
@@ -1680,7 +1680,7 @@ EXCMD(CMD_vimgrepadd,	"vimgrepadd",	ex_vimgrep,
 	EX_RANGE|EX_BANG|EX_NEEDARG|EX_EXTRA|EX_NOTRLCOM|EX_TRLBAR|EX_XFILE|EX_LOCK_OK,
 	ADDR_OTHER),
 EXCMD(CMD_vim9script,	"vim9script",	ex_vim9script,
-	EX_CMDWIN|EX_LOCK_OK,
+	EX_WORD1|EX_CMDWIN|EX_LOCK_OK,
 	ADDR_NONE),
 EXCMD(CMD_viusage,	"viusage",	ex_viusage,
 	EX_TRLBAR,
diff --git a/src/ex_docmd.c b/src/ex_docmd.c
index 69e3f12e69..156f0df16a 100644
--- a/src/ex_docmd.c
+++ b/src/ex_docmd.c
@@ -2594,7 +2594,7 @@ do_one_cmd(
     // Set flag that any command was executed, used by ex_vim9script().
     if (getline_equal(ea.getline, ea.cookie, getsourceline)
 						    && current_sctx.sc_sid > 0)
-	SCRIPT_ITEM(current_sctx.sc_sid)->sn_had_command = TRUE;
+	SCRIPT_ITEM(current_sctx.sc_sid)->sn_state = SN_STATE_HAD_COMMAND;
 
     /*
      * If the command just executed called do_cmdline(), any throw or ":return"
diff --git a/src/scriptfile.c b/src/scriptfile.c
index 327500bd07..38704a7fd7 100644
--- a/src/scriptfile.c
+++ b/src/scriptfile.c
@@ -1320,43 +1320,27 @@ do_source(
     if (sid > 0)
     {
 	hashtab_T	*ht;
-	int		is_vim9 = si->sn_version == SCRIPT_VERSION_VIM9;
+	int		todo;
+	hashitem_T	*hi;
+	dictitem_T	*di;
 
 	// loading the same script again
-	si->sn_had_command = FALSE;
+	si->sn_state = SN_STATE_RELOAD;
 	si->sn_version = 1;
 	current_sctx.sc_sid = sid;
 
-	// In Vim9 script all script-local variables are removed when reloading
-	// the same script.  In legacy script they remain but "const" can be
-	// set again.
+	// Script-local variables remain but "const" can be set again.
+	// In Vim9 script variables will be cleared when "vim9script" is
+	// encountered without the "noclear" argument.
 	ht = &SCRIPT_VARS(sid);
-	if (is_vim9)
-	{
-	    hashtab_free_contents(ht);
-	    hash_init(ht);
-	}
-	else
-	{
-	    int		todo = (int)ht->ht_used;
-	    hashitem_T	*hi;
-	    dictitem_T	*di;
-
-	    for (hi = ht->ht_array; todo > 0; ++hi)
-		if (!HASHITEM_EMPTY(hi))
-		{
-		    --todo;
-		    di = HI2DI(hi);
-		    di->di_flags |= DI_FLAGS_RELOAD;
-		}
-	}
-
-	// old imports and script variables are no longer valid
-	free_imports_and_script_vars(sid);
-
-	// in Vim9 script functions are marked deleted
-	if (is_vim9)
-	    delete_script_functions(sid);
+	todo = (int)ht->ht_used;
+	for (hi = ht->ht_array; todo > 0; ++hi)
+	    if (!HASHITEM_EMPTY(hi))
+	    {
+		--todo;
+		di = HI2DI(hi);
+		di->di_flags |= DI_FLAGS_RELOAD;
+	    }
     }
     else
     {
@@ -1390,8 +1374,10 @@ do_source(
 	fname_exp = vim_strsave(si->sn_name);  // used for autocmd
 	if (ret_sid != NULL)
 	    *ret_sid = current_sctx.sc_sid;
+
+	// Used to check script variable index is still valid.
+	si->sn_script_seq = current_sctx.sc_seq;
     }
-    si->sn_script_seq = current_sctx.sc_seq;
 
 # ifdef FEAT_PROFILE
     if (do_profiling == PROF_YES)
diff --git a/src/structs.h b/src/structs.h
index ea994f8b70..93a6b0ad52 100644
--- a/src/structs.h
+++ b/src/structs.h
@@ -1821,7 +1821,7 @@ typedef struct
     int		sn_last_block_id;  // Unique ID for each script block
 
     int		sn_version;	// :scriptversion
-    int		sn_had_command;	// TRUE if any command was executed
+    int		sn_state;	// SN_STATE_ values
     char_u	*sn_save_cpo;	// 'cpo' value when :vim9script found
 
 # ifdef FEAT_PROFILE
@@ -1845,6 +1845,10 @@ typedef struct
 # endif
 } scriptitem_T;
 
+#define SN_STATE_NEW		0   // newly loaded script, nothing done
+#define SN_STATE_RELOAD		1   // script loaded before, nothing done
+#define SN_STATE_HAD_COMMAND	9   // a command was executed
+
 // Struct passed through eval() functions.
 // See EVALARG_EVALUATE for a fixed value with eval_flags set to EVAL_EVALUATE.
 typedef struct {
diff --git a/src/testdir/test_vim9_script.vim b/src/testdir/test_vim9_script.vim
index 1e64daaec3..cdd83c348b 100644
--- a/src/testdir/test_vim9_script.vim
+++ b/src/testdir/test_vim9_script.vim
@@ -1158,6 +1158,53 @@ def Run_Test_import_fails_on_command_line()
   StopVimInTerminal(buf)
 enddef
 
+def Test_vim9script_reload_noclear()
+  var lines =<< trim END
+    vim9script noclear
+    g:loadCount += 1
+    var s:reloaded = 'init'
+
+    def Again(): string
+      return 'again'
+    enddef
+
+    if exists('s:loaded') | finish | endif
+    var s:loaded = true
+
+    var s:notReloaded = 'yes'
+    s:reloaded = 'first'
+    def g:Values(): list<string>
+      return [s:reloaded, s:notReloaded, Once()]
+    enddef
+    def g:CallAgain(): string
+      return Again()
+    enddef
+
+    def Once(): string
+      return 'once'
+    enddef
+  END
+  writefile(lines, 'XReloaded')
+  g:loadCount = 0
+  source XReloaded
+  assert_equal(1, g:loadCount)
+  assert_equal(['first', 'yes', 'once'], g:Values())
+  assert_equal('again', g:CallAgain())
+  source XReloaded
+  assert_equal(2, g:loadCount)
+  assert_equal(['init', 'yes', 'once'], g:Values())
+  assert_fails('call g:CallAgain()', 'E933:')
+  source XReloaded
+  assert_equal(3, g:loadCount)
+  assert_equal(['init', 'yes', 'once'], g:Values())
+  assert_fails('call g:CallAgain()', 'E933:')
+
+  delete('Xreloaded')
+  delfunc g:Values
+  delfunc g:CallAgain
+  unlet g:loadCount
+enddef
+
 def Test_vim9script_reload_import()
   var lines =<< trim END
     vim9script
diff --git a/src/version.c b/src/version.c
index ba5df927a6..f488d77c32 100644
--- a/src/version.c
+++ b/src/version.c
@@ -751,6 +751,8 @@ static char *(features[]) =
 static int included_patches[] =
 {   /* Add new patch number below this line */
 /**/
+    2222,
+/**/
     2221,
 /**/
     2220,
diff --git a/src/vim9script.c b/src/vim9script.c
index 163b8a7f35..70efc4065b 100644
--- a/src/vim9script.c
+++ b/src/vim9script.c
@@ -32,6 +32,7 @@ in_vim9script(void)
     void
 ex_vim9script(exarg_T *eap)
 {
+    int		    sid = current_sctx.sc_sid;
     scriptitem_T    *si;
 
     if (!getline_equal(eap->getline, eap->cookie, getsourceline))
@@ -39,15 +40,35 @@ ex_vim9script(exarg_T *eap)
 	emsg(_(e_vim9script_can_only_be_used_in_script));
 	return;
     }
-    si = SCRIPT_ITEM(current_sctx.sc_sid);
-    if (si->sn_had_command)
+
+    si = SCRIPT_ITEM(sid);
+    if (si->sn_state == SN_STATE_HAD_COMMAND)
     {
 	emsg(_(e_vim9script_must_be_first_command_in_script));
 	return;
     }
+    if (!IS_WHITE_OR_NUL(*eap->arg) && STRCMP(eap->arg, "noclear") != 0)
+    {
+	semsg(_(e_invarg2), eap->arg);
+	return;
+    }
+    if (si->sn_state == SN_STATE_RELOAD && IS_WHITE_OR_NUL(*eap->arg))
+    {
+	hashtab_T	*ht = &SCRIPT_VARS(sid);
+
+	// Reloading a script without the "noclear" argument: clear
+	// script-local variables and functions.
+	hashtab_free_contents(ht);
+	hash_init(ht);
+	delete_script_functions(sid);
+
+	// old imports and script variables are no longer valid
+	free_imports_and_script_vars(sid);
+    }
+    si->sn_state = SN_STATE_HAD_COMMAND;
+
     current_sctx.sc_version = SCRIPT_VERSION_VIM9;
     si->sn_version = SCRIPT_VERSION_VIM9;
-    si->sn_had_command = TRUE;
 
     if (STRCMP(p_cpo, CPO_VIM) != 0)
     {
@@ -719,6 +740,9 @@ free_all_script_vars(scriptitem_T *si)
     hash_init(ht);
 
     ga_clear(&si->sn_var_vals);
+
+    // existing commands using script variable indexes are no longer valid
+    si->sn_script_seq = current_sctx.sc_seq;
 }
 
 /*