vim-patch:9.2.0223: Option handling for key:value suboptions is limited (#38426)

Problem:  Option handling for key:value suboptions is limited
Solution: Improve :set+=, :set-= and :set^= for options that use
          "key:value" pairs (Hirohito Higashi)

For comma-separated options with P_COLON (e.g., diffopt, listchars,
fillchars), :set += -= ^= now processes each comma-separated item
individually instead of treating the whole value as a single string.

For :set += and :set ^=:
- A "key:value" item where the key already exists with a different value:
  the old item is replaced.
- An exact duplicate item is left unchanged.
- A new item is appended (+=) or prepended (^=).

For :set -=:
- A "key:value" or "key:" item removes by key match regardless of value.
- A non-colon item removes by exact match.

This also handles multiple non-colon items (e.g., :set
diffopt-=filler,internal) by processing each item individually, making
the behavior order-independent.

Previously, :set += simply appended the value, causing duplicate keys to
accumulate.

fixes:  vim/vim#18495
closes: vim/vim#19783

e2f4e18437

Co-authored-by: Hirohito Higashi <h.east.727@gmail.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

runtime/doc/news.txt

@@ -384,6 +384,8 @@ OPTIONS
   previous highlight attributes)
 • 'messagesopt' added progress:c to control if progress messages are shown in
   cmdline.
+• Improved |:set+=|, |:set^=| and |:set-=| handling of comma-separated "key:value"
+  pairs (e.g. 'listchars', 'fillchars', 'diffopt').
 
 PERFORMANCE
 

runtime/doc/options.txt

@@ -78,6 +78,17 @@ achieve special effects.  These options come in three forms:
 			If the option is a list of flags, superfluous flags
 			are removed.  When adding a flag that was already
 			present the option value doesn't change.
+			When the option supports "key:value" items and {value}
+			contains a "key:value" item or multiple
+			comma-separated items, each item is processed
+			individually:
+			- A "key:value" item where the key already exists with
+			  a different value: the old item is removed and the
+			  new item is appended to the end.
+			- A "key:value" item that is an exact duplicate is
+			  left unchanged.
+			- Other items that already exist are left unchanged.
+			- New items are appended to the end.
 			Also see |:set-args| above.
 
 :se[t] {option}^={value}				*:set^=*
@@ -85,6 +96,11 @@ achieve special effects.  These options come in three forms:
 			the {value} to a string option.  When the option is a
 			comma-separated list, a comma is added, unless the
 			value was empty.
+			When the option supports "key:value" items and {value}
+			contains a "key:value" item or multiple
+			comma-separated items, each item is processed
+			individually.  Works like |:set+=| but new items are
+			prepended to the beginning instead of appended.
 			Also see |:set-args| above.
 
 :se[t] {option}-={value}				*:set-=*
@@ -97,6 +113,12 @@ achieve special effects.  These options come in three forms:
 			When the option is a list of flags, {value} must be
 			exactly as they appear in the option.  Remove flags
 			one by one to avoid problems.
+			When the option supports "key:value" items and {value}
+			contains a "key:value" item or multiple
+			comma-separated items, each item is processed
+			individually.  A "key:value" item removes the existing
+			item with that key regardless of its value.  A "key:"
+			item also removes by key match.
 			The individual values from a comma separated list or
 			list of flags can be inserted by typing 'wildchar'.
 			See |complete-set-option|.

src/nvim/option.c

@@ -853,6 +853,206 @@ static void stropt_remove_val(const char *origval, char *newval, uint32_t flags,
   }
 }
 
