patch 9.0.0040: use of set_chars_option() is confusing

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

diff --git a/src/mbyte.c b/src/mbyte.c
index 1e3ef72a3633af8996633540eaa032b5642bd078..691c1608e3a1d675d49f2bfe4bfd9f6bf72cdc60 100644 (file)
--- 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 dd4ed1b4f52b8cf87595361ffa51f671c8f70c42..1b4b9da0d328877ced11ca8bb5bf6b7aaffda6a0 100644 (file)
--- 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 1f9a0e5d5213de857c8509a0c052dc67b428a24e..5147a2120cf677f5c2ed63223d44fed9667a4981 100644 (file)
--- 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 de42c43de47be44d55c32443d2444261c606cb52..66fc0f3da23eeb8ac2dd715cf0368e6f9d1c1503 100644 (file)
--- 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 1ddafb3b37e486b89bfc10f1f2a9b3547c7eec51..db251c0c49813e0e805835ad448f1758dab2228f 100644 (file)
--- 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 b2c63c4441e140880bf2fef1400fcead071bb6a2..ed2caddd6f5a0b4b30a4c2d40a940f6f084887ec 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -735,6 +735,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    40,
 /**/
     39,
 /**/