patch 9.0.0040: use of set_chars_option() is confusingv9.0.0040

Problem:    Use of set_chars_option() is confusing.
Solution:   Add "apply" argument to store the result or not.  Merge similar
            code.

6 files changed, 66 insertions, 61 deletions

diff --git a/src/mbyte.c b/src/mbyte.c
index 1e3ef72a3..691c1608e 100644
--- a/src/mbyte.c
+++ b/src/mbyte.c
@@ -5647,9 +5647,9 @@ f_setcellwidths(typval_T *argvars, typval_T *rettv UNUSED)
 
     // Check that the new value does not conflict with 'fillchars' or
     // 'listchars'.
-    if (set_chars_option(curwin, &p_fcs) != NULL)
+    if (set_chars_option(curwin, &p_fcs, FALSE) != NULL)
 	error = e_conflicts_with_value_of_fillchars;
-    else if (set_chars_option(curwin, &p_lcs) != NULL)
+    else if (set_chars_option(curwin, &p_lcs, FALSE) != NULL)
 	error = e_conflicts_with_value_of_listchars;
     else
     {
@@ -5658,12 +5658,12 @@ f_setcellwidths(typval_T *argvars, typval_T *rettv UNUSED)
 
 	FOR_ALL_TAB_WINDOWS(tp, wp)
 	{
-	    if (set_chars_option(wp, &wp->w_p_lcs) != NULL)
+	    if (set_chars_option(wp, &wp->w_p_lcs, FALSE) != NULL)
 	    {
 		error = e_conflicts_with_value_of_listchars;
 		break;
 	    }
-	    if (set_chars_option(wp, &wp->w_p_fcs) != NULL)
+	    if (set_chars_option(wp, &wp->w_p_fcs, FALSE) != NULL)
 	    {
 		error = e_conflicts_with_value_of_fillchars;
 		break;
diff --git a/src/option.c b/src/option.c
index dd4ed1b4f..1b4b9da0d 100644
--- a/src/option.c
+++ b/src/option.c
@@ -2433,10 +2433,10 @@ didset_options2(void)
     check_opt_wim();
 
     // Parse default for 'listchars'.
-    (void)set_chars_option(curwin, &curwin->w_p_lcs);
+    (void)set_chars_option(curwin, &curwin->w_p_lcs, TRUE);
 
     // Parse default for 'fillchars'.
-    (void)set_chars_option(curwin, &curwin->w_p_fcs);
+    (void)set_chars_option(curwin, &curwin->w_p_fcs, TRUE);
 
 #ifdef FEAT_CLIPBOARD
     // Parse default for 'clipboard'
@@ -5204,12 +5204,12 @@ unset_global_local_option(char_u *name, void *from)
 	    break;
 	case PV_LCS:
 	    clear_string_option(&((win_T *)from)->w_p_lcs);
-	    set_chars_option((win_T *)from, &((win_T *)from)->w_p_lcs);
+	    set_chars_option((win_T *)from, &((win_T *)from)->w_p_lcs, TRUE);
 	    redraw_later(NOT_VALID);
 	    break;
 	case PV_FCS:
 	    clear_string_option(&((win_T *)from)->w_p_fcs);
-	    set_chars_option((win_T *)from, &((win_T *)from)->w_p_fcs);
+	    set_chars_option((win_T *)from, &((win_T *)from)->w_p_fcs, TRUE);
 	    redraw_later(NOT_VALID);
 	    break;
 	case PV_VE:
@@ -5607,8 +5607,8 @@ after_copy_winopt(win_T *wp)
     fill_culopt_flags(NULL, wp);
     check_colorcolumn(wp);
 #endif
-    set_chars_option(wp, &wp->w_p_lcs);
-    set_chars_option(wp, &wp->w_p_fcs);
+    set_chars_option(wp, &wp->w_p_lcs, TRUE);
+    set_chars_option(wp, &wp->w_p_fcs, TRUE);
 }
 
     static char_u *
diff --git a/src/optionstr.c b/src/optionstr.c
index 1f9a0e5d5..5147a2120 100644
--- a/src/optionstr.c
+++ b/src/optionstr.c
@@ -866,7 +866,7 @@ did_set_string_option(
     {
 	if (check_opt_strings(p_ambw, p_ambw_values, FALSE) != OK)
 	    errmsg = e_invalid_argument;
-	else if (set_chars_option(curwin, &p_fcs) != NULL)
+	else if (set_chars_option(curwin, &p_fcs, FALSE) != NULL)
 	    errmsg = e_conflicts_with_value_of_fillchars;
 	else
 	{
@@ -875,7 +875,7 @@ did_set_string_option(
 
 	    FOR_ALL_TAB_WINDOWS(tp, wp)
 	    {
-		if (set_chars_option(wp, &wp->w_p_lcs) != NULL)
+		if (set_chars_option(wp, &wp->w_p_lcs, FALSE) != NULL)
 		{
 		    errmsg = e_conflicts_with_value_of_listchars;
 		    goto ambw_end;
@@ -1304,60 +1304,47 @@ ambw_end:
 	}
     }
 
-    // global 'listchars'
-    else if (varp == &p_lcs)
+    // global 'listchars' or 'fillchars'
+    else if (varp == &p_lcs || varp == &p_fcs)
     {
-	errmsg = set_chars_option(curwin, varp);
+	char_u **local_ptr = varp == &p_lcs
+					 ? &curwin->w_p_lcs : &curwin->w_p_fcs;
+
+	// only apply the global value to "curwin" when it does not have a
+	// local value
+	errmsg = set_chars_option(curwin, varp,
+			      **local_ptr == NUL || !(opt_flags & OPT_GLOBAL));
 	if (errmsg == NULL)
 	{
 	    tabpage_T	*tp;
 	    win_T	*wp;
 
-	    // If the current window is set to use the global 'listchars'
-	    // value, clear the window-local value.
+	    // If the current window is set to use the global
+	    // 'listchars'/'fillchars' value, clear the window-local value.
 	    if (!(opt_flags & OPT_GLOBAL))
-		clear_string_option(&curwin->w_p_lcs);
+		clear_string_option(local_ptr);
 	    FOR_ALL_TAB_WINDOWS(tp, wp)
+	    {
 		// If the current window has a local value need to apply it
 		// again, it was changed when setting the global value.
 		// If no error was returned above, we don't expect an error
 		// here, so ignore the return value.
-		(void)set_chars_option(wp, &wp->w_p_lcs);
+		local_ptr = varp == &p_lcs ? &wp->w_p_lcs : &wp->w_p_fcs;
+		if (**local_ptr == NUL)
+		    (void)set_chars_option(wp, local_ptr, TRUE);
+	    }
 
 	    redraw_all_later(NOT_VALID);
 	}
     }
     // local 'listchars'
     else if (varp == &curwin->w_p_lcs)
-	errmsg = set_chars_option(curwin, varp);
-
-    // 'fillchars'
-    else if (varp == &p_fcs)
-    {
-	errmsg = set_chars_option(curwin, varp);
-	if (errmsg == NULL)
-	{
-	    tabpage_T	*tp;
-	    win_T	*wp;
+	errmsg = set_chars_option(curwin, varp, TRUE);
 
-	    // If the current window is set to use the global 'fillchars'
-	    // value clear the window-local value.
-	    if (!(opt_flags & OPT_GLOBAL))
-		clear_string_option(&curwin->w_p_fcs);
-	    FOR_ALL_TAB_WINDOWS(tp, wp)
-		// If the current window has a local value need to apply it
-		// again, it was changed when setting the global value.
-		// If no error was returned above, we don't expect an error
-		// here, so ignore the return value.
-		(void)set_chars_option(wp, &wp->w_p_fcs);
-
-	    redraw_all_later(NOT_VALID);
-	}
-    }
     // local 'fillchars'
     else if (varp == &curwin->w_p_fcs)
     {
-	errmsg = set_chars_option(curwin, varp);
+	errmsg = set_chars_option(curwin, varp, TRUE);
     }
 
 #ifdef FEAT_CMDWIN
diff --git a/src/proto/screen.pro b/src/proto/screen.pro
index de42c43de..66fc0f3da 100644
--- a/src/proto/screen.pro
+++ b/src/proto/screen.pro
@@ -55,5 +55,5 @@ void comp_col(void);
 int number_width(win_T *wp);
 int screen_screencol(void);
 int screen_screenrow(void);
-char *set_chars_option(win_T *wp, char_u **varp);
+char *set_chars_option(win_T *wp, char_u **varp, int apply);
 /* vim: set ft=c : */
diff --git a/src/screen.c b/src/screen.c
index 1ddafb3b3..db251c0c4 100644
--- a/src/screen.c
+++ b/src/screen.c
@@ -4843,11 +4843,13 @@ get_encoded_char_adv(char_u **p)
 
 /*
  * Handle setting 'listchars' or 'fillchars'.
+ * "varp" points to either the global or the window-local value.
+ * When "apply" is FALSE do not store the flags, only check for errors.
  * Assume monocell characters.
  * Returns error message, NULL if it's OK.
  */
     char *
-set_chars_option(win_T *wp, char_u **varp)
+set_chars_option(win_T *wp, char_u **varp, int apply)
 {
     int	    round, i, len, len2, entries;
     char_u  *p, *s;
@@ -4856,11 +4858,16 @@ set_chars_option(win_T *wp, char_u **varp)
     char_u  *last_lmultispace = NULL; // Last occurrence of "leadmultispace:"
     int	    multispace_len = 0;	      // Length of lcs-multispace string
     int	    lead_multispace_len = 0;  // Length of lcs-leadmultispace string
+    int	    is_listchars = (varp == &p_lcs || varp == &wp->w_p_lcs);
+    char_u  *value = *varp;
+
     struct charstab
     {
 	int	*cp;
 	char	*name;
     };
+    struct charstab *tab;
+
     static fill_chars_T fill_chars;
     static struct charstab filltab[] =
     {
@@ -4874,6 +4881,7 @@ set_chars_option(win_T *wp, char_u **varp)
 	{&fill_chars.diff,	"diff"},
 	{&fill_chars.eob,	"eob"},
     };
+
     static lcs_chars_T lcs_chars;
     struct charstab lcstab[] =
     {
@@ -4891,22 +4899,21 @@ set_chars_option(win_T *wp, char_u **varp)
 	{NULL,			"conceal"},
 #endif
     };
-    struct charstab *tab;
 
-    if (varp == &p_lcs || varp == &wp->w_p_lcs)
+    if (is_listchars)
     {
 	tab = lcstab;
 	CLEAR_FIELD(lcs_chars);
 	entries = ARRAY_LENGTH(lcstab);
 	if (varp == &wp->w_p_lcs && wp->w_p_lcs[0] == NUL)
-	    varp = &p_lcs;
+	    value = p_lcs;  // local value is empty, us the global value
     }
     else
     {
 	tab = filltab;
 	entries = ARRAY_LENGTH(filltab);
 	if (varp == &wp->w_p_fcs && wp->w_p_fcs[0] == NUL)
-	    varp = &p_fcs;
+	    value = p_fcs;  // local value is empty, us the global value
     }
 
     // first round: check for valid value, second round: assign values
@@ -4915,7 +4922,7 @@ set_chars_option(win_T *wp, char_u **varp)
 	if (round > 0)
 	{
 	    // After checking that the value is valid: set defaults.
-	    if (varp == &p_lcs || varp == &wp->w_p_lcs)
+	    if (is_listchars)
 	    {
 		for (i = 0; i < entries; ++i)
 		    if (tab[i].cp != NULL)
@@ -4926,7 +4933,8 @@ set_chars_option(win_T *wp, char_u **varp)
 		if (multispace_len > 0)
 		{
 		    lcs_chars.multispace = ALLOC_MULT(int, multispace_len + 1);
-		    lcs_chars.multispace[multispace_len] = NUL;
+		    if (lcs_chars.multispace != NULL)
+			lcs_chars.multispace[multispace_len] = NUL;
 		}
 		else
 		    lcs_chars.multispace = NULL;
@@ -4953,7 +4961,7 @@ set_chars_option(win_T *wp, char_u **varp)
 		fill_chars.eob = '~';
 	    }
 	}
-	p = *varp;
+	p = value;
 	while (*p)
 	{
 	    for (i = 0; i < entries; ++i)
@@ -5007,7 +5015,7 @@ set_chars_option(win_T *wp, char_u **varp)
 	    {
 		len = (int)STRLEN("multispace");
 		len2 = (int)STRLEN("leadmultispace");
-		if ((varp == &p_lcs || varp == &wp->w_p_lcs)
+		if (is_listchars
 			&& STRNCMP(p, "multispace", len) == 0
 			&& p[len] == ':'
 			&& p[len + 1] != NUL)
@@ -5044,7 +5052,7 @@ set_chars_option(win_T *wp, char_u **varp)
 		    }
 		}
 
-		else if ((varp == &p_lcs || varp == &wp->w_p_lcs)
+		else if (is_listchars
 			&& STRNCMP(p, "leadmultispace", len2) == 0
 			&& p[len2] == ':'
 			&& p[len2 + 1] != NUL)
@@ -5090,15 +5098,23 @@ set_chars_option(win_T *wp, char_u **varp)
 	}
     }
 
-    if (tab == lcstab)
+    if (apply)
     {
-	vim_free(wp->w_lcs_chars.multispace);
-	vim_free(wp->w_lcs_chars.leadmultispace);
-	wp->w_lcs_chars = lcs_chars;
+	if (is_listchars)
+	{
+	    vim_free(wp->w_lcs_chars.multispace);
+	    vim_free(wp->w_lcs_chars.leadmultispace);
+	    wp->w_lcs_chars = lcs_chars;
+	}
+	else
+	{
+	    wp->w_fill_chars = fill_chars;
+	}
     }
-    else
+    else if (is_listchars)
     {
-	wp->w_fill_chars = fill_chars;
+	vim_free(lcs_chars.multispace);
+	vim_free(lcs_chars.leadmultispace);
     }
 
     return NULL;	// no error
diff --git a/src/version.c b/src/version.c
index b2c63c444..ed2caddd6 100644
--- a/src/version.c
+++ b/src/version.c
@@ -736,6 +736,8 @@ static char *(features[]) =
 static int included_patches[] =
 {   /* Add new patch number below this line */
 /**/
+    40,
+/**/
     39,
 /**/
     38,