patch 9.1.0039: too vague errors for 'listchars'/'fillchars'

Problem:  too vague errors for 'listchars'/'fillchars'
Solution: Include the field name in error message.
          (zeertzjq)

related: neovim/neovim#27050
closes: #13877

Co-authored-by: Cole Frankenhoff <cole.nhf@gmail.com>
Signed-off-by: zeertzjq <zeertzjq@outlook.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>

runtime/doc/options.txt

@@ -1,4 +1,4 @@
-*options.txt*	For Vim version 9.1.  Last change: 2024 Jan 03
+*options.txt*	For Vim version 9.1.  Last change: 2024 Jan 17
 
 
 		  VIM REFERENCE MANUAL	  by Bram Moolenaar
@@ -3478,12 +3478,10 @@ A jump table for the options with a short description can be found at |Q_op|.
 						*'fillchars'* *'fcs'*
 'fillchars' 'fcs'	string	(default "vert:|,fold:-,eob:~")
 			global or local to window |global-local|
-			{not available when compiled without the |+folding|
-			feature}
 	Characters to fill the statuslines, vertical separators and special
 	lines in the window.
 	It is a comma-separated list of items.  Each item has a name, a colon
-	and the value of that item:
+	and the value of that item: |E1511|
 
 	  item name	default		Used for ~
 	  stl		' '		statusline of the current window
@@ -3504,7 +3502,7 @@ A jump table for the options with a short description can be found at |Q_op|.
 <
 	For the "stl", "stlnc", "foldopen", "foldclose" and "foldsep" items
 	single-byte and multibyte characters are supported.  But double-width
-	characters are not supported.
+	characters are not supported. |E1512|
 
 	The highlighting used for these items:
 	  item name	highlight group ~
@@ -5243,7 +5241,7 @@ A jump table for the options with a short description can be found at |Q_op|.
 'listchars' 'lcs'	string	(default "eol:$")
 			global or local to window |global-local|
 	Strings to use in 'list' mode and for the |:list| command.  It is a
-	comma-separated list of string settings.
+	comma-separated list of string settings. *E1511*
 							*lcs-eol*
 	  eol:c		Character to show at the end of each line.  When
 			omitted, there is no extra character at the end of the
@@ -5319,7 +5317,7 @@ A jump table for the options with a short description can be found at |Q_op|.
 
 	The characters ':' and ',' should not be used.  UTF-8 characters can
 	be used when 'encoding' is "utf-8", otherwise only printable
-	characters are allowed.  All characters must be single width.
+	characters are allowed.  All characters must be single width. *E1512*
 
 	Each character can be specified as hex: >
 		set listchars=eol:\\x24

runtime/doc/tags

