vim-patch:9.1.0803: tests: no error check when setting global 'isk'

Problem:  tests: no error check when setting global 'isk'
Solution: also parse and check global 'isk' value (Milly)

closes: vim/vim#15915

5e7a6a4a10

Co-authored-by: Milly <milly.ca@gmail.com>

src/nvim/charset.c

@@ -88,13 +88,11 @@ int init_chartab(void)
 /// an error, OK otherwise.
 int buf_init_chartab(buf_T *buf, bool global)
 {
-  int c;
-
   if (global) {
     // Set the default size for printable characters:
     // From <Space> to '~' is 1 (printable), others are 2 (not printable).
     // This also inits all 'isident' and 'isfname' flags to false.
-    c = 0;
+    int c = 0;
 
     while (c < ' ') {
       g_chartab[c++] = (dy_flags & DY_UHEX) ? 4 : 2;
@@ -124,9 +122,7 @@ int buf_init_chartab(buf_T *buf, bool global)
     SET_CHARTAB(buf, '-');
   }
 
-  // Walk through the 'isident', 'iskeyword', 'isfname' and 'isprint'
+  // Walk through the 'isident', 'iskeyword', 'isfname' and 'isprint' options.
-  // options Each option is a list of characters, character numbers or
-  // ranges, separated by commas, e.g.: "200-210,x,#-178,-"
   for (int i = global ? 0 : 3; i <= 3; i++) {
     const char *p;
     if (i == 0) {
@@ -142,113 +138,133 @@ int buf_init_chartab(buf_T *buf, bool global)
       // fourth round: 'iskeyword'
       p = buf->b_p_isk;
     }
-
+    if (parse_isopt(p, buf, false) == FAIL) {
-    while (*p) {
+      return FAIL;
-      bool tilde = false;
-      bool do_isalpha = false;
-
-      if ((*p == '^') && (p[1] != NUL)) {
-        tilde = true;
-        p++;
-      }
-
-      if (ascii_isdigit(*p)) {
-        c = getdigits_int((char **)&p, true, 0);
-      } else {
-        c = mb_ptr2char_adv(&p);
-      }
-      int c2 = -1;
-
-      if ((*p == '-') && (p[1] != NUL)) {
-        p++;
-
-        if (ascii_isdigit(*p)) {
-          c2 = getdigits_int((char **)&p, true, 0);
-        } else {
-          c2 = mb_ptr2char_adv(&p);
-        }
-      }
-
-      if ((c <= 0)
-          || (c >= 256)
-          || ((c2 < c) && (c2 != -1))
-          || (c2 >= 256)
-          || !((*p == NUL) || (*p == ','))) {
-        return FAIL;
-      }
-
-      if (c2 == -1) {  // not a range
-        // A single '@' (not "@-@"):
-        // Decide on letters being ID/printable/keyword chars with
-        // standard function isalpha(). This takes care of locale for
-        // single-byte characters).
-        if (c == '@') {
-          do_isalpha = true;
-          c = 1;
-          c2 = 255;
-        } else {
-          c2 = c;
-        }
-      }
-
-      while (c <= c2) {
-        // Use the MB_ functions here, because isalpha() doesn't
-        // work properly when 'encoding' is "latin1" and the locale is
-        // "C".
-        if (!do_isalpha
-            || mb_islower(c)
-            || mb_isupper(c)) {
-          if (i == 0) {
-            // (re)set ID flag
-            if (tilde) {
-              g_chartab[c] &= (uint8_t) ~CT_ID_CHAR;
-            } else {
-              g_chartab[c] |= CT_ID_CHAR;
-            }
-          } else if (i == 1) {
-            // (re)set printable
-            if (c < ' ' || c > '~') {
-              if (tilde) {
-                g_chartab[c] = (uint8_t)((g_chartab[c] & ~CT_CELL_MASK)
-                                         + ((dy_flags & DY_UHEX) ? 4 : 2));
-                g_chartab[c] &= (uint8_t) ~CT_PRINT_CHAR;
-              } else {
-                g_chartab[c] = (uint8_t)((g_chartab[c] & ~CT_CELL_MASK) + 1);
-                g_chartab[c] |= CT_PRINT_CHAR;
-              }
-            }
-          } else if (i == 2) {
-            // (re)set fname flag
-            if (tilde) {
-              g_chartab[c] &= (uint8_t) ~CT_FNAME_CHAR;
-            } else {
-              g_chartab[c] |= CT_FNAME_CHAR;
-            }
-          } else {  // i == 3
-            // (re)set keyword flag
-            if (tilde) {
-              RESET_CHARTAB(buf, c);
-            } else {
-              SET_CHARTAB(buf, c);
-            }
-          }
-        }
-        c++;
-      }
-
-      c = (uint8_t)(*p);
-      p = skip_to_option_part(p);
-
-      if ((c == ',') && (*p == NUL)) {
-        // Trailing comma is not allowed.
-        return FAIL;
-      }
     }
   }
+
   chartab_initialized = true;
   return OK;
 }
 
+/// Checks the format for the option settings 'iskeyword', 'isident', 'isfname'
+/// or 'isprint'.
+/// Returns FAIL if has an error, OK otherwise.
+int check_isopt(char *var)
+{
+  return parse_isopt(var, NULL, true);
+}
+
+/// @param only_check  if false: refill g_chartab[]
+static int parse_isopt(const char *var, buf_T *buf, bool only_check)
+{
+  const char *p = var;
+
+  // Parses the 'isident', 'iskeyword', 'isfname' and 'isprint' options.
+  // Each option is a list of characters, character numbers or ranges,
+  // separated by commas, e.g.: "200-210,x,#-178,-"
+  while (*p) {
+    bool tilde = false;
+    bool do_isalpha = false;
+
+    if (*p == '^' && p[1] != NUL) {
+      tilde = true;
+      p++;
+    }
+
+    int c;
+    if (ascii_isdigit(*p)) {
+      c = getdigits_int((char **)&p, true, 0);
+    } else {
+      c = mb_ptr2char_adv(&p);
+    }
+    int c2 = -1;
+
+    if (*p == '-' && p[1] != NUL) {
+      p++;
+
+      if (ascii_isdigit(*p)) {
+        c2 = getdigits_int((char **)&p, true, 0);
+      } else {
+        c2 = mb_ptr2char_adv(&p);
+      }
+    }
+
+    if (c <= 0 || c >= 256 || (c2 < c && c2 != -1) || c2 >= 256
+        || !(*p == NUL || *p == ',')) {
+      return FAIL;
+    }
+
+    bool trail_comma = *p == ',';
+    p = skip_to_option_part(p);
+    if (trail_comma && *p == NUL) {
+      // Trailing comma is not allowed.
+      return FAIL;
+    }
+
+    if (only_check) {
+      continue;
+    }
+
+    if (c2 == -1) {  // not a range
+      // A single '@' (not "@-@"):
+      // Decide on letters being ID/printable/keyword chars with
+      // standard function isalpha(). This takes care of locale for
+      // single-byte characters).
+      if (c == '@') {
+        do_isalpha = true;
+        c = 1;
+        c2 = 255;
+      } else {
+        c2 = c;
+      }
+    }
+
+    while (c <= c2) {
+      // Use the MB_ functions here, because isalpha() doesn't
+      // work properly when 'encoding' is "latin1" and the locale is
+      // "C".
+      if (!do_isalpha
+          || mb_islower(c)
+          || mb_isupper(c)) {
+        if (var == p_isi) {  // (re)set ID flag
+          if (tilde) {
+            g_chartab[c] &= (uint8_t) ~CT_ID_CHAR;
+          } else {
+            g_chartab[c] |= CT_ID_CHAR;
+          }
+        } else if (var == p_isp) {  // (re)set printable
+          if (c < ' ' || c > '~') {
+            if (tilde) {
+              g_chartab[c] = (uint8_t)((g_chartab[c] & ~CT_CELL_MASK)
+                                       + ((dy_flags & DY_UHEX) ? 4 : 2));
+              g_chartab[c] &= (uint8_t) ~CT_PRINT_CHAR;
+            } else {
+              g_chartab[c] = (uint8_t)((g_chartab[c] & ~CT_CELL_MASK) + 1);
+              g_chartab[c] |= CT_PRINT_CHAR;
+            }
+          }
+        } else if (var == p_isf) {  // (re)set fname flag
+          if (tilde) {
+            g_chartab[c] &= (uint8_t) ~CT_FNAME_CHAR;
+          } else {
+            g_chartab[c] |= CT_FNAME_CHAR;
+          }
+        } else {  // (var == p_isk || var == buf->b_p_isk) (re)set keyword flag
+          if (tilde) {
+            RESET_CHARTAB(buf, c);
+          } else {
+            SET_CHARTAB(buf, c);
+          }
+        }
+      }
+      c++;
+    }
+  }
+
+  return OK;
+}
+
 /// Translate any special characters in buf[bufsize] in-place.
 ///
 /// The result is a string with only printable characters, but if there is not

src/nvim/options.lua

@@ -4459,7 +4459,7 @@ return {
     {
       abbreviation = 'isk',
       alloced = true,
-      cb = 'did_set_isopt',
+      cb = 'did_set_iskeyword',
       defaults = { if_true = '@,48-57,_,192-255' },
       deny_duplicates = true,
       desc = [=[

src/nvim/optionstr.c

@@ -1572,12 +1572,28 @@ int expand_set_inccommand(optexpand_T *args, int *numMatches, char ***matches)
                                matches);
 }
 
+/// The 'iskeyword' option is changed.
+const char *did_set_iskeyword(optset_T *args)
+{
+  char **varp = (char **)args->os_varp;
+
+  if (varp == &p_isk) {       // only check for global-value
+    if (check_isopt(*varp) == FAIL) {
+      return e_invarg;
+    }
+  } else {                    // fallthrough for local-value
+    return did_set_isopt(args);
+  }
+
+  return NULL;
+}
+
 /// The 'isident' or the 'iskeyword' or the 'isprint' or the 'isfname' option is
 /// changed.
 const char *did_set_isopt(optset_T *args)
 {
   buf_T *buf = (buf_T *)args->os_buf;
-  // 'isident', 'iskeyword', 'isprint or 'isfname' option: refill g_chartab[]
+  // 'isident', 'iskeyword', 'isprint' or 'isfname' option: refill g_chartab[]
   // If the new option is invalid, use old value.
   // 'lisp' option: refill g_chartab[] for '-' char
   if (buf_init_chartab(buf, true) == FAIL) {

test/old/testdir/gen_opt_test.vim

@@ -49,7 +49,6 @@ let skip_setglobal_reasons = #{
       \ colorcolumn:	'TODO: fix missing error handling for setglobal',
       \ conceallevel:	'TODO: fix missing error handling for setglobal',
       \ foldcolumn:	'TODO: fix missing error handling for setglobal',
-      \ iskeyword:	'TODO: fix missing error handling for setglobal',
       \ numberwidth:	'TODO: fix missing error handling for setglobal',
       \ scrolloff:	'TODO: fix missing error handling for setglobal',
       \ shiftwidth:	'TODO: fix missing error handling for setglobal',