To: vim_dev@googlegroups.com Subject: Patch 9.0.0040 Fcc: outbox From: Bram Moolenaar Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ------------ Patch 9.0.0040 Problem: Use of set_chars_option() is confusing. Solution: Add "apply" argument to store the result or not. Merge similar code. Files: src/screen.c, src/proto/screen.pro, src/mbyte.c, src/option.c, src/optionstr.c *** ../vim-9.0.0039/src/screen.c 2022-07-04 17:34:06.386292140 +0100 --- src/screen.c 2022-07-04 20:31:36.548865385 +0100 *************** *** 4843,4853 **** /* * Handle setting 'listchars' or 'fillchars'. * Assume monocell characters. * Returns error message, NULL if it's OK. */ char * ! set_chars_option(win_T *wp, char_u **varp) { int round, i, len, len2, entries; char_u *p, *s; --- 4843,4855 ---- /* * 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, int apply) { int round, i, len, len2, entries; char_u *p, *s; *************** *** 4856,4866 **** --- 4858,4873 ---- 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,4879 **** --- 4881,4887 ---- {&fill_chars.diff, "diff"}, {&fill_chars.eob, "eob"}, }; + static lcs_chars_T lcs_chars; struct charstab lcstab[] = { *************** *** 4891,4912 **** {NULL, "conceal"}, #endif }; - struct charstab *tab; ! if (varp == &p_lcs || varp == &wp->w_p_lcs) { 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; } else { tab = filltab; entries = ARRAY_LENGTH(filltab); if (varp == &wp->w_p_fcs && wp->w_p_fcs[0] == NUL) ! varp = &p_fcs; } // first round: check for valid value, second round: assign values --- 4899,4919 ---- {NULL, "conceal"}, #endif }; ! 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) ! 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) ! value = p_fcs; // local value is empty, us the global value } // first round: check for valid value, second round: assign values *************** *** 4915,4921 **** if (round > 0) { // After checking that the value is valid: set defaults. ! if (varp == &p_lcs || varp == &wp->w_p_lcs) { for (i = 0; i < entries; ++i) if (tab[i].cp != NULL) --- 4922,4928 ---- if (round > 0) { // After checking that the value is valid: set defaults. ! if (is_listchars) { for (i = 0; i < entries; ++i) if (tab[i].cp != NULL) *************** *** 4926,4932 **** if (multispace_len > 0) { lcs_chars.multispace = ALLOC_MULT(int, multispace_len + 1); ! lcs_chars.multispace[multispace_len] = NUL; } else lcs_chars.multispace = NULL; --- 4933,4940 ---- if (multispace_len > 0) { lcs_chars.multispace = ALLOC_MULT(int, multispace_len + 1); ! if (lcs_chars.multispace != NULL) ! lcs_chars.multispace[multispace_len] = NUL; } else lcs_chars.multispace = NULL; *************** *** 4953,4959 **** fill_chars.eob = '~'; } } ! p = *varp; while (*p) { for (i = 0; i < entries; ++i) --- 4961,4967 ---- fill_chars.eob = '~'; } } ! p = value; while (*p) { for (i = 0; i < entries; ++i) *************** *** 5007,5013 **** { len = (int)STRLEN("multispace"); len2 = (int)STRLEN("leadmultispace"); ! if ((varp == &p_lcs || varp == &wp->w_p_lcs) && STRNCMP(p, "multispace", len) == 0 && p[len] == ':' && p[len + 1] != NUL) --- 5015,5021 ---- { len = (int)STRLEN("multispace"); len2 = (int)STRLEN("leadmultispace"); ! if (is_listchars && STRNCMP(p, "multispace", len) == 0 && p[len] == ':' && p[len + 1] != NUL) *************** *** 5044,5050 **** } } ! else if ((varp == &p_lcs || varp == &wp->w_p_lcs) && STRNCMP(p, "leadmultispace", len2) == 0 && p[len2] == ':' && p[len2 + 1] != NUL) --- 5052,5058 ---- } } ! else if (is_listchars && STRNCMP(p, "leadmultispace", len2) == 0 && p[len2] == ':' && p[len2 + 1] != NUL) *************** *** 5090,5104 **** } } ! if (tab == lcstab) { ! 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; } return NULL; // no error --- 5098,5120 ---- } } ! if (apply) { ! 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 if (is_listchars) { ! vim_free(lcs_chars.multispace); ! vim_free(lcs_chars.leadmultispace); } return NULL; // no error *** ../vim-9.0.0039/src/proto/screen.pro 2022-07-04 17:34:06.386292140 +0100 --- src/proto/screen.pro 2022-07-04 21:00:47.616375669 +0100 *************** *** 55,59 **** int number_width(win_T *wp); int screen_screencol(void); int screen_screenrow(void); ! char *set_chars_option(win_T *wp, char_u **varp); /* vim: set ft=c : */ --- 55,59 ---- int number_width(win_T *wp); int screen_screencol(void); int screen_screenrow(void); ! char *set_chars_option(win_T *wp, char_u **varp, int apply); /* vim: set ft=c : */ *** ../vim-9.0.0039/src/mbyte.c 2022-07-04 17:34:06.386292140 +0100 --- src/mbyte.c 2022-07-04 21:01:21.256263851 +0100 *************** *** 5647,5655 **** // Check that the new value does not conflict with 'fillchars' or // 'listchars'. ! if (set_chars_option(curwin, &p_fcs) != NULL) error = e_conflicts_with_value_of_fillchars; ! else if (set_chars_option(curwin, &p_lcs) != NULL) error = e_conflicts_with_value_of_listchars; else { --- 5647,5655 ---- // Check that the new value does not conflict with 'fillchars' or // 'listchars'. ! if (set_chars_option(curwin, &p_fcs, FALSE) != NULL) error = e_conflicts_with_value_of_fillchars; ! else if (set_chars_option(curwin, &p_lcs, FALSE) != NULL) error = e_conflicts_with_value_of_listchars; else { *************** *** 5658,5669 **** FOR_ALL_TAB_WINDOWS(tp, wp) { ! if (set_chars_option(wp, &wp->w_p_lcs) != NULL) { error = e_conflicts_with_value_of_listchars; break; } ! if (set_chars_option(wp, &wp->w_p_fcs) != NULL) { error = e_conflicts_with_value_of_fillchars; break; --- 5658,5669 ---- FOR_ALL_TAB_WINDOWS(tp, wp) { ! 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, FALSE) != NULL) { error = e_conflicts_with_value_of_fillchars; break; *** ../vim-9.0.0039/src/option.c 2022-07-04 17:34:06.386292140 +0100 --- src/option.c 2022-07-04 20:33:24.191938927 +0100 *************** *** 2433,2442 **** check_opt_wim(); // Parse default for 'listchars'. ! (void)set_chars_option(curwin, &curwin->w_p_lcs); // Parse default for 'fillchars'. ! (void)set_chars_option(curwin, &curwin->w_p_fcs); #ifdef FEAT_CLIPBOARD // Parse default for 'clipboard' --- 2433,2442 ---- check_opt_wim(); // Parse default for 'listchars'. ! (void)set_chars_option(curwin, &curwin->w_p_lcs, TRUE); // Parse default for 'fillchars'. ! (void)set_chars_option(curwin, &curwin->w_p_fcs, TRUE); #ifdef FEAT_CLIPBOARD // Parse default for 'clipboard' *************** *** 5204,5215 **** 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); 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); redraw_later(NOT_VALID); break; case PV_VE: --- 5204,5215 ---- 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, 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, TRUE); redraw_later(NOT_VALID); break; case PV_VE: *************** *** 5607,5614 **** 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); } static char_u * --- 5607,5614 ---- fill_culopt_flags(NULL, wp); check_colorcolumn(wp); #endif ! set_chars_option(wp, &wp->w_p_lcs, TRUE); ! set_chars_option(wp, &wp->w_p_fcs, TRUE); } static char_u * *** ../vim-9.0.0039/src/optionstr.c 2022-07-04 18:05:48.333038691 +0100 --- src/optionstr.c 2022-07-04 21:00:33.296423415 +0100 *************** *** 866,872 **** { if (check_opt_strings(p_ambw, p_ambw_values, FALSE) != OK) errmsg = e_invalid_argument; ! else if (set_chars_option(curwin, &p_fcs) != NULL) errmsg = e_conflicts_with_value_of_fillchars; else { --- 866,872 ---- { if (check_opt_strings(p_ambw, p_ambw_values, FALSE) != OK) errmsg = e_invalid_argument; ! else if (set_chars_option(curwin, &p_fcs, FALSE) != NULL) errmsg = e_conflicts_with_value_of_fillchars; else { *************** *** 875,881 **** FOR_ALL_TAB_WINDOWS(tp, wp) { ! if (set_chars_option(wp, &wp->w_p_lcs) != NULL) { errmsg = e_conflicts_with_value_of_listchars; goto ambw_end; --- 875,881 ---- FOR_ALL_TAB_WINDOWS(tp, wp) { ! if (set_chars_option(wp, &wp->w_p_lcs, FALSE) != NULL) { errmsg = e_conflicts_with_value_of_listchars; goto ambw_end; *************** *** 1304,1363 **** } } ! // global 'listchars' ! else if (varp == &p_lcs) { ! errmsg = set_chars_option(curwin, varp); 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 (!(opt_flags & OPT_GLOBAL)) ! clear_string_option(&curwin->w_p_lcs); 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); 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; - // 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); } #ifdef FEAT_CMDWIN --- 1304,1350 ---- } } ! // global 'listchars' or 'fillchars' ! else if (varp == &p_lcs || varp == &p_fcs) { ! 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'/'fillchars' value, clear the window-local value. if (!(opt_flags & OPT_GLOBAL)) ! 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. ! 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, TRUE); // local 'fillchars' else if (varp == &curwin->w_p_fcs) { ! errmsg = set_chars_option(curwin, varp, TRUE); } #ifdef FEAT_CMDWIN *** ../vim-9.0.0039/src/version.c 2022-07-04 19:58:14.238360165 +0100 --- src/version.c 2022-07-04 21:02:28.280042266 +0100 *************** *** 737,738 **** --- 737,740 ---- { /* Add new patch number below this line */ + /**/ + 40, /**/ -- There is a difference between "should work" and "does work", it's called testing. /// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net \\\ /// \\\ \\\ sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ /// \\\ help me help AIDS victims -- http://ICCF-Holland.org ///