@@ -4536,6 +4536,8 @@ E1508	editing.txt	/*E1508*
 E1509	editing.txt	/*E1509*
 E151	helphelp.txt	/*E151*
 E1510	change.txt	/*E1510*
+E1511	options.txt	/*E1511*
+E1512	options.txt	/*E1512*
 E152	helphelp.txt	/*E152*
 E153	helphelp.txt	/*E153*
 E154	helphelp.txt	/*E154*

src/errors.h

@@ -3602,3 +3602,7 @@ EXTERN char e_xattr_other[]
 	INIT(= N_("E1509: Error occurred when reading or writing extended attribute"));
 EXTERN char e_val_too_large[]
 	INIT(= N_("E1510: Value too large: %s"));
+EXTERN char e_wrong_number_of_characters_for_field_str[]
+	INIT(= N_("E1511: Wrong number of characters for field \"%s\""));
+EXTERN char e_wrong_character_width_for_field_str[]
+	INIT(= N_("E1512: Wrong character width for field \"%s\""));

src/option.c

@@ -2858,10 +2858,10 @@ didset_options2(void)
     check_opt_wim();
 
     // Parse default for 'listchars'.
-    (void)set_listchars_option(curwin, curwin->w_p_lcs, TRUE);
+    (void)set_listchars_option(curwin, curwin->w_p_lcs, TRUE, NULL, 0);
 
     // Parse default for 'fillchars'.
-    (void)set_fillchars_option(curwin, curwin->w_p_fcs, TRUE);
+    (void)set_fillchars_option(curwin, curwin->w_p_fcs, TRUE, NULL, 0);
 
 #ifdef FEAT_CLIPBOARD
     // Parse default for 'clipboard'
@@ -6219,12 +6219,14 @@ unset_global_local_option(char_u *name, void *from)
 	    break;
 	case PV_LCS:
 	    clear_string_option(&((win_T *)from)->w_p_lcs);
-	    set_listchars_option((win_T *)from, ((win_T *)from)->w_p_lcs, TRUE);
+	    set_listchars_option((win_T *)from, ((win_T *)from)->w_p_lcs, TRUE,
+								      NULL, 0);
 	    redraw_later(UPD_NOT_VALID);
 	    break;
 	case PV_FCS:
 	    clear_string_option(&((win_T *)from)->w_p_fcs);
-	    set_fillchars_option((win_T *)from, ((win_T *)from)->w_p_fcs, TRUE);
+	    set_fillchars_option((win_T *)from, ((win_T *)from)->w_p_fcs, TRUE,
+								      NULL, 0);
 	    redraw_later(UPD_NOT_VALID);
 	    break;
 	case PV_VE:
@@ -6630,8 +6632,8 @@ after_copy_winopt(win_T *wp)
     fill_culopt_flags(NULL, wp);
     check_colorcolumn(wp);
 #endif
-    set_listchars_option(wp, wp->w_p_lcs, TRUE);
+    set_listchars_option(wp, wp->w_p_lcs, TRUE, NULL, 0);
-    set_fillchars_option(wp, wp->w_p_fcs, TRUE);
+    set_fillchars_option(wp, wp->w_p_fcs, TRUE, NULL, 0);
 }
 
     static char_u *

src/optionstr.c

@@ -233,7 +233,7 @@ illegal_char(char *errbuf, size_t errbuflen, int c)
 {
     if (errbuf == NULL)
 	return "";
-    vim_snprintf((char *)errbuf, errbuflen, _(e_illegal_character_str),
+    vim_snprintf(errbuf, errbuflen, _(e_illegal_character_str),
 		    (char *)transchar(c));
     return errbuf;
 }
@@ -1350,7 +1350,8 @@ expand_set_clipboard(optexpand_T *args, int *numMatches, char_u ***matches)
  * The global 'listchars' or 'fillchars' option is changed.
  */
     static char *
-did_set_global_listfillchars(char_u *val, int opt_lcs, int opt_flags)
+did_set_global_listfillchars(char_u *val, int opt_lcs, int opt_flags,
+						char *errbuf, size_t errbuflen)
 {
     char	*errmsg = NULL;
     char_u	**local_ptr = opt_lcs ? &curwin->w_p_lcs : &curwin->w_p_fcs;
@@ -1359,10 +1360,12 @@ did_set_global_listfillchars(char_u *val, int opt_lcs, int opt_flags)
     // local value
     if (opt_lcs)
 	errmsg = set_listchars_option(curwin, val,
-		**local_ptr == NUL || !(opt_flags & OPT_GLOBAL));
+		**local_ptr == NUL || !(opt_flags & OPT_GLOBAL),
+							    errbuf, errbuflen);
     else
 	errmsg = set_fillchars_option(curwin, val,
-		**local_ptr == NUL || !(opt_flags & OPT_GLOBAL));
+		**local_ptr == NUL || !(opt_flags & OPT_GLOBAL),
+							    errbuf, errbuflen);
     if (errmsg != NULL)
 	return errmsg;
 
@@ -1382,12 +1385,12 @@ did_set_global_listfillchars(char_u *val, int opt_lcs, int opt_flags)
 	if (opt_lcs)
 	{
 	    if (*wp->w_p_lcs == NUL)
-		(void)set_listchars_option(wp, wp->w_p_lcs, TRUE);
+		(void)set_listchars_option(wp, wp->w_p_lcs, TRUE, NULL, 0);
 	}
 	else
 	{
 	    if (*wp->w_p_fcs == NUL)
-		(void)set_fillchars_option(wp, wp->w_p_fcs, TRUE);
+		(void)set_fillchars_option(wp, wp->w_p_fcs, TRUE, NULL, 0);
 	}
     }
 
