patch 9.0.1275: the code for setting options is too complicated

Problem:    The code for setting options is too complicated.
Solution:   Refactor the do_set() function. (Yegappan Lakshmanan, Lewis
            Russell, closes #11932)

src/option.c

@@ -10,16 +10,20 @@
 /*
  * Code to handle user-settable options. This is all pretty much table-
  * driven. Checklist for adding a new option:
- * - Put it in the options array below (copy an existing entry).
+ * - Put it in the options array in optiondefs.h (copy an existing entry).
  * - For a global option: Add a variable for it in option.h.
  * - For a buffer or window local option:
- *   - Add a PV_XX entry to the enum below.
+ *   - Add a PV_XX macro definition to the optiondefs.h file.
  *   - Add a variable to the window or buffer struct in structs.h.
  *   - For a window option, add some code to copy_winopt().
+ *   - For a window string option, add code to check_win_options() and
+ *     clear_winopt().
  *   - For a buffer option, add some code to buf_copy_options().
  *   - For a buffer string option, add code to check_buf_options().
- * - If it's a numeric option, add any necessary bounds checks to do_set().
+ * - If it's a numeric option, add any necessary bounds checks to
- * - If it's a list of flags, add some code in do_set(), search for WW_ALL.
+ *   set_num_option().
+ * - If it's a list of flags, add some code in did_set_string_option(), search
+ *   for WW_ALL.
  * - When adding an option with expansion (P_EXPAND), but with a different
  *   default for Vi and Vim (no P_VI_DEF), add some code at VIMEXP.
  * - Add documentation!  One line in doc/quickref.txt, full description in
@@ -1633,89 +1637,37 @@ do_set_string(
 }
 
 /*
- * Parse 'arg' for option settings.
+ * Set an option to a new value.
- *
+ * Return NULL if OK, return an untranslated error message when something is
- * 'arg' may be IObuff, but only when no errors can be present and option
+ * wrong.  "errbuf[errbuflen]" can be used to create the error message.
- * does not need to be expanded with option_expand().
- * "opt_flags":
- * 0 for ":set"
- * OPT_GLOBAL     for ":setglobal"
- * OPT_LOCAL      for ":setlocal" and a modeline
- * OPT_MODELINE   for a modeline
- * OPT_WINONLY    to only set window-local options
- * OPT_NOWIN	  to skip setting window-local options
- * OPT_ONECOLUMN  do not use multiple columns
- *
- * returns FAIL if an error is detected, OK otherwise
  */
-    int
+    static char *
-do_set(
+do_set_option(
-    char_u	*arg_start,	// option string (may be written to!)
+    int		opt_flags,
-    int		opt_flags)
+    char_u	**argp,
+    char_u	*arg_start,
+    char_u	**startarg,
+    int		*did_show,
+    int		*stopopteval,
+    char	*errbuf,
+    size_t	errbuflen)
 {
-    char_u	*arg = arg_start;
+    char	*errmsg = NULL;
-    int		opt_idx;
-    char	*errmsg;
-    char	errbuf[80];
-    char_u	*startarg;
     int		prefix;	    // 1: nothing, 0: "no", 2: "inv" in front of name
     int		nextchar;   // next non-white char after option name
     int		afterchar;  // character just after option name
-    int		len;
+    char_u	*arg = *argp;
-    int		i;
-    varnumber_T	value;
     int		key;
+    int		opt_idx;
+    int		len;
+    set_op_T	op = 0;
     long_u	flags;		    // flags for current option
     char_u	*varp = NULL;	    // pointer to variable for current option
-    int		did_show = FALSE;   // already showed one value
-    set_op_T	op = 0;
-    int		cp_val = 0;
     char_u	key_name[2];
+    int		cp_val = 0;
+    varnumber_T	value;
+    int		i;
 
-    if (*arg == NUL)
-    {
-	showoptions(0, opt_flags);
-	did_show = TRUE;
-	goto theend;
-    }
-
-    while (*arg != NUL)		// loop to process all options
-    {
-	errmsg = NULL;
-	startarg = arg;		// remember for error message
-
-	if (STRNCMP(arg, "all", 3) == 0 && !ASCII_ISALPHA(arg[3])
-						&& !(opt_flags & OPT_MODELINE))
-	{
-	    /*
-	     * ":set all"  show all options.
-	     * ":set all&" set all options to their default value.
-	     */
-	    arg += 3;
-	    if (*arg == '&')
-	    {
-		++arg;
-		// Only for :set command set global value of local options.
-		set_options_default(OPT_FREE | opt_flags);
-		didset_options();
-		didset_options2();
-		redraw_all_later(UPD_CLEAR);
-	    }
-	    else
-	    {
-		showoptions(1, opt_flags);
-		did_show = TRUE;
-	    }
-	}
-	else if (STRNCMP(arg, "termcap", 7) == 0 && !(opt_flags & OPT_MODELINE))
-	{
-	    showoptions(2, opt_flags);
-	    show_termcodes(opt_flags);
-	    did_show = TRUE;
-	    arg += 7;
-	}
-	else
-	{
     prefix = 1;
     if (STRNCMP(arg, "no", 2) == 0 && STRNCMP(arg, "novice", 6) != 0)
     {
@@ -1787,7 +1739,7 @@ do_set(
 	{
 	    errmsg = e_no_white_space_allowed_between_option_and;
 	    arg = p;
-		    startarg = p;
+	    *startarg = p;
 	    goto skip;
 	}
     }
@@ -1943,12 +1895,12 @@ do_set(
 	/*
 	 * print value
 	 */
-		if (did_show)
+	if (*did_show)
 	    msg_putchar('\n');	    // cursor below last one
 	else
 	{
 	    gotocmdline(TRUE);	    // cursor at status line
-		    did_show = TRUE;	    // remember that we did a line
+	    *did_show = TRUE;	    // remember that we did a line
 	}
 	if (opt_idx >= 0)
 	{
@@ -2115,7 +2067,7 @@ do_set(
 		else if (op == OP_REMOVING)
 		    value = *(long *)varp - value;
 		errmsg = set_num_option(opt_idx, varp, value,
-					   errbuf, sizeof(errbuf), opt_flags);
+			errbuf, errbuflen, opt_flags);
 	    }
 	    else if (opt_idx >= 0)		    // string
 	    {
@@ -2125,7 +2077,8 @@ do_set(
 		{
 		    if (errmsg != NULL)
 			goto skip;
-			    break;
+		    *stopopteval = TRUE;
+		    goto skip;
 		}
 	    }
 	    else	    // key code option
@@ -2160,6 +2113,87 @@ do_set(
     }
 
 skip:
+    *argp = arg;
+    return errmsg;
+}
+
+/*
+ * Parse 'arg' for option settings.
+ *
+ * 'arg' may be IObuff, but only when no errors can be present and option
+ * does not need to be expanded with option_expand().
+ * "opt_flags":
+ * 0 for ":set"
+ * OPT_GLOBAL     for ":setglobal"
+ * OPT_LOCAL      for ":setlocal" and a modeline
+ * OPT_MODELINE   for a modeline
+ * OPT_WINONLY    to only set window-local options
+ * OPT_NOWIN	  to skip setting window-local options
+ * OPT_ONECOLUMN  do not use multiple columns
+ *
+ * Returns FAIL if an error is detected, OK otherwise.
+ */
+    int
+do_set(
+    char_u	*arg_start,	// option string (may be written to!)
+    int		opt_flags)
+{
+    char_u	*arg = arg_start;
+    int		i;
+    int		did_show = FALSE;   // already showed one value
+
+    if (*arg == NUL)
+    {
+	showoptions(0, opt_flags);
+	did_show = TRUE;
+	goto theend;
+    }
+
+    while (*arg != NUL)		// loop to process all options
+    {
+	if (STRNCMP(arg, "all", 3) == 0 && !ASCII_ISALPHA(arg[3])
+						&& !(opt_flags & OPT_MODELINE))
+	{
+	    /*
+	     * ":set all"  show all options.
+	     * ":set all&" set all options to their default value.
+	     */
+	    arg += 3;
+	    if (*arg == '&')
+	    {
+		++arg;
+		// Only for :set command set global value of local options.
+		set_options_default(OPT_FREE | opt_flags);
+		didset_options();
+		didset_options2();
+		redraw_all_later(UPD_CLEAR);
+	    }
+	    else
+	    {
+		showoptions(1, opt_flags);
+		did_show = TRUE;
+	    }
+	}
+	else if (STRNCMP(arg, "termcap", 7) == 0 && !(opt_flags & OPT_MODELINE))
+	{
+	    showoptions(2, opt_flags);
+	    show_termcodes(opt_flags);
+	    did_show = TRUE;
+	    arg += 7;
+	}
+	else
+	{
+	    int		stopopteval = FALSE;
+	    char	*errmsg = NULL;
+	    char	errbuf[80];
+	    char_u	*startarg = arg;
+
+	    errmsg = do_set_option(opt_flags, &arg, arg_start, &startarg,
+					&did_show, &stopopteval, errbuf,
+					sizeof(errbuf));
+	    if (stopopteval)
+		break;
+
 	    /*
 	     * Advance to next argument.
 	     * - skip until a blank found, taking care of backslashes
@@ -2175,7 +2209,6 @@ skip:
 		if (*arg != '=')
 		    break;
 	    }
-	}
 
 	    if (errmsg != NULL)
 	    {
@@ -2197,6 +2230,7 @@ skip:
 
 		return FAIL;
 	    }
+	}
 
 	arg = skipwhite(arg);
     }

src/optionstr.c

@@ -680,6 +680,7 @@ did_set_term(int *opt_idx, long_u *free_oldval)
 	// Both 'term' and 'ttytype' point to T_NAME, only set the
 	// P_ALLOCED flag on 'term'.
 	*opt_idx = findoption((char_u *)"term");
+	if (*opt_idx >= 0)
 	    *free_oldval = (get_option_flags(*opt_idx) & P_ALLOCED);
     }
 

src/version.c

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