IntenseFinance/neovim
3
0
Fork 0
mirror of https://github.com/neovim/neovim.git synced 2026-08-17 20:24:48 -05:00
Code Issues Packages Projects Releases Wiki Activity

refactor(options): store dict options in :set-string form

Browse Source 
The parent commit made a valiant effort to store "dict options" in their
reified form, but this is more trouble than it's worth:
- inconsistent model for developers to understand.
- string lifetime issues (the "varp" convention is to pass around an
  aliased, long-lived option value).
- lots of extra plumbing to deal with the 2 different option-storage
  paradigms.
This commit is contained in:
Justin M. Keyes
2026-07-24 13:05:20 +02:00
parent f8cfd7f06a
commit 3fceb84560
16 changed files with 124 additions and 398 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/gen/gen_options.lua
+4 -3
View File
@@ -29,8 +29,9 @@ local function get_values_var(o)
return ('opt_%s_values'):format(o.abbreviation or o.full_name)
end
--- True if an option is stored as a reified keyset (kOptValTypeDict): a `dict` schema (typed
--- key:value map, e.g. 'diffopt'), as opposed to a flag/enum/char category.
--- True if an option is a dict option: a `dict` schema (typed key:value map, e.g. 'diffopt'), as
--- opposed to a flag/enum/char category. Dict options are stored as their (canonical) ":set"
--- string; the `dict` schema generates the keyset machinery used to validate and reify it on demand.
--- @param o vim.option_meta
--- @return boolean
local function is_dict_option(o)
@@ -215,7 +216,7 @@ local function dump_option(i, o, write)
if o.abbreviation then
write(' .shortname=', cstr(o.abbreviation))
end
write(' .type=', is_dict_option(o) and 'kOptValTypeDict' or opt_type_enum(o.type))
write(' .type=', opt_type_enum(o.type))
write(' .flags=', get_flags(o))
write(' .scope_flags=', get_scope_flags(o))
write(' .scope_idx=', get_scope_idx(o))
src/nvim/api/deprecated.c
+1 -6
View File
@@ -757,12 +757,7 @@ static Object get_option_from(void *from, OptScope scope, String name, Error *er
return (Object)OBJECT_INIT;
});
Object rv = optval_as_object(value);
// A struct value serializes into a fresh Object string, so its keyset must be freed here.
if (value.type == kOptValTypeDict) {
optval_free(value);
}
return rv;
return optval_as_object(value);
}
/// Sets the value of a global or local (buffer, window) option.
src/nvim/api/options.c
+1 -11
View File
@@ -305,13 +305,7 @@ Object nvim_get_option_value(String name, Dict(option) *opts, Error *err)
goto err;
});
Object rv = optval_as_object(value);
// kOptValTypeDict serializes into a fresh Object string (not aliased like a plain string), so
// its keyset must be freed here.
if (value.type == kOptValTypeDict) {
optval_free(value);
}
return rv;
return optval_as_object(value);
err:
optval_free(value);
return (Object)OBJECT_INIT;
@@ -403,10 +397,6 @@ Object nvim_set_option_value(uint64_t channel_id, String name, Object value, Dic
case kOptValTypeBoolean:
merged_val = optval_right;
break;
case kOptValTypeDict:
// Unreachable: object_as_optval_for() yields the string form for dict options, which is
// merged as a string (below) and reified by set_option().
break;
}
optval_free(optval_right);
src/nvim/buffer_defs.h
+1 -1
View File
@@ -92,7 +92,7 @@ typedef struct {
#define w_p_arab w_onebuf_opt.wo_arab // 'arabic'
int wo_bri;
#define w_p_bri w_onebuf_opt.wo_bri // 'breakindent'
struct OptKeyDict_briopt *wo_briopt; // 'breakindentopt'
char *wo_briopt;
#define w_p_briopt w_onebuf_opt.wo_briopt // 'breakindentopt'
int wo_diff;
#define w_p_diff w_onebuf_opt.wo_diff // 'diff'
src/nvim/diff.c
+27 -30
View File
@@ -2654,12 +2654,13 @@ int diffanchors_changed(bool buflocal)
return result;
}
/// Map the 'diffopt' keyset onto diff.c's globals. `v` is the stored value (`p_dip`), reified from
/// the ":set" string by `opt_fill()` when the option is set.
/// Map the 'diffopt' keyset onto diff.c's globals. `v` is `p_dip` reified into a keyset.
///
/// @return FAIL only for the cross-part "horizontal" + "vertical" conflict.
static int diffopt_apply(OptKeyDict_dip *v)
int diffopt_changed(void)
{
OptKeyDict_dip *v = opt_keyset_alloc(kOptDiffopt, p_dip);
int flags = (v->filler ? DIFF_FILLER : 0) | (v->anchor ? DIFF_ANCHOR : 0)
| (v->iblank ? DIFF_IBLANK : 0) | (v->icase ? DIFF_ICASE : 0)
| (v->iwhiteall ? DIFF_IWHITEALL : 0) | (v->iwhiteeol ? DIFF_IWHITEEOL : 0)
@@ -2693,38 +2694,34 @@ static int diffopt_apply(OptKeyDict_dip *v)
} // else "myers" -> 0
}
int ret = OK;
// Can't have both "horizontal" and "vertical".
if ((flags & DIFF_HORIZONTAL) && (flags & DIFF_VERTICAL)) {
return FAIL;
}
// If flags were added or removed, or the algorithm was changed, update the diff.
if (diff_flags != flags || diff_algorithm != algorithm) {
FOR_ALL_TABS(tp) {
tp->tp_diff_invalid = true;
ret = FAIL;
} else {
// If flags were added or removed, or the algorithm was changed, update the diff.
if (diff_flags != flags || diff_algorithm != algorithm) {
FOR_ALL_TABS(tp) {
tp->tp_diff_invalid = true;
}
}
int context = HAS_KEY(v, dip, context) ? (int)v->context : 6;
diff_flags = flags;
diff_context = context == 0 ? 1 : context;
linematch_lines = HAS_KEY(v, dip, linematch) ? (int)v->linematch : 0;
diff_foldcolumn = HAS_KEY(v, dip, foldcolumn) ? (int)v->foldcolumn : 2;
diff_algorithm = algorithm;
diff_redraw(true);
// recompute the scroll binding with the new option value, may
// remove or add filler lines
check_scrollbind(0, 0);
}
int context = HAS_KEY(v, dip, context) ? (int)v->context : 6;
diff_flags = flags;
diff_context = context == 0 ? 1 : context;
linematch_lines = HAS_KEY(v, dip, linematch) ? (int)v->linematch : 0;
diff_foldcolumn = HAS_KEY(v, dip, foldcolumn) ? (int)v->foldcolumn : 2;
diff_algorithm = algorithm;
diff_redraw(true);
// recompute the scroll binding with the new option value, may
// remove or add filler lines
check_scrollbind(0, 0);
return OK;
}
/// Apply the current 'diffopt'. Its value is stored as the reified keyset `p_dip` (see
/// `opt_dict_info()`), so this just hands the stored struct to `diffopt_apply()`.
int diffopt_changed(void)
{
return p_dip == NULL ? OK : diffopt_apply(p_dip);
opt_keyset_free(kOptDiffopt, v);
return ret;
}
/// Check that "diffopt" contains "horizontal".
src/nvim/eval.c
-4
View File
@@ -3409,10 +3409,6 @@ int eval_option(const char **const arg, typval_T *const rettv, const bool evalua
assert(value.type != kOptValTypeNil);
*rettv = optval_as_tv(value, true);
// A dict option serializes into a fresh typval string that rettv now owns, so free its keyset.
if (value.type == kOptValTypeDict) {
optval_free(value);
}
} else if (working && !is_tty_opt && is_option_hidden(opt_idx)) {
ret = FAIL;
}
src/nvim/eval/vars.c
+1 -29
View File
@@ -1378,14 +1378,6 @@ static char *ex_let_option(char *arg, typval_T *const tv, const bool is_const,
semsg(_(e_unknown_option2), arg);
goto theend;
}
// `:let &opt` operates on the string form; a dict option is handled as its serialization
// (set_option() reifies it back). This also lets `.=` concatenate onto it like any string option.
if (curval.type == kOptValTypeDict) {
OptVal strval = CSTR_AS_OPTVAL(opt_serialize(curval.data.dictval.ptr,
curval.data.dictval.table));
optval_free(curval);
curval = strval;
}
if (op != NULL && *op != '='
&& ((curval.type != kOptValTypeString && *op == '.')
|| (curval.type == kOptValTypeString && *op != '.'))) {
@@ -3211,9 +3203,7 @@ static OptVal tv_to_optval(typval_T *tv, OptIndex opt_idx, const char *option, b
const bool is_tty_opt = is_tty_option(option);
const bool option_has_bool = !is_tty_opt && option_has_type(opt_idx, kOptValTypeBoolean);
const bool option_has_num = !is_tty_opt && option_has_type(opt_idx, kOptValTypeNumber);
// Struct-stored options (e.g. 'diffopt') take their ":set" string here; set_option() reifies it.
const bool option_has_str = is_tty_opt || option_has_type(opt_idx, kOptValTypeString)
|| option_has_type(opt_idx, kOptValTypeDict);
const bool option_has_str = is_tty_opt || option_has_type(opt_idx, kOptValTypeString);
if (!is_tty_opt && (get_option(opt_idx)->flags & kOptFlagFunc) && tv_is_func(*tv)) {
// If the option can be set to a function reference or a lambda
@@ -3260,10 +3250,6 @@ static OptVal tv_to_optval(typval_T *tv, OptIndex opt_idx, const char *option, b
/// Convert an option value to typval.
///
/// A "schema.dict" option has no stored string to alias, so it serializes to an allocated string
/// owned by the returned; every other type borrows from `value`. Either transfer that string to
/// a longer-lived owner or release it with `optval_as_tv_free()`.
///
/// @param[in] value Option value to convert.
/// @param numbool Whether to convert boolean values to number.
/// Used for backwards compatibility.
@@ -3293,25 +3279,11 @@ typval_T optval_as_tv(OptVal value, bool numbool)
rettv.v_type = VAR_STRING;
rettv.vval.v_string = value.data.string.data;
break;
case kOptValTypeDict:
// Surfaced to Vimscript as its ":set" string, allocated.
rettv.v_type = VAR_STRING;
rettv.vval.v_string = opt_serialize(value.data.dictval.ptr, value.data.dictval.table);
break;
}
return rettv;
}
/// Release `optval_as_tv()` result. Only for "schema.dict" options; no-op for other types (they
/// alias the source value).
void optval_as_tv_free(OptVal value, typval_T tv)
{
if (value.type == kOptValTypeDict) {
xfree(tv.vval.v_string);
}
}
/// Set option "varname" to the value of "varp" for the current buffer/window.
static void set_option_from_tv(const char *varname, typval_T *varp)
{
src/nvim/indent.c
+9 -8
View File
@@ -787,14 +787,15 @@ void briopt_check(win_T *wp)
if (wp == NULL) {
return; // Setting the global value: nothing to apply to a window.
}
// 'breakindentopt' is stored as its reified keyset (validated when set); just map it onto the
// applied per-window fields. A NULL keyset (before the option is set) means all-default.
OptKeyDict_briopt *v = wp->w_p_briopt;
wp->w_briopt_shift = (v != NULL && HAS_KEY(v, briopt, shift)) ? (int)v->shift : 0;
wp->w_briopt_min = (v != NULL && HAS_KEY(v, briopt, min)) ? (int)v->min : 20;
wp->w_briopt_sbr = (v != NULL && HAS_KEY(v, briopt, sbr));
wp->w_briopt_list = (v != NULL && HAS_KEY(v, briopt, list)) ? (int)v->list : 0;
wp->w_briopt_vcol = (v != NULL && HAS_KEY(v, briopt, column)) ? (int)v->column : 0;
// 'breakindentopt' is stored as its ":set" string (validated when set); reify it into a keyset and
// map that onto the applied per-window fields.
OptKeyDict_briopt *v = opt_keyset_alloc(kOptBreakindentopt, wp->w_p_briopt);
wp->w_briopt_shift = HAS_KEY(v, briopt, shift) ? (int)v->shift : 0;
wp->w_briopt_min = HAS_KEY(v, briopt, min) ? (int)v->min : 20;
wp->w_briopt_sbr = HAS_KEY(v, briopt, sbr);
wp->w_briopt_list = HAS_KEY(v, briopt, list) ? (int)v->list : 0;
wp->w_briopt_vcol = HAS_KEY(v, briopt, column) ? (int)v->column : 0;
opt_keyset_free(kOptBreakindentopt, v);
}
// Return appropriate space number for breakindent, taking influencing
src/nvim/option.c
+43 -201
View File
@@ -472,19 +472,6 @@ static void alloc_options_default(void)
{
for (OptIndex opt_idx = 0; opt_idx < kOptCount; opt_idx++) {
options[opt_idx].def_val = optval_copy(options[opt_idx].def_val);
// A dict option declares its default as a ":set" string (options.lua). Reify it into a
// keyset once here, so the default is a keyset, for reset and ":set opt&".
if (option_has_type(opt_idx, kOptValTypeDict)
&& options[opt_idx].def_val.type == kOptValTypeString) {
const char *emsg = NULL;
OptVal reified = opt_dict_from_string(opt_idx, options[opt_idx].def_val.data.string.data,
NULL, 0, &emsg);
assert(emsg == NULL); // A built-in default must be valid.
(void)emsg;
optval_free(options[opt_idx].def_val);
options[opt_idx].def_val = reified;
}
}
}
@@ -1454,21 +1441,14 @@ OptVal get_option_newval(OptIndex opt_idx, int opt_flags, set_prefix_T prefix, c
break;
}
case kOptValTypeString: {
// A dict option merges here too: its stored value is already a ":set" string, so =/+=/-= apply
// as for any string, and set_option() validates and canonicalizes the result.
const char *oldval_str = oldval.data.string.data;
// Get the new value for the option
const char *newval_str = stropt_get_newval(opt_idx, argp, varp, oldval_str, &op);
newval = CSTR_AS_OPTVAL(newval_str);
break;
}
case kOptValTypeDict: {
// Merge at the string level (serialize the old keyset, apply =/+=/-=); set_option() reifies and
// validates the result. stropt_get_newval() doesn't dereference varp except for 'keywordprg'.
char *oldval_str = opt_serialize(oldval.data.dictval.ptr, oldval.data.dictval.table);
char *newval_str = stropt_get_newval(opt_idx, argp, varp, oldval_str, &op);
xfree(oldval_str);
newval = CSTR_AS_OPTVAL(newval_str);
break;
}
}
return newval;
@@ -2136,12 +2116,6 @@ void apply_optionset_autocmd_now(OptIndex opt_idx, int opt_flags, OptVal oldval,
}
apply_autocmds(EVENT_OPTIONSET, options[opt_idx].fullname, NULL, false, NULL);
reset_v_option_vars();
// set_vim_var_tv() copied each typval, so release the strings optval_as_tv() freshly allocated.
optval_as_tv_free(oldval, oldval_tv);
optval_as_tv_free(oldval_g, oldval_g_tv);
optval_as_tv_free(oldval_l, oldval_l_tv);
optval_as_tv_free(newval, newval_tv);
}
/// For 'modified', the event is deferred.
@@ -3362,84 +3336,37 @@ OptIndex find_option(const char *const name)
return find_option_len(name, strlen(name));
}
/// Free the owned resources of a dict option value: its `String` fields and the keyset.
static void opt_dict_free(OptDict s)
/// True if `opt_idx` is a dict option (`schema.dict` in options.lua, e.g. 'diffopt'). Stored as its
/// canonical ":set" string; reified on-demand (`opt_keyset_alloc()`) for validation and applying.
bool is_dict_option(OptIndex opt_idx)
{
if (s.ptr == NULL) {
return;
}
for (const KeySetLink *f = s.table; f->str != NULL; f++) {
if (f->type == kObjectTypeString) {
api_free_string(*(String *)((char *)s.ptr + f->ptr_off));
}
}
xfree(s.ptr);
return opt_dict_info(opt_idx) != NULL;
}
/// Deep-copy a dict option value (keyset + its `String` fields).
static OptDict opt_dict_dup(OptDict s)
{
if (s.ptr == NULL) {
return s;
}
void *ptr = xmemdup(s.ptr, s.size);
for (const KeySetLink *f = s.table; f->str != NULL; f++) {
if (f->type == kObjectTypeString) {
String *field = (String *)((char *)ptr + f->ptr_off);
*field = copy_string(*field, NULL);
}
}
return (OptDict){ ptr, s.table, s.size };
}
/// Compare two dict option values: same keys present (`is_set_`) and same field values.
static bool opt_dict_equal(OptDict a, OptDict b)
{
if (a.ptr == NULL || b.ptr == NULL) {
return a.ptr == b.ptr;
}
for (const KeySetLink *f = a.table; f->str != NULL; f++) {
const void *fa = (const char *)a.ptr + f->ptr_off;
const void *fb = (const char *)b.ptr + f->ptr_off;
switch (f->type) {
case kObjectTypeInteger:
if (*(const Integer *)fa != *(const Integer *)fb) {
return false;
}
break;
case kObjectTypeString: {
const String *sa = fa;
const String *sb = fb;
if (sa->size != sb->size || (sa->size != 0 && memcmp(sa->data, sb->data, sa->size) != 0)) {
return false;
}
break;
}
default: // Boolean and other scalars
if (*(const Boolean *)fa != *(const Boolean *)fb) {
return false;
}
break;
}
}
// is_set__<abbr>_ (OptionalKeys) is the first member of every keyset.
return *(const OptionalKeys *)a.ptr == *(const OptionalKeys *)b.ptr;
}
/// Validate a ":set" string against a dict option's grammar and reify it into a fresh
/// heap keyset. On success returns an owned struct OptVal; on failure sets `errmsg` and returns Nil.
static OptVal opt_dict_from_string(OptIndex opt_idx, const char *str, char *errbuf,
size_t errbuflen, const char **errmsg)
/// Parse a dict option's ":set" string to an allocated keyset (`OptKeyDict_…`). A NULL string
/// yields an all-unset keyset. Free it with `opt_keyset_free()`.
void *opt_keyset_alloc(OptIndex opt_idx, const char *str)
{
const OptDictInfo *si = opt_dict_info(opt_idx);
const char *err = opt_strings_check(str, si->schema, errbuf, errbuflen);
if (err != NULL) {
*errmsg = err;
return NIL_OPTVAL;
void *keyset = xcalloc(1, si->size);
if (str != NULL) {
opt_fill(str, si->get_field, keyset);
}
void *ptr = xcalloc(1, si->size);
opt_fill(str, si->get_field, ptr);
return (OptVal){ .type = kOptValTypeDict, .data.dictval = { ptr, si->table, si->size } };
return keyset;
}
/// Free a keyset from `opt_keyset_alloc()`: its owned `String` fields, then the keyset itself.
void opt_keyset_free(OptIndex opt_idx, void *keyset)
{
if (keyset == NULL) {
return;
}
for (const KeySetLink *f = opt_dict_info(opt_idx)->table; f->str != NULL; f++) {
if (f->type == kObjectTypeString) {
api_free_string(*(String *)((char *)keyset + f->ptr_off));
}
}
xfree(keyset);
}
/// Free an allocated OptVal.
@@ -3456,9 +3383,6 @@ void optval_free(OptVal o)
api_free_string(o.data.string);
}
break;
case kOptValTypeDict:
opt_dict_free(o.data.dictval);
break;
}
}
@@ -3472,9 +3396,6 @@ OptVal optval_copy(OptVal o)
return o;
case kOptValTypeString:
return STRING_OPTVAL(copy_string(o.data.string, NULL));
case kOptValTypeDict:
return (OptVal){ .type = kOptValTypeDict,
.data.dictval = opt_dict_dup(o.data.dictval) };
}
UNREACHABLE;
}
@@ -3497,8 +3418,6 @@ bool optval_equal(OptVal o1, OptVal o2)
return o1.data.string.size == o2.data.string.size
&& (o1.data.string.data == o2.data.string.data
|| strnequal(o1.data.string.data, o2.data.string.data, o1.data.string.size));
case kOptValTypeDict:
return opt_dict_equal(o1.data.dictval, o2.data.dictval);
}
UNREACHABLE;
}
@@ -3535,13 +3454,6 @@ OptVal optval_from_varp(OptIndex opt_idx, void *varp)
return NUMBER_OPTVAL(*(OptInt *)varp);
case kOptValTypeString:
return STRING_OPTVAL(cstr_as_string(*(char **)varp));
case kOptValTypeDict: {
// Alias the stored keyset (like the string case aliases the stored char*): free frees it, copy
// dupes it. The table/size travel with the value so free/copy need no option index.
const OptDictInfo *si = opt_dict_info(opt_idx);
return (OptVal){ .type = kOptValTypeDict,
.data.dictval = { *(void **)varp, si->table, si->size } };
}
}
UNREACHABLE;
}
@@ -3573,11 +3485,6 @@ static void set_option_varp(OptIndex opt_idx, void *varp, OptVal value, bool fre
case kOptValTypeString:
*(char **)varp = value.data.string.data;
return;
case kOptValTypeDict:
// Move the keyset pointer in (ownership transfers, like the string case). The old value was
// already freed above when free_oldval is set.
*(void **)varp = value.data.dictval.ptr;
return;
}
UNREACHABLE;
}
@@ -3600,14 +3507,6 @@ static char *optval_to_cstr(OptVal o)
snprintf(buf, o.data.string.size + 3, "\"%s\"", o.data.string.data);
return buf;
}
case kOptValTypeDict: {
char *s = opt_serialize(o.data.dictval.ptr, o.data.dictval.table);
size_t len = strlen(s);
char *buf = xmalloc(len + 3);
snprintf(buf, len + 3, "\"%s\"", s);
xfree(s);
return buf;
}
}
UNREACHABLE;
}
@@ -3631,9 +3530,6 @@ Object optval_as_object(OptVal o)
return INTEGER_OBJ(o.data.number);
case kOptValTypeString:
return STRING_OBJ(o.data.string);
case kOptValTypeDict:
// The API surfaces the string form; the returned Object owns a fresh serialization.
return STRING_OBJ(cstr_as_string(opt_serialize(o.data.dictval.ptr, o.data.dictval.table)));
}
UNREACHABLE;
}
@@ -3643,21 +3539,16 @@ Object optval_as_object(OptVal o)
/// @return Object allocated in `arena`.
Object optval_to_struct(OptIndex opt_idx, OptVal value, Arena *arena)
{
if (value.type != kOptValTypeString && value.type != kOptValTypeDict) {
if (value.type != kOptValTypeString) {
return optval_as_object(value); // boolean/number/nil scalar
}
const uint32_t flags = options[opt_idx].flags;
const OptSchemaItem *schema = options[opt_idx].schema;
bool owned = false;
char *str;
if (value.type == kOptValTypeDict) {
str = opt_serialize(value.data.dictval.ptr, value.data.dictval.table);
owned = true;
} else {
str = value.data.string.data != NULL ? value.data.string.data : "";
}
// A dict option (`schema != NULL`) parses here like any comma "key:value" map: its stored value
// is already the canonical ":set" string.
char *str = value.data.string.data != NULL ? value.data.string.data : "";
Object rv;
// Plain string option: no list/map structure.
@@ -3715,9 +3606,6 @@ Object optval_to_struct(OptIndex opt_idx, OptVal value, Arena *arena)
rv = as_map ? DICT_OBJ(d) : ARRAY_OBJ(a);
}
if (owned) {
xfree(str);
}
return rv;
}
@@ -3757,7 +3645,7 @@ OptVal object_as_optval_for(OptIndex opt_idx, Object o, set_op_T op, bool *error
const bool is_list = flags & (kOptFlagComma | kOptFlagFlagList);
// "key:value" list, e.g. 'listchars', or a dict option, e.g. 'breakindentopt' (which
// accepts a Dict even without kOptFlagColon, mirroring optval_to_struct()'s `as_map`).
const bool is_map = (flags & kOptFlagColon) || option_has_type(opt_idx, kOptValTypeDict);
const bool is_map = (flags & kOptFlagColon) || is_dict_option(opt_idx);
const bool is_flaglist = flags & kOptFlagFlagList; // single-char flag list, e.g. 'shortmess'.
const bool is_comma = flags & kOptFlagComma;
const bool allow_dup = !(flags & kOptFlagNoDup);
@@ -3772,9 +3660,7 @@ OptVal object_as_optval_for(OptIndex opt_idx, Object o, set_op_T op, bool *error
type_ok = option_has_type(opt_idx, kOptValTypeNumber);
break;
case kObjectTypeString:
// Struct-stored options take their ":set" string here; set_option() reifies it.
type_ok = option_has_type(opt_idx, kOptValTypeString)
|| option_has_type(opt_idx, kOptValTypeDict)
|| opt_idx == kOptWildchar || opt_idx == kOptWildcharm;
break;
case kObjectTypeArray:
@@ -4281,17 +4167,15 @@ static const char *set_option(const OptIndex opt_idx, OptVal value, int opt_flag
const char *errmsg = NULL;
// Struct-stored options keep a reified keyset as the stored value. Every set path (":set", the
// API, Vimscript, a merge) funnels through here as a ":set" string; validate and reify it once,
// the single choke point. Reset-to-default/global (":set opt&/<") already produces a keyset.
if (value.type == kOptValTypeString && option_has_type(opt_idx, kOptValTypeDict)) {
OptVal reified = opt_dict_from_string(opt_idx, value.data.string.data, errbuf, errbuflen,
&errmsg);
optval_free(value);
// Every set path for a dict option (":set", the API, Vimscript, a merge) funnels through here as a
// ":set" string. Validate it once.
if (value.type == kOptValTypeString && is_dict_option(opt_idx)) {
errmsg = opt_strings_check(value.data.string.data, opt_dict_info(opt_idx)->schema, errbuf,
errbuflen);
if (errmsg != NULL) {
optval_free(value);
return errmsg;
}
value = reified;
}
if (!direct) {
@@ -5108,16 +4992,6 @@ static int put_set(FILE *fd, char *cmd, OptIndex opt_idx, void *varp)
xfree(part);
return FAIL;
}
case kOptValTypeDict: {
// Written back as its ":set" string, e.g. `set diffopt=internal,filler,...`.
char *value_str = opt_serialize(value.data.dictval.ptr, value.data.dictval.table);
bool ok = fprintf(fd, "%s %s=", cmd, name) >= 0 && put_escstr(fd, value_str, 2) == OK;
xfree(value_str);
if (!ok) {
return FAIL;
}
break;
}
}
if (put_eol(fd) < 0) {
@@ -5591,21 +5465,6 @@ static char *copy_option_val(const char *val)
return xstrdup(val);
}
/// Deep-copy a dict (keyset) window/buffer option value, or NULL.
static void *copy_opt_dict(void *keyset, OptIndex opt_idx)
{
const OptDictInfo *si = opt_dict_info(opt_idx);
return opt_dict_dup((OptDict){ keyset, si->table, si->size }).ptr;
}
/// Free a dict option value and clear the pointer.
static void clear_opt_dict(void **keyset, OptIndex opt_idx)
{
const OptDictInfo *si = opt_dict_info(opt_idx);
opt_dict_free((OptDict){ *keyset, si->table, si->size });
*keyset = NULL;
}
/// Copy the options from one winopt_T to another.
/// Doesn't free the old option values in "to", use clear_winopt() for that.
/// The 'scroll' option is not copied, because it depends on the window height.
@@ -5630,7 +5489,7 @@ void copy_winopt(winopt_T *from, winopt_T *to)
to->wo_wrap_save = from->wo_wrap_save;
to->wo_lbr = from->wo_lbr;
to->wo_bri = from->wo_bri;
to->wo_briopt = copy_opt_dict(from->wo_briopt, kOptBreakindentopt);
to->wo_briopt = copy_option_val(from->wo_briopt);
to->wo_scb = from->wo_scb;
to->wo_scb_save = from->wo_scb_save;
to->wo_sms = from->wo_sms;
@@ -5706,7 +5565,7 @@ static void check_winopt(winopt_T *wop)
check_string_option(&wop->wo_culopt);
check_string_option(&wop->wo_cc);
check_string_option(&wop->wo_cocu);
// wo_briopt: keyset (may be NULL until set); briopt_check() handles NULL.
check_string_option(&wop->wo_briopt);
check_string_option(&wop->wo_winhl);
check_string_option(&wop->wo_lcs);
check_string_option(&wop->wo_fcs);
@@ -5734,7 +5593,7 @@ void clear_winopt(winopt_T *wop)
clear_string_option(&wop->wo_culopt);
clear_string_option(&wop->wo_cc);
clear_string_option(&wop->wo_cocu);
clear_opt_dict((void **)&wop->wo_briopt, kOptBreakindentopt);
clear_string_option(&wop->wo_briopt);
clear_string_option(&wop->wo_winhl);
clear_string_option(&wop->wo_lcs);
clear_string_option(&wop->wo_fcs);
@@ -6535,14 +6394,6 @@ int ExpandSettingSubtract(expand_T *xp, regmatch_T *regmatch, int *numMatches, c
expand_option_flags,
curbuf, curwin);
// A dict option's varp holds a keyset; expand on its serialized string form instead.
char *option_serialized = NULL;
if (option_has_type(expand_option_idx, kOptValTypeDict)) {
option_serialized = opt_serialize((void *)option_val,
opt_dict_info(expand_option_idx)->table);
option_val = option_serialized;
}
uint32_t option_flags = options[expand_option_idx].flags;
if (option_has_type(expand_option_idx, kOptValTypeNumber)) {
@@ -6554,13 +6405,11 @@ int ExpandSettingSubtract(expand_T *xp, regmatch_T *regmatch, int *numMatches, c
// kOptFlagComma and kOptFlagFlagList.
if (*option_val == NUL) {
xfree(option_serialized);
return FAIL;
}
// Make a copy as we need to inject null characters destructively.
char *option_copy = xstrdup(option_val);
xfree(option_serialized); // struct form copied into option_copy; no longer needed
char *next_val = option_copy;
garray_T ga;
@@ -6662,11 +6511,7 @@ static void option_value2string(vimoption_T *opt, int opt_flags)
"%" PRId64,
(int64_t)(*(OptInt *)varp));
}
} else if (option_has_type(get_opt_idx(opt), kOptValTypeDict)) {
char *str = opt_serialize(*(void **)varp, opt_dict_info(get_opt_idx(opt))->table);
xstrlcpy(NameBuff, str, MAXPATHL);
xfree(str);
} else { // string
} else { // string (including dict options, which are stored as a ":set" string)
varp = *(char **)varp;
if (opt->flags & kOptFlagExpand) {
@@ -7138,11 +6983,8 @@ dict_T *get_winbuf_options(const int bufopt)
void *varp = get_varp(opt);
if (varp != NULL) {
OptVal value = optval_from_varp(opt_idx, varp);
typval_T opt_tv = optval_as_tv(value, true);
typval_T opt_tv = optval_as_tv(optval_from_varp(opt_idx, varp), true);
tv_dict_add_tv(d, opt->fullname, strlen(opt->fullname), &opt_tv);
// tv_dict_add_tv() copied the typval, so release any string optval_as_tv() allocated.
optval_as_tv_free(value, opt_tv);
}
}
}
src/nvim/option.h
-2
View File
@@ -52,8 +52,6 @@ static inline const char *optval_type_get_name(const OptValType type)
return "number";
case kOptValTypeString:
return "string";
case kOptValTypeDict:
return "string"; // dict options present as strings at the API/error surface.
}
UNREACHABLE;
}
src/nvim/option_defs.h
+1 -12
View File
@@ -50,18 +50,8 @@ typedef enum {
kOptValTypeBoolean,
kOptValTypeNumber,
kOptValTypeString,
kOptValTypeDict, ///< Option stored as a reified keyset (`schema.dict` in options.lua).
} OptValType;
/// Storage for a dict option ("schema" in options.lua): a heap-allocated keyset
/// (`OptKeyDict_<name>`) plus the field table needed to free, copy and serialize it without an
/// option index. The string form is derived on-demand (`opt_serialize()`), never stored.
typedef struct {
void *ptr; ///< Heap keyset (owned), or NULL.
const KeySetLink *table; ///< Field layout (borrowed; points at generated static data).
size_t size; ///< sizeof the keyset, for (re)allocation.
} OptDict;
/// Scopes that an option can support.
typedef enum {
kOptScopeGlobal = 0, ///< Request global option value
@@ -78,7 +68,6 @@ typedef union {
TriState boolean;
OptInt number;
String string;
OptDict dictval;
} OptValData;
/// Option value
@@ -108,7 +97,7 @@ typedef struct {
/// `opt_dict_info()`. NULL from `opt_dict_info()` means the option is not a dict option.
typedef struct {
FieldHashfn get_field; ///< Keyset perfect-hash lookup, for opt_fill().
const KeySetLink *table; ///< Field layout, for opt_serialize()/free/copy.
const KeySetLink *table; ///< Field layout, for freeing a keyset's `String` fields.
const OptSchemaItem *schema; ///< Grammar, for opt_strings_check() validation.
size_t size; ///< sizeof the keyset, for allocation.
} OptDictInfo;
src/nvim/option_vars.h
+2 -2
View File
@@ -238,7 +238,7 @@ EXTERN char *p_debug; ///< 'debug'
EXTERN char *p_def; ///< 'define'
EXTERN char *p_inc;
EXTERN char *p_dia; ///< 'diffanchors'
EXTERN struct OptKeyDict_dip *p_dip; ///< 'diffopt'
EXTERN char *p_dip; ///< 'diffopt'
EXTERN char *p_dex; ///< 'diffexpr'
EXTERN char *p_dict; ///< 'dictionary'
EXTERN int p_dg; ///< 'digraph'
@@ -348,7 +348,7 @@ EXTERN char *p_mousem; ///< 'mousemodel'
EXTERN int p_mousemev; ///< 'mousemoveevent'
EXTERN int p_mousef; ///< 'mousefocus'
EXTERN int p_mh; ///< 'mousehide'
EXTERN struct OptKeyDict_mousescroll *p_mousescroll; ///< 'mousescroll'
EXTERN char *p_mousescroll; ///< 'mousescroll'
EXTERN OptInt p_mousescroll_vert INIT( = MOUSESCROLL_VERT_DFLT);
EXTERN OptInt p_mousescroll_hor INIT( = MOUSESCROLL_HOR_DFLT);
EXTERN OptInt p_mouset; ///< 'mousetime'
src/nvim/options.lua
+3 -3
View File
@@ -898,7 +898,7 @@ local options = {
redraw = { 'current_buffer' },
scope = { 'win' },
short_desc = N_("settings for 'breakindent'"),
type = 'string', -- The `schema` reifies to OptKeyDict_briopt.
type = 'string', -- OptKeyDict_briopt
},
{
abbreviation = 'bsdir',
@@ -2625,7 +2625,7 @@ local options = {
redraw = { 'current_window' },
scope = { 'global' },
short_desc = N_('options for using diff mode'),
type = 'string', -- The `schema` reifies to OptKeyDict_dip.
type = 'string', -- OptKeyDict_dip
varname = 'p_dip',
},
{
@@ -6368,7 +6368,7 @@ local options = {
scope = { 'global' },
short_desc = N_('amount to scroll by when scrolling with a mouse'),
tags = { 'E5080' },
type = 'string', -- The `schema` reifies to OptKeyDict_mousescroll.
type = 'string', -- OptKeyDict_mousescroll
varname = 'p_mousescroll',
vi_def = true,
},
src/nvim/optionstr.c
+12 -67
View File
@@ -1560,14 +1560,15 @@ int expand_set_mouse(optexpand_T *args, int *numMatches, char ***matches)
/// @return error message, NULL if it's OK.
const char *did_set_mousescroll(optset_T *args FUNC_ATTR_UNUSED)
{
OptKeyDict_mousescroll *v = opt_keyset_alloc(kOptMousescroll, p_mousescroll);
// An empty value sets no direction; reject it (mousescroll always needs at least one).
OptKeyDict_mousescroll *v = p_mousescroll;
if (!HAS_KEY(v, mousescroll, hor) && !HAS_KEY(v, mousescroll, ver)) {
return e_invarg;
bool has_dir = HAS_KEY(v, mousescroll, hor) || HAS_KEY(v, mousescroll, ver);
if (has_dir) {
p_mousescroll_hor = HAS_KEY(v, mousescroll, hor) ? (int)v->hor : MOUSESCROLL_HOR_DFLT;
p_mousescroll_vert = HAS_KEY(v, mousescroll, ver) ? (int)v->ver : MOUSESCROLL_VERT_DFLT;
}
p_mousescroll_hor = HAS_KEY(v, mousescroll, hor) ? (int)v->hor : MOUSESCROLL_HOR_DFLT;
p_mousescroll_vert = HAS_KEY(v, mousescroll, ver) ? (int)v->ver : MOUSESCROLL_VERT_DFLT;
return NULL;
opt_keyset_free(kOptMousescroll, v);
return has_dir ? NULL : e_invarg;
}
/// One of the '*expr' options is changed:, 'diffexpr', 'foldexpr', 'foldtext',
@@ -2301,8 +2302,8 @@ const char *opt_strings_check(const char *val, const OptSchemaItem *schema, char
}
/// Parses a validated dict option ":set" string into its keyset (`OptKeyDict_…`), see also
/// `api_dict_to_keydict()`. The keyset owns its `String` fields, so free it with `opt_dict_free()`
/// (via `optval_free()`/`clear_opt_dict()`), not `xfree()`.
/// `api_dict_to_keydict()`. The keyset owns its `String` fields, so free it with `opt_keyset_free()`
/// (which frees those fields), never a bare `xfree()`. Callers use `opt_keyset_alloc()`.
void opt_fill(const char *value, FieldHashfn get_field, void *out)
FUNC_ATTR_NONNULL_ALL
{
@@ -2324,7 +2325,9 @@ void opt_fill(const char *value, FieldHashfn get_field, void *out)
break;
}
case kObjectTypeString:
*(String *)field = (String){ .data = xmemdupz(v, vlen), .size = vlen }; // owned; keyset is stored
// A repeated key is "last wins" (e.g. 'diffopt' "inline:char,inline:word"); free the old one.
xfree(((String *)field)->data);
*(String *)field = (String){ .data = xmemdupz(v, vlen), .size = vlen }; // owned by the keyset
break;
default:
break;
@@ -2333,64 +2336,6 @@ void opt_fill(const char *value, FieldHashfn get_field, void *out)
}
}
/// Serializes a keyset to ":set" string form (inverse of `opt_fill()`), for storage and `:set opt?`.
/// Emits set keys only: "key" for a true flag, "key:value" for a set num/enum. `table` is the
/// keyset's `KeySetLink` table.
///
/// XXX: Keys are emitted in a canonical (alphanum) order, so the string is deterministic regardless
/// of the order they were set in. The reified value is an unordered map; unlike Vim, the serialized
/// order is not insertion-order.
///
/// @return an owned string (caller frees).
char *opt_serialize(const void *keyset, const KeySetLink *table)
FUNC_ATTR_NONNULL_ALL
{
// Collect the set keys, then sort by name. `set[]` is bounded by the number of sub-options.
const KeySetLink *set[64];
int n = 0;
for (const KeySetLink *f = table; f->str != NULL; f++) {
if (!(((const OptKeySet *)keyset)->is_set_ & (1ULL << (unsigned)f->opt_index))) {
continue;
}
if (f->type == kObjectTypeBoolean && !*(const Boolean *)((const char *)keyset + f->ptr_off)) {
continue; // a false flag is simply absent
}
assert(n < (int)ARRAY_SIZE(set));
set[n++] = f;
}
for (int i = 1; i < n; i++) { // insertion sort (n is small)
const KeySetLink *cur = set[i];
int j = i - 1;
while (j >= 0 && strcmp(set[j]->str, cur->str) > 0) {
set[j + 1] = set[j];
j--;
}
set[j + 1] = cur;
}
garray_T ga;
ga_init(&ga, 1, 64);
for (int i = 0; i < n; i++) {
const KeySetLink *f = set[i];
const void *field = (const char *)keyset + f->ptr_off;
if (ga.ga_len > 0) {
ga_append(&ga, ',');
}
ga_concat(&ga, f->str);
if (f->type == kObjectTypeInteger) {
char buf[32];
snprintf(buf, sizeof(buf), ":%" PRId64, *(const Integer *)field);
ga_concat(&ga, buf);
} else if (f->type == kObjectTypeString) {
const String *s = field;
ga_append(&ga, ':');
ga_concat_len(&ga, s->data, s->size);
}
}
ga_append(&ga, NUL);
return ga.ga_data;
}
/// @return OK if "p" is a valid fileformat name, FAIL otherwise.
int check_ff_value(char *p)
{
test/functional/options/mousescroll_spec.lua
+1 -1
View File
@@ -62,7 +62,7 @@ describe("'mousescroll'", function()
end)
it('default set correctly', function()
eq('hor:6,ver:3', eval('&mousescroll'))
eq('ver:3,hor:6', eval('&mousescroll'))
eq(10, screenrow())
scroll('up')
test/old/testdir/test_options.vim
+18 -18
View File
@@ -2982,49 +2982,49 @@ 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('algorithm:histogram,filler,internal', &diffopt)
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('algorithm:patience,filler,internal', &diffopt)
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('algorithm:histogram,filler', &diffopt)
call assert_equal('filler,algorithm:histogram', &diffopt)
" += with non-colon item appends normally
set diffopt=internal,filler
set diffopt+=iwhite
call assert_equal('filler,internal,iwhite', &diffopt)
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('algorithm:myers,filler,internal', &diffopt)
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('algorithm:patience,filler,internal', &diffopt)
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('filler,internal', &diffopt)
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('filler,internal', &diffopt)
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('algorithm:patience,filler,internal', &diffopt)
call assert_equal('internal,filler,algorithm:patience', &diffopt)
" -= with multiple non-colon items (order independent)
set diffopt=internal,filler,closeoff
@@ -3039,22 +3039,22 @@ func Test_comma_option_key_value()
" -= 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('closeoff,filler,internal', &diffopt)
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('closeoff,filler,internal', &diffopt)
call assert_equal('internal,filler,closeoff', &diffopt)
" += with multiple non-colon items
set diffopt=internal,filler
set diffopt+=closeoff,iwhite
call assert_equal('closeoff,filler,internal,iwhite', &diffopt)
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('closeoff,filler,internal,iwhite', &diffopt)
call assert_equal('internal,filler,closeoff,iwhite', &diffopt)
" -= with multiple items including key match
set diffopt=internal,filler,algorithm:patience
@@ -3064,12 +3064,12 @@ func Test_comma_option_key_value()
" -= key match when item is at the beginning
set diffopt=algorithm:patience,internal,filler
set diffopt-=algorithm:
call assert_equal('filler,internal', &diffopt)
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('filler,internal', &diffopt)
call assert_equal('internal,filler', &diffopt)
" -= key match when item is the only item
set diffopt=algorithm:patience
@@ -3079,17 +3079,17 @@ func Test_comma_option_key_value()
" ^= prepends new item
set diffopt=internal,filler
set diffopt^=algorithm:histogram
call assert_equal('algorithm:histogram,filler,internal', &diffopt)
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,filler,internal', &diffopt)
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('algorithm:patience,filler,internal', &diffopt)
call assert_equal('internal,filler,algorithm:patience', &diffopt)
set diffopt&