@@ -1408,11 +1411,13 @@ did_set_chars_option(optset_T *args)
     if (   varp == &p_lcs		// global 'listchars'
 	|| varp == &p_fcs)		// global 'fillchars'
 	errmsg = did_set_global_listfillchars(*varp, varp == &p_lcs,
-							       args->os_flags);
+			  args->os_flags, args->os_errbuf, args->os_errbuflen);
     else if (varp == &curwin->w_p_lcs)	// local 'listchars'
-	errmsg = set_listchars_option(curwin, *varp, TRUE);
+	errmsg = set_listchars_option(curwin, *varp, TRUE,
+					  args->os_errbuf, args->os_errbuflen);
     else if (varp == &curwin->w_p_fcs)	// local 'fillchars'
-	errmsg = set_fillchars_option(curwin, *varp, TRUE);
+	errmsg = set_fillchars_option(curwin, *varp, TRUE,
+					  args->os_errbuf, args->os_errbuflen);
 
     return errmsg;
 }

src/proto/screen.pro

@@ -55,8 +55,8 @@ void comp_col(void);
 int number_width(win_T *wp);
 int screen_screencol(void);
 int screen_screenrow(void);
-char *set_fillchars_option(win_T *wp, char_u *val, int apply);
+char *set_fillchars_option(win_T *wp, char_u *val, int apply, char *errbuf, size_t errbuflen);
-char *set_listchars_option(win_T *wp, char_u *val, int apply);
+char *set_listchars_option(win_T *wp, char_u *val, int apply, char *errbuf, size_t errbuflen);
 char_u *get_fillchars_name(expand_T *xp, int idx);
 char_u *get_listchars_name(expand_T *xp, int idx);
 char *check_chars_options(void);

src/screen.c

@@ -4690,6 +4690,15 @@ static struct charstab lcstab[] =
     {NULL,			"leadmultispace"},
 };
 
+    static char *
+field_value_err(char *errbuf, size_t errbuflen, char *fmt, char *field)
+{
+    if (errbuf == NULL)
+	return "";
+    vim_snprintf(errbuf, errbuflen, _(fmt), field);
+    return errbuf;
+}
+
 /*
  * Handle setting 'listchars' or 'fillchars'.
  * "value" points to either the global or the window-local value.
@@ -4699,7 +4708,8 @@ static struct charstab lcstab[] =
  * Returns error message, NULL if it's OK.
  */
     static char *
-set_chars_option(win_T *wp, char_u *value, int is_listchars, int apply)
+set_chars_option(win_T *wp, char_u *value, int is_listchars, int apply,
+						char *errbuf, size_t errbuflen)
 {
     int	    round, i, len, entries;
     char_u  *p, *s;
@@ -4779,9 +4789,7 @@ set_chars_option(win_T *wp, char_u *value, int is_listchars, int apply)
 	    for (i = 0; i < entries; ++i)
 	    {
 		len = (int)STRLEN(tab[i].name);
-		if (!(STRNCMP(p, tab[i].name, len) == 0
+		if (!(STRNCMP(p, tab[i].name, len) == 0 && p[len] == ':'))
-			&& p[len] == ':'
-			&& p[len + 1] != NUL))
 		    continue;
 
 		if (is_listchars && strcmp(tab[i].name, "multispace") == 0)
@@ -4796,12 +4804,16 @@ set_chars_option(win_T *wp, char_u *value, int is_listchars, int apply)
 			{
 			    c1 = get_encoded_char_adv(&s);
 			    if (char2cells(c1) > 1)
-				return e_invalid_argument;
+				return field_value_err(errbuf, errbuflen,
+					 e_wrong_character_width_for_field_str,
+					 tab[i].name);
 			    ++multispace_len;
 			}
 			if (multispace_len == 0)
 			    // lcs-multispace cannot be an empty string
-			    return e_invalid_argument;
+			    return field_value_err(errbuf, errbuflen,
+				    e_wrong_number_of_characters_for_field_str,
+				    tab[i].name);
 			p = s;
 		    }
 		    else
@@ -4832,12 +4844,16 @@ set_chars_option(win_T *wp, char_u *value, int is_listchars, int apply)
 			{
 			    c1 = get_encoded_char_adv(&s);
 			    if (char2cells(c1) > 1)
-				return e_invalid_argument;
+				return field_value_err(errbuf, errbuflen,
+					 e_wrong_character_width_for_field_str,
+					 tab[i].name);
 			    ++lead_multispace_len;
 			}
 			if (lead_multispace_len == 0)
 			    // lcs-leadmultispace cannot be an empty string
-			    return e_invalid_argument;
+			    return field_value_err(errbuf, errbuflen,
+				    e_wrong_number_of_characters_for_field_str,
+				    tab[i].name);
 			p = s;
 		    }
 		    else