+/// Find a comma-separated item in "src" that matches the key part of "key".
+/// The key is the part before ':'.  "keylen" is the length including ':'.
+/// Returns a pointer to the found item in "src", or NULL if not found.
+/// Sets "*itemlenp" to the length of the found item (up to ',' or NUL).
+static char *find_key_item(char *src, char *key, ptrdiff_t keylen, ptrdiff_t *itemlenp)
+{
+  char *p = src;
+
+  while (*p != NUL) {
+    // Check if this item starts with the same key
+    if ((p == src || *(p - 1) == ',') && strncmp(p, key, (size_t)keylen) == 0) {
+      // Find the end of this item
+      char *end = vim_strchr(p, ',');
+      if (end == NULL) {
+        end = p + strlen(p);
+      }
+      *itemlenp = end - p;
+      return p;
+    }
+    p++;
+  }
+  return NULL;
+}
+
+/// Remove one item of length "itemlen" at position "item" from comma-separated
+/// string "str" in-place.  Handles the comma before or after the item.
+static void remove_comma_item(const char *str, char *item, ptrdiff_t itemlen)
+{
+  if (item[itemlen] == ',') {
+    // Remove item and trailing comma
+    STRMOVE(item, item + itemlen + 1);
+  } else if (item > str && *(item - 1) == ',') {
+    // Last item: remove leading comma and item
+    STRMOVE(item - 1, item + itemlen);
+  } else {
+    // Only item
+    *item = NUL;
+  }
+}
+
+/// Remove all items matching "key" (with ':') from comma-separated string "str"
+/// in-place.  If "skip" is not NULL, the item at that position is kept.
+static void remove_key_item(char *str, char *key, ptrdiff_t keylen, const char *skip)
+{
+  ptrdiff_t itemlen;
+  char *found;
+
+  while ((found = find_key_item(str, key, keylen, &itemlen)) != NULL) {
+    if (found == skip) {
+      // Search for the next match after this one.
+      char *next = found + itemlen;
+      if (*next == ',') {
+        next++;
+      }
+      found = find_key_item(next, key, keylen, &itemlen);
+      if (found == NULL) {
+        break;
+      }
+    }
+
+    remove_comma_item(str, found, itemlen);
+  }
+}
+
+/// Append a comma-separated item to the end of "str" in-place.
+/// Adds a comma before the item if "str" is not empty.
+static void append_item(char *str, char *item, ptrdiff_t item_len)
+{
+  ptrdiff_t len = (ptrdiff_t)strlen(str);
+
+  if (len > 0) {
+    str[len++] = ',';
+  }
+  memmove(str + len, item, (size_t)item_len);
+  str[len + item_len] = NUL;
+}
+
+/// Prepend a comma-separated item to the beginning of "str" in-place.
+/// Adds a comma after the item if "str" is not empty.
+static void prepend_item(char *str, char *item, ptrdiff_t item_len)
+{
+  ptrdiff_t len = (ptrdiff_t)strlen(str);
+  int comma = (len > 0) ? 1 : 0;
+
+  memmove(str + item_len + comma, str, (size_t)len + 1);
+  memmove(str, item, (size_t)item_len);
+  if (comma) {
+    str[item_len] = ',';
+  }
+}
+
+/// For a P_COMMA option: process "key:value" items in "newval" individually.
+/// Each comma-separated item in "newval" is checked against "origval":
+///
+/// For OP_ADDING/OP_PREPENDING, each item is handled as follows:
+///   - colon item, key exists with different value: replace (remove old, add)
+///   - colon item, exact duplicate: do nothing
+///   - colon item, not found: add to end
+///   - non-colon item, exists: do nothing
+///   - non-colon item, not found: add to end
+///
+/// For OP_REMOVING, each item is handled as follows:
+///   - colon item: remove by key match
+///   - non-colon item: remove by exact match
+///
+/// The result is written to "newval".
+/// Returns true if the operation was fully handled (caller should skip the
+/// normal add/remove logic).  Returns false if newval is a single non-colon
+/// item, meaning the caller should use the existing code path.
+static bool stropt_handle_keymatch(const char *origval, char *newval, set_op_T op, uint32_t flags)
+{
+  // Check if newval contains any "key:value" item or multiple
+  // comma-separated items.  If neither, let the caller use the existing
+  // code path.
+  if (vim_strchr(newval, ':') == NULL && vim_strchr(newval, ',') == NULL) {
+    return false;
+  }
+
+  // Work on a copy of newval for iteration.
+  char *newval_copy = xstrdup(newval);
+
+  // Build the result in newval.  Start with a copy of origval, then
+  // modify it per-item.  newval buffer has room for origval + arg.
+  STRCPY(newval, origval);
+
+  // Process each item individually, modifying newval in-place.
+  char *item_start = newval_copy;
+  while (true) {
+    char *p = vim_strchr(item_start, ',');
+    ptrdiff_t item_len = p == NULL ? (ptrdiff_t)strlen(item_start) : p - item_start;
+
+    if (item_len > 0) {
+      char *colon = vim_strchr(item_start, ':');
+      if (colon != NULL && colon < item_start + item_len) {
+        ptrdiff_t keylen = (colon - item_start) + 1;
+
+        if (op == OP_ADDING || op == OP_PREPENDING) {
+          ptrdiff_t old_itemlen;
+          char *found = find_key_item(newval, item_start, keylen, &old_itemlen);
+          if (found != NULL) {
+            if (old_itemlen == item_len
+                && strncmp(found, item_start, (size_t)item_len) == 0) {
+              // Exact duplicate: keep it in place, but
+              // remove other items with the same key.
+              remove_key_item(newval, item_start, keylen, found);
+            } else {
+              // Key match with different value: remove all
+              // items with the same key, then add.
+              remove_key_item(newval, item_start, keylen, NULL);
+              if (op == OP_PREPENDING) {
+                prepend_item(newval, item_start, item_len);
+              } else {
+                append_item(newval, item_start, item_len);
+              }
+            }
+          } else {
+            // New item.
+            if (op == OP_PREPENDING) {
+              prepend_item(newval, item_start, item_len);
+            } else {
+              append_item(newval, item_start, item_len);
+            }
+          }
+        } else if (op == OP_REMOVING) {
+          remove_key_item(newval, item_start, keylen, NULL);
+        }
+      } else {
+        if (op == OP_ADDING || op == OP_PREPENDING) {
+          const char *found = find_dup_item(newval, item_start, (size_t)item_len,
+                                            kOptFlagComma);
+          if (found == NULL) {
+            // New item.
+            if (op == OP_PREPENDING) {
+              prepend_item(newval, item_start, item_len);
+            } else {
+              append_item(newval, item_start, item_len);
+            }
+          }
+          // else: exact duplicate — do nothing
+        } else if (op == OP_REMOVING) {
+          char *found = (char *)find_dup_item(newval, item_start, (size_t)item_len,
+                                              kOptFlagComma);
+          if (found != NULL) {
+            remove_comma_item(newval, found, item_len);
+          }
+        }
+      }
+    }
+
+    if (p == NULL) {
+      break;
+    }
+    item_start = p + 1;
+  }
+
+  xfree(newval_copy);
+
+  return true;
+}
+
 /// Remove flags that appear twice in the string option value 'newval'.
 static void stropt_remove_dupflags(char *newval, uint32_t flags)
 {
@@ -908,34 +1108,42 @@ static char *stropt_get_newval(int nextchar, OptIndex opt_idx, char **argp, void
     newval = stropt_expand_envvar(opt_idx, origval, newval, op);
   }
 
-  // locate newval[] in origval[] when removing it
+  // For kOptFlagComma|kOptFlagColon options with "key:value" items: process each item
-  // and when adding to avoid duplicates
+  // individually by matching on the key part.
-  int len = 0;
+  // If handled, skip the normal add/remove logic below.
-  if (op == OP_REMOVING || (flags & kOptFlagNoDup)) {
+  if ((flags & kOptFlagComma) && (flags & kOptFlagColon) && op != OP_NONE
-    len = (int)strlen(newval);
+      && stropt_handle_keymatch(origval, newval, op, flags)) {
-    s = find_dup_item(origval, newval, (size_t)len, flags);
+    // fully handled
+  } else {
+    // locate newval[] in origval[] when removing it and when
+    // adding to avoid duplicates
+    int len = 0;
+    if (op == OP_REMOVING || (flags & kOptFlagNoDup)) {
+      len = (int)strlen(newval);
+      s = find_dup_item(origval, newval, (size_t)len, flags);
 
-    // do not add if already there
+      // do not add if already there
-    if ((op == OP_ADDING || op == OP_PREPENDING) && s != NULL) {
+      if ((op == OP_ADDING || op == OP_PREPENDING) && s != NULL) {
-      op = OP_NONE;
+        op = OP_NONE;
-      STRCPY(newval, origval);
+        STRCPY(newval, origval);
+      }
+
+      // if no duplicate, move pointer to end of original value
+      if (s == NULL) {
+        s = origval + (int)strlen(origval);
+      }
     }
 
-    // if no duplicate, move pointer to end of original value
+    // concatenate the two strings; add a ',' if needed
-    if (s == NULL) {
+    if (op == OP_ADDING || op == OP_PREPENDING) {
-      s = origval + (int)strlen(origval);
+      stropt_concat_with_comma(origval, newval, op, flags);
+    } else if (op == OP_REMOVING) {
+      // Remove newval[] from origval[]. (Note: "len" has been
+      // set above and is used here).
+      stropt_remove_val(origval, newval, flags, s, len);
     }
   }
 
-  // concatenate the two strings; add a ',' if needed
-  if (op == OP_ADDING || op == OP_PREPENDING) {
-    stropt_concat_with_comma(origval, newval, op, flags);
-  } else if (op == OP_REMOVING) {
-    // Remove newval[] from origval[]. (Note: "len" has been set above
-    // and is used here).
-    stropt_remove_val(origval, newval, flags, s, len);
-  }
-
   if (flags & kOptFlagFlagList) {
     // Remove flags that appear twice.
     stropt_remove_dupflags(newval, flags);

src/nvim/options.lua

@@ -3287,7 +3287,7 @@ local options = {
       ]=],
       expand_cb = 'expand_set_chars_option',
       full_name = 'fillchars',
-      list = 'onecomma',
+      list = 'onecommacolon',
       redraw = { 'current_window' },
       scope = { 'global', 'win' },
       short_desc = N_('characters to use for displaying special items'),
@@ -5531,7 +5531,7 @@ local options = {
       ]=],
       expand_cb = 'expand_set_chars_option',
       full_name = 'listchars',
-      list = 'onecomma',
+      list = 'onecommacolon',
       redraw = { 'current_window' },
       scope = { 'global', 'win' },
       short_desc = N_('characters for displaying in list mode'),

test/old/testdir/test_listchars.vim

@@ -252,7 +252,7 @@ func Test_listchars()
 	      \ '.-+*0++0>>>>$',
 	      \ '$'
 	      \ ]
-  call assert_equal('eol:$,nbsp:S,leadmultispace:.-+*,space:+,trail:>,eol:$', &listchars)
+  call assert_equal('eol:$,nbsp:S,leadmultispace:.-+*,space:+,trail:>', &listchars)
   call Check_listchars(expected, 7)
   call Check_listchars(expected, 6, -1, 1)
   call Check_listchars(expected, 6, -1, 2)
@@ -340,11 +340,21 @@ func Test_listchars()
 	      \ 'XyYX0Xy0XyYX$',
 	      \ '$'
 	      \ ]
+  call assert_equal('eol:$,space:x,multispace:XyY', &listchars)
+  call Check_listchars(expected, 7)
+  call Check_listchars(expected, 6, -1, 6)
+  call assert_equal(expected, split(execute("%list"), "\n"))
+
+  " when using :let, multiple 'multispace:' fields can exist
+  " and the last occurrence of 'multispace:' is used
+  let &listchars = 'eol:$,multispace:yYzZ,space:x,multispace:XyY'
   call assert_equal('eol:$,multispace:yYzZ,space:x,multispace:XyY', &listchars)
   call Check_listchars(expected, 7)
   call Check_listchars(expected, 6, -1, 6)
   call assert_equal(expected, split(execute("%list"), "\n"))
 
+  " restore to single multispace: for subsequent tests
+  set listchars=eol:$,space:x,multispace:XyY
   set listchars+=lead:>,trail:<
 
   let expected = [
@@ -361,8 +371,7 @@ func Test_listchars()
   call assert_equal(expected, split(execute("%list"), "\n"))
 
   " removing 'multispace:'
-  set listchars-=multispace:XyY
+  set listchars-=multispace:
-  set listchars-=multispace:yYzZ
 
   let expected = [
 	      \ '>>>>ffff<<<<$',

test/old/testdir/test_options.vim

@@ -2926,4 +2926,145 @@ func Test_showcmd()
   let &cp = _cp
 endfunc
 
+" Test that :set+= and :set-= handle "key:value" items in comma-separated
+" options by matching on the key part.
+func Test_comma_option_key_value()
+  " += replaces existing item with same key
+  set diffopt=internal,filler,algorithm:patience
+  set diffopt+=algorithm:histogram
+  call assert_equal('internal,filler,algorithm:histogram', &diffopt)
+
+  " += with exact duplicate does nothing
+  set diffopt=internal,filler,algorithm:patience
+  set diffopt+=algorithm:patience
+  call assert_equal('internal,filler,algorithm:patience', &diffopt)
+
+  " += with multiple items, each processed individually
+  set diffopt=algorithm:patience,filler
+  set diffopt+=algorithm:histogram,filler
+  call assert_equal('filler,algorithm:histogram', &diffopt)
+
+  " += with non-colon item appends normally
+  set diffopt=internal,filler
+  set diffopt+=iwhite
+  call assert_equal('internal,filler,iwhite', &diffopt)
+
+  " += repeated updates
+  set diffopt=internal,filler,algorithm:patience
+  set diffopt+=algorithm:histogram
+  set diffopt+=algorithm:minimal
+  set diffopt+=algorithm:myers
+  call assert_equal('internal,filler,algorithm:myers', &diffopt)
+
+  " += all exact duplicates does nothing
+  set diffopt=internal,filler,algorithm:patience
+  set diffopt+=algorithm:patience,filler
+  call assert_equal('internal,filler,algorithm:patience', &diffopt)
+
+  " -= with "key:" removes item regardless of value
+  set diffopt=internal,filler,algorithm:patience
+  set diffopt-=algorithm:
+  call assert_equal('internal,filler', &diffopt)
+
+  " -= with "key:value" also matches by key
+  set diffopt=internal,filler,algorithm:patience
+  set diffopt-=algorithm:histogram
+  call assert_equal('internal,filler', &diffopt)
+
+  " -= without colon does not match "key:value" items
+  set diffopt=internal,filler,algorithm:patience
+  set diffopt-=algorithm
+  call assert_equal('internal,filler,algorithm:patience', &diffopt)
+
+  " -= with multiple non-colon items (order independent)
+  set diffopt=internal,filler,closeoff
+  set diffopt-=filler,internal
+  call assert_equal('closeoff', &diffopt)
+
+  " -= with multiple non-colon items (same order as in option)
+  set diffopt=internal,filler,closeoff
+  set diffopt-=internal,filler
+  call assert_equal('closeoff', &diffopt)
+
+  " -= with multiple items: non-colon and colon mixed
+  set diffopt& diffopt=internal,filler,closeoff,indent-heuristic,inline:char
+  set diffopt-=indent-heuristic,inline:char
+  call assert_equal('internal,filler,closeoff', &diffopt)
+
+  " -= with multiple items: colon and non-colon mixed (reverse order)
+  set diffopt& diffopt=internal,filler,closeoff,indent-heuristic,inline:char
+  set diffopt-=inline:char,indent-heuristic
+  call assert_equal('internal,filler,closeoff', &diffopt)
+
+  " += with multiple non-colon items
+  set diffopt=internal,filler
+  set diffopt+=closeoff,iwhite
+  call assert_equal('internal,filler,closeoff,iwhite', &diffopt)
+
+  " += with multiple non-colon items, some already exist
+  set diffopt=internal,filler,closeoff
+  set diffopt+=filler,iwhite
+  call assert_equal('internal,filler,closeoff,iwhite', &diffopt)
+
+  " -= with multiple items including key match
+  set diffopt=internal,filler,algorithm:patience
+  set diffopt-=algorithm:,filler
+  call assert_equal('internal', &diffopt)
+
+  " -= key match when item is at the beginning
+  set diffopt=algorithm:patience,internal,filler
+  set diffopt-=algorithm:
+  call assert_equal('internal,filler', &diffopt)
+
+  " -= key match when item is at the end
+  set diffopt=internal,filler,algorithm:patience
+  set diffopt-=algorithm:
+  call assert_equal('internal,filler', &diffopt)
+
+  " -= key match when item is the only item
+  set diffopt=algorithm:patience
+  set diffopt-=algorithm:
+  call assert_equal('', &diffopt)
+
+  " ^= prepends new item
+  set diffopt=internal,filler
+  set diffopt^=algorithm:histogram
+  call assert_equal('algorithm:histogram,internal,filler', &diffopt)
+
+  " ^= replaces item and prepends
+  set diffopt=internal,filler,algorithm:patience
+  set diffopt^=algorithm:histogram
+  call assert_equal('algorithm:histogram,internal,filler', &diffopt)
+
+  " ^= with exact duplicate does nothing
+  set diffopt=internal,filler,algorithm:patience
+  set diffopt^=algorithm:patience
+  call assert_equal('internal,filler,algorithm:patience', &diffopt)
+
+  set diffopt&
+
+  " Multiple items with the same key (set via :let)
+  " += with different value removes all items with the same key
+  let &lcs = 'eol:$,multispace:yY,space:x,multispace:XY'
+  set lcs+=multispace:AB
+  call assert_equal('eol:$,space:x,multispace:AB', &lcs)
+
+  " += with exact duplicate keeps it and removes others with the same key
+  let &lcs = 'eol:$,multispace:XY,space:x,multispace:XY'
+  set lcs+=multispace:XY
+  call assert_equal('eol:$,multispace:XY,space:x', &lcs)
+
+  " -= removes all items with the same key
+  let &lcs = 'eol:$,multispace:yY,space:x,multispace:XY'
+  set lcs-=multispace:
+  call assert_equal('eol:$,space:x', &lcs)
+
+  " ^= with different value removes all items and prepends
+  let &lcs = 'eol:$,multispace:yY,space:x,multispace:XY'
+  set lcs^=multispace:AB
+  call assert_equal('multispace:AB,eol:$,space:x', &lcs)
+
+  set lcs&
+endfunc
+
 " vim: shiftwidth=2 sts=2 expandtab