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>
This commit is contained in:
zeertzjq
2026-03-23 10:13:48 +08:00
committed by GitHub
parent fae782557c
commit e51f217be7
6 changed files with 409 additions and 27 deletions

View File

@@ -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

View File

@@ -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|.

View File

@@ -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
// 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);
// For kOptFlagComma|kOptFlagColon options with "key:value" items: process each item
// individually by matching on the key part.
// If handled, skip the normal add/remove logic below.
if ((flags & kOptFlagComma) && (flags & kOptFlagColon) && op != OP_NONE
&& stropt_handle_keymatch(origval, newval, op, 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
if ((op == OP_ADDING || op == OP_PREPENDING) && s != NULL) {
op = OP_NONE;
STRCPY(newval, origval);
// do not add if already there
if ((op == OP_ADDING || op == OP_PREPENDING) && s != NULL) {
op = OP_NONE;
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
if (s == NULL) {
s = origval + (int)strlen(origval);
// 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);
}
}
// 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);

View File

@@ -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'),

View File

@@ -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:yYzZ
set listchars-=multispace:
let expected = [
\ '>>>>ffff<<<<$',

View File

@@ -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