@@ -4857,21 +4873,33 @@ set_chars_option(win_T *wp, char_u *value, int is_listchars, int apply)
 
 		c2 = c3 = 0;
 		s = p + len + 1;
+		if (*s == NUL)
+		    return field_value_err(errbuf, errbuflen,
+				    e_wrong_number_of_characters_for_field_str,
+				    tab[i].name);
 		c1 = get_encoded_char_adv(&s);
 		if (char2cells(c1) > 1)
-		    return e_invalid_argument;
+		    return field_value_err(errbuf, errbuflen,
+					 e_wrong_character_width_for_field_str,
+					 tab[i].name);
 		if (tab[i].cp == &lcs_chars.tab2)
 		{
 		    if (*s == NUL)
-			return e_invalid_argument;
+			return field_value_err(errbuf, errbuflen,
+				    e_wrong_number_of_characters_for_field_str,
+				    tab[i].name);
 		    c2 = get_encoded_char_adv(&s);
 		    if (char2cells(c2) > 1)
-			return e_invalid_argument;
+			return field_value_err(errbuf, errbuflen,
+					 e_wrong_character_width_for_field_str,
+					 tab[i].name);
 		    if (!(*s == ',' || *s == NUL))
 		    {
 			c3 = get_encoded_char_adv(&s);
 			if (char2cells(c3) > 1)
-			    return e_invalid_argument;
+			    return field_value_err(errbuf, errbuflen,
+					 e_wrong_character_width_for_field_str,
+					 tab[i].name);
 		    }
 		}
 
@@ -4892,6 +4920,10 @@ set_chars_option(win_T *wp, char_u *value, int is_listchars, int apply)
 		    p = s;
 		    break;
 		}
+		else
+		    return field_value_err(errbuf, errbuflen,
+				    e_wrong_number_of_characters_for_field_str,
+				    tab[i].name);
 	    }
 
 	    if (i == entries)
@@ -4923,18 +4955,20 @@ set_chars_option(win_T *wp, char_u *value, int is_listchars, int apply)
  * Handle the new value of 'fillchars'.
  */
     char *
-set_fillchars_option(win_T *wp, char_u *val, int apply)
+set_fillchars_option(win_T *wp, char_u *val, int apply, char *errbuf,
+							      size_t errbuflen)
 {
-    return set_chars_option(wp, val, FALSE, apply);
+    return set_chars_option(wp, val, FALSE, apply, errbuf, errbuflen);
 }
 
 /*
  * Handle the new value of 'listchars'.
  */
     char *
