patch 8.0.0974: resetting a string option does not trigger OptionSet

Problem:    Resetting a string option does not trigger OptionSet. (Rick Howe)
Solution:   Set the origval.

src/option.c

@@ -4351,8 +4351,6 @@ trigger_optionsset_string(
 		       (char_u *)options[opt_idx].fullname, NULL, FALSE, NULL);
 	reset_v_option_vars();
     }
-    vim_free(oldval);
-    vim_free(newval);
 }
 #endif
 
@@ -4818,19 +4816,19 @@ do_set(
 		    }
 		    else if (opt_idx >= 0)		    /* string */
 		    {
-			char_u	    *save_arg = NULL;
+			char_u	  *save_arg = NULL;
-			char_u	    *s = NULL;
+			char_u	  *s = NULL;
-			char_u	    *oldval = NULL;	/* previous value if *varp */
+			char_u	  *oldval = NULL; /* previous value if *varp */
-			char_u	    *newval;
+			char_u	  *newval;
-			char_u	    *origval = NULL;
+			char_u	  *origval = NULL;
 #if defined(FEAT_AUTOCMD) && defined(FEAT_EVAL)
-			char_u	    *saved_origval = NULL;
+			char_u	  *saved_origval = NULL;
-			char_u	    *saved_newval = NULL;
+			char_u	  *saved_newval = NULL;
 #endif
-			unsigned    newlen;
+			unsigned  newlen;
-			int	    comma;
+			int	  comma;
-			int	    bs;
+			int	  bs;
-			int	    new_value_alloced;	/* new string option
+			int	  new_value_alloced;	/* new string option
 							   was allocated */
 
 			/* When using ":set opt=val" for a global option
@@ -4843,6 +4841,16 @@ do_set(
 			/* The old value is kept until we are sure that the
 			 * new value is valid. */
 			oldval = *(char_u **)varp;
+
+			/* When setting the local value of a global
+			 * option, the old value may be the global value. */
+			if (((int)options[opt_idx].indir & PV_BOTH)
+					       && (opt_flags & OPT_LOCAL))
+			    origval = *(char_u **)get_varp(
+						       &options[opt_idx]);
+			else
+			    origval = oldval;
+
 			if (nextchar == '&')	/* set to default val */
 			{
 			    newval = options[opt_idx].def_val[
@@ -4957,15 +4965,6 @@ do_set(
 				++arg;
 			    }
 
-			    /* When setting the local value of a global
-			     * option, the old value may be the global value. */
-			    if (((int)options[opt_idx].indir & PV_BOTH)
-						   && (opt_flags & OPT_LOCAL))
-				origval = *(char_u **)get_varp(
-							   &options[opt_idx]);
-			    else
-				origval = oldval;
-
 			    /*
 			     * Copy the new string into allocated memory.
 			     * Can't use set_string_option_direct(), because
@@ -5169,7 +5168,9 @@ do_set(
 			    new_value_alloced = TRUE;
 			}
 
-			/* Set the new value. */
+			/*
+			 * Set the new value.
+			 */
 			*(char_u **)(varp) = newval;
 
 #if defined(FEAT_AUTOCMD) && defined(FEAT_EVAL)
@@ -5195,19 +5196,16 @@ do_set(
 			errmsg = did_set_string_option(opt_idx, (char_u **)varp,
 				new_value_alloced, oldval, errbuf, opt_flags);
 
+#if defined(FEAT_AUTOCMD) && defined(FEAT_EVAL)
+			if (errmsg == NULL)
+			    trigger_optionsset_string(opt_idx, opt_flags,
+						  saved_origval, saved_newval);
+			vim_free(saved_origval);
+			vim_free(saved_newval);
+#endif
 			/* If error detected, print the error message. */
 			if (errmsg != NULL)
-			{
-#if defined(FEAT_AUTOCMD) && defined(FEAT_EVAL)
-			    vim_free(saved_origval);
-			    vim_free(saved_newval);
-#endif
 			    goto skip;
-			}
-#if defined(FEAT_AUTOCMD) && defined(FEAT_EVAL)
-			trigger_optionsset_string(opt_idx, opt_flags,
-						  saved_origval, saved_newval);
-#endif
 		    }
 		    else	    /* key code option */
 		    {
@@ -6014,8 +6012,11 @@ set_string_option(
 
 #if defined(FEAT_AUTOCMD) && defined(FEAT_EVAL)
 	/* call autocommand after handling side effects */
-	trigger_optionsset_string(opt_idx, opt_flags,
+	if (r == NULL)
+	    trigger_optionsset_string(opt_idx, opt_flags,
 						   saved_oldval, saved_newval);
+	vim_free(saved_oldval);
+	vim_free(saved_newval);
 #endif
     }
     return r;

src/testdir/test_autocmd.vim

@@ -548,6 +548,19 @@ func Test_OptionSet()
   call assert_equal([['key', 'invalid', 'invalid1', 'invalid']], g:options)
   call assert_equal(g:opt[0], g:opt[1])
 
+  " 18: Setting string option"
+  let oldval = &tags
+  let g:options=[['tags', oldval, 'tagpath', 'global']]
+  set tags=tagpath
+  call assert_equal([], g:options)
+  call assert_equal(g:opt[0], g:opt[1])
+
+  " 1l: Resetting string option"
+  let g:options=[['tags', 'tagpath', oldval, 'global']]
+  set tags&
+  call assert_equal([], g:options)
+  call assert_equal(g:opt[0], g:opt[1])
+
   " Cleanup
   au! OptionSet
   for opt in ['nu', 'ai', 'acd', 'ar', 'bs', 'backup', 'cul', 'cp']

src/version.c

@@ -769,6 +769,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    974,
 /**/
     973,
 /**/