-set_listchars_option(win_T *wp, char_u *val, int apply)
+set_listchars_option(win_T *wp, char_u *val, int apply, char *errbuf,
+							      size_t errbuflen)
 {
-    return set_chars_option(wp, val, TRUE, apply);
+    return set_chars_option(wp, val, TRUE, apply, errbuf, errbuflen);
 }
 
 /*
@@ -4974,15 +5008,15 @@ check_chars_options(void)
     tabpage_T   *tp;
     win_T	    *wp;
 
-    if (set_listchars_option(curwin, p_lcs, FALSE) != NULL)
+    if (set_listchars_option(curwin, p_lcs, FALSE, NULL, 0) != NULL)
 	return e_conflicts_with_value_of_listchars;
-    if (set_fillchars_option(curwin, p_fcs, FALSE) != NULL)
+    if (set_fillchars_option(curwin, p_fcs, FALSE, NULL, 0) != NULL)
 	return e_conflicts_with_value_of_fillchars;
     FOR_ALL_TAB_WINDOWS(tp, wp)
     {
-	if (set_listchars_option(wp, wp->w_p_lcs, FALSE) != NULL)
+	if (set_listchars_option(wp, wp->w_p_lcs, FALSE, NULL, 0) != NULL)
 	    return e_conflicts_with_value_of_listchars;
-	if (set_fillchars_option(wp, wp->w_p_fcs, FALSE) != NULL)
+	if (set_fillchars_option(wp, wp->w_p_fcs, FALSE, NULL, 0) != NULL)
 	    return e_conflicts_with_value_of_fillchars;
     }
     return NULL;

src/testdir/test_display.vim

@@ -252,12 +252,12 @@ func Test_eob_fillchars()
   " default value
   call assert_match('eob:\~', &fillchars)
   " invalid values
-  call assert_fails(':set fillchars=eob:', 'E474:')
+  call assert_fails(':set fillchars=eob:', 'E1511:')
-  call assert_fails(':set fillchars=eob:xy', 'E474:')
+  call assert_fails(':set fillchars=eob:xy', 'E1511:')
-  call assert_fails(':set fillchars=eob:\255', 'E474:')
+  call assert_fails(':set fillchars=eob:\255', 'E1511:')
-  call assert_fails(':set fillchars=eob:<ff>', 'E474:')
+  call assert_fails(':set fillchars=eob:<ff>', 'E1511:')
-  call assert_fails(":set fillchars=eob:\x01", 'E474:')
+  call assert_fails(":set fillchars=eob:\x01", 'E1512:')
-  call assert_fails(':set fillchars=eob:\\x01', 'E474:')
+  call assert_fails(':set fillchars=eob:\\x01', 'E1512:')
   " default is ~
   new
   redraw
@@ -411,14 +411,16 @@ func Run_Test_display_lastline(euro)
   call StopVimInTerminal(buf)
 endfunc
 
-func Test_display_lastline()
+func Test_display_lastline_dump()
   CheckScreendump
 
   call Run_Test_display_lastline('')
   call Run_Test_display_lastline('euro_')
+endfunc
 
-  call assert_fails(':set fillchars=lastline:', 'E474:')
+func Test_display_lastline_fails()
-  call assert_fails(':set fillchars=lastline:〇', 'E474:')
+  call assert_fails(':set fillchars=lastline:', 'E1511:')
+  call assert_fails(':set fillchars=lastline:〇', 'E1512:')
 endfunc
 
 func Test_display_long_lastline()

src/testdir/test_listchars.vim

@@ -429,52 +429,52 @@ func Test_listchars_invalid()
   call assert_fails('set listchars=leadmultispace', 'E474:')
 
   " Too short
-  call assert_fails('set listchars=space:', 'E474:')
+  call assert_fails('set listchars=space:', 'E1511:')
-  call assert_fails('set listchars=tab:x', 'E474:')
+  call assert_fails('set listchars=tab:x', 'E1511:')
-  call assert_fails('set listchars=multispace:', 'E474:')
+  call assert_fails('set listchars=multispace:', 'E1511:')
-  call assert_fails('set listchars=leadmultispace:', 'E474:')
+  call assert_fails('set listchars=leadmultispace:', 'E1511:')
 
   " One occurrence too short
-  call assert_fails('set listchars=space:,space:x', 'E474:')
+  call assert_fails('set listchars=space:x,space:', 'E1511:')
-  call assert_fails('set listchars=space:x,space:', 'E474:')
+  call assert_fails('set listchars=space:,space:x', 'E1511:')
-  call assert_fails('set listchars=tab:x,tab:xx', 'E474:')
+  call assert_fails('set listchars=tab:xx,tab:x', 'E1511:')
-  call assert_fails('set listchars=tab:xx,tab:x', 'E474:')
+  call assert_fails('set listchars=tab:x,tab:xx', 'E1511:')
-  call assert_fails('set listchars=multispace:,multispace:x', 'E474:')
+  call assert_fails('set listchars=multispace:,multispace:x', 'E1511:')
-  call assert_fails('set listchars=multispace:x,multispace:', 'E474:')
+  call assert_fails('set listchars=multispace:x,multispace:', 'E1511:')
-  call assert_fails('set listchars=leadmultispace:,leadmultispace:x', 'E474:')
+  call assert_fails('set listchars=leadmultispace:,leadmultispace:x', 'E1511:')
-  call assert_fails('set listchars=leadmultispace:x,leadmultispace:', 'E474:')
+  call assert_fails('set listchars=leadmultispace:x,leadmultispace:', 'E1511:')
 
   " Too long
-  call assert_fails('set listchars=space:xx', 'E474:')
+  call assert_fails('set listchars=space:xx', 'E1511:')
-  call assert_fails('set listchars=tab:xxxx', 'E474:')
+  call assert_fails('set listchars=tab:xxxx', 'E1511:')
 
   " Has double-width character
-  call assert_fails('set listchars=space:·', 'E474:')
+  call assert_fails('set listchars=space:·', 'E1512:')
-  call assert_fails('set listchars=tab:·x', 'E474:')
+  call assert_fails('set listchars=tab:·x', 'E1512:')
-  call assert_fails('set listchars=tab:x·', 'E474:')
+  call assert_fails('set listchars=tab:x·', 'E1512:')
-  call assert_fails('set listchars=tab:xx·', 'E474:')
+  call assert_fails('set listchars=tab:xx·', 'E1512:')
-  call assert_fails('set listchars=multispace:·', 'E474:')
+  call assert_fails('set listchars=multispace:·', 'E1512:')
-  call assert_fails('set listchars=multispace:xxx·', 'E474:')
+  call assert_fails('set listchars=multispace:xxx·', 'E1512:')
-  call assert_fails('set listchars=leadmultispace:·', 'E474:')
+  call assert_fails('set listchars=leadmultispace:·', 'E1512:')
-  call assert_fails('set listchars=leadmultispace:xxx·', 'E474:')
+  call assert_fails('set listchars=leadmultispace:xxx·', 'E1512:')
 
   " Has control character
-  call assert_fails("set listchars=space:\x01", 'E474:')
+  call assert_fails("set listchars=space:\x01", 'E1512:')
-  call assert_fails("set listchars=tab:\x01x", 'E474:')
+  call assert_fails("set listchars=tab:\x01x", 'E1512:')
-  call assert_fails("set listchars=tab:x\x01", 'E474:')
+  call assert_fails("set listchars=tab:x\x01", 'E1512:')
-  call assert_fails("set listchars=tab:xx\x01", 'E474:')
+  call assert_fails("set listchars=tab:xx\x01", 'E1512:')
-  call assert_fails("set listchars=multispace:\x01", 'E474:')
+  call assert_fails("set listchars=multispace:\x01", 'E1512:')
-  call assert_fails("set listchars=multispace:xxx\x01", 'E474:')
+  call assert_fails("set listchars=multispace:xxx\x01", 'E1512:')
-  call assert_fails('set listchars=space:\\x01', 'E474:')
+  call assert_fails('set listchars=space:\\x01', 'E1512:')
-  call assert_fails('set listchars=tab:\\x01x', 'E474:')
+  call assert_fails('set listchars=tab:\\x01x', 'E1512:')
-  call assert_fails('set listchars=tab:x\\x01', 'E474:')
+  call assert_fails('set listchars=tab:x\\x01', 'E1512:')
-  call assert_fails('set listchars=tab:xx\\x01', 'E474:')
+  call assert_fails('set listchars=tab:xx\\x01', 'E1512:')
-  call assert_fails('set listchars=multispace:\\x01', 'E474:')
+  call assert_fails('set listchars=multispace:\\x01', 'E1512:')
-  call assert_fails('set listchars=multispace:xxx\\x01', 'E474:')
+  call assert_fails('set listchars=multispace:xxx\\x01', 'E1512:')
-  call assert_fails("set listchars=leadmultispace:\x01", 'E474:')
+  call assert_fails("set listchars=leadmultispace:\x01", 'E1512:')
-  call assert_fails('set listchars=leadmultispace:\\x01', 'E474:')
+  call assert_fails('set listchars=leadmultispace:\\x01', 'E1512:')
-  call assert_fails("set listchars=leadmultispace:xxx\x01", 'E474:')
+  call assert_fails("set listchars=leadmultispace:xxx\x01", 'E1512:')
-  call assert_fails('set listchars=leadmultispace:xxx\\x01', 'E474:')
+  call assert_fails('set listchars=leadmultispace:xxx\\x01', 'E1512:')
 
   enew!
   set ambiwidth& listchars& ff&

src/version.c

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