feat(options)!: disallow setting hidden options #28400

Problem:
There are three different ways of marking an option as hidden, `enable_if
= false`, `hidden = true` and `immutable = true`. These also have different
behaviors. Options hidden with `enable_if = false` can't have their value
fetched using Vim script or the API, but options hidden with `hidden = true` or
`immutable = true` can. On the other hand, options with `hidden = true` do not
error when trying to set their value, but options with `immutable = true` do.

Solution:
Remove `enable_if = false`, remove the `hidden` property for options, and use
`immutable = true` to mark an option as hidden instead. Also make hidden option
variable pointers always point to the default value, which allows fetching the
value of every hidden option using Vim script and the API. This does also mean
that trying to set a hidden option will now give an error instead of just being
ignored.
This commit is contained in:
Famiu Haque
2024-11-04 19:00:12 +06:00
committed by GitHub
parent 04d178053f
commit a27419f3fc
18 changed files with 102 additions and 99 deletions

View File

@@ -647,10 +647,6 @@ static bool option_has_scope(OptIndex opt_idx, OptReqScope req_scope)
vimoption_T *opt = get_option(opt_idx);
// Hidden option.
if (opt->var == NULL) {
return false;
}
// TTY option.
if (is_tty_option(opt->fullname)) {
return req_scope == kOptReqGlobal;

View File

@@ -80,7 +80,7 @@ static int validate_option_value_args(Dict(option) *opts, char *name, OptIndex *
*opt_idxp = find_option(name);
int flags = get_option_attrs(*opt_idxp);
if (flags == 0) {
// hidden or unknown option
// unknown option
api_set_error(err, kErrorTypeValidation, "Unknown option '%s'", name);
} else if (*req_scope == kOptReqBuf || *req_scope == kOptReqWin) {
// if 'buf' or 'win' is passed, make sure the option supports it
@@ -175,7 +175,6 @@ Object nvim_get_option_value(String name, Dict(option) *opts, Error *err)
}
OptVal value = get_option_value_for(opt_idx, scope, req_scope, from, err);
bool hidden = is_option_hidden(opt_idx);
if (ftbuf != NULL) {
// restore curwin/curbuf and a few other things
@@ -189,7 +188,7 @@ Object nvim_get_option_value(String name, Dict(option) *opts, Error *err)
goto err;
}
VALIDATE_S(!hidden && value.type != kOptValTypeNil, "option", name.data, {
VALIDATE_S(value.type != kOptValTypeNil, "option", name.data, {
goto err;
});

View File

@@ -191,23 +191,17 @@ local function dump_option(i, o)
w(get_cond(o.enable_if))
end
-- An option cannot be both hidden and immutable.
assert(not o.hidden or not o.immutable)
local has_var = true
if o.varname then
w(' .var=&' .. o.varname)
elseif o.hidden or o.immutable then
-- Hidden and immutable options can directly point to the default value.
elseif o.immutable then
-- Immutable options can directly point to the default value.
w((' .var=&options[%u].def_val.data'):format(i - 1))
elseif #o.scope == 1 and o.scope[1] == 'window' then
w(' .var=VAR_WIN')
else
has_var = false
-- Option must be immutable or have a variable.
assert(false)
end
-- `enable_if = false` should be present iff there is no variable.
assert((o.enable_if == false) == not has_var)
w(' .hidden=' .. (o.hidden and 'true' or 'false'))
w(' .immutable=' .. (o.immutable and 'true' or 'false'))
if #o.scope == 1 and o.scope[1] == 'global' then
w(' .indir=PV_NONE')
@@ -237,7 +231,10 @@ local function dump_option(i, o)
end
if o.enable_if then
w('#else')
w(' .var=NULL')
-- Hidden option directly points to default value.
w((' .var=&options[%u].def_val.data'):format(i - 1))
-- Option is always immutable on the false branch of `enable_if`.
w(' .immutable=true')
w(' .indir=PV_NONE')
w('#endif')
end

View File

@@ -565,13 +565,16 @@ static char *find_dup_item(char *origval, const char *newval, const size_t newva
void free_all_options(void)
{
for (OptIndex opt_idx = 0; opt_idx < kOptIndexCount; opt_idx++) {
if (options[opt_idx].indir == PV_NONE) {
bool hidden = is_option_hidden(opt_idx);
if (options[opt_idx].indir == PV_NONE || hidden) {
// global option: free value and default value.
if (options[opt_idx].var != NULL) {
// hidden option: free default value only.
if (!hidden) {
optval_free(optval_from_varp(opt_idx, options[opt_idx].var));
}
} else if (options[opt_idx].var != VAR_WIN) {
// buffer-local option: free global value
// buffer-local option: free global value.
optval_free(optval_from_varp(opt_idx, options[opt_idx].var));
}
optval_free(options[opt_idx].def_val);
@@ -1244,16 +1247,6 @@ static void do_one_set_option(int opt_flags, char **argp, bool *did_show, char *
uint32_t flags = 0; // flags for current option
void *varp = NULL; // pointer to variable for current option
if (options[opt_idx].var == NULL) { // hidden option: skip
// Only give an error message when requesting the value of
// a hidden option, ignore setting it.
if (vim_strchr("=:!&<", nextchar) == NULL
&& (!option_has_type(opt_idx, kOptValTypeBoolean) || nextchar == '?')) {
*errmsg = e_unsupportedoption;
}
return;
}
flags = options[opt_idx].flags;
varp = get_varp_scope(&(options[opt_idx]), opt_flags);
@@ -1325,11 +1318,6 @@ static void do_one_set_option(int opt_flags, char **argp, bool *did_show, char *
}
}
// Don't try to change hidden option.
if (varp == NULL) {
return;
}
OptVal newval = get_option_newval(opt_idx, opt_flags, prefix, argp, nextchar, op, flags, varp,
errbuf, errbuflen, errmsg);
@@ -1587,7 +1575,7 @@ char *find_shada_parameter(int type)
static char *option_expand(OptIndex opt_idx, char *val)
{
// if option doesn't need expansion nothing to do
if (!(options[opt_idx].flags & kOptFlagExpand) || options[opt_idx].var == NULL) {
if (!(options[opt_idx].flags & kOptFlagExpand) || is_option_hidden(opt_idx)) {
return NULL;
}
@@ -2863,6 +2851,8 @@ static const char *validate_num_option(OptIndex opt_idx, void *varp, OptInt *new
} else if (value > p_wiw) {
return e_winwidth;
}
} else if (varp == &p_mco) {
*newval = MAX_MCO;
} else if (varp == &p_titlelen) {
if (value < 0) {
return e_positive;
@@ -3161,6 +3151,7 @@ static OptValType option_get_type(const OptIndex opt_idx)
///
/// @return Option value stored in varp.
OptVal optval_from_varp(OptIndex opt_idx, void *varp)
FUNC_ATTR_NONNULL_ARG(2)
{
// Special case: 'modified' is b_changed, but we also want to consider it set when 'ff' or 'fenc'
// changed.
@@ -3170,7 +3161,7 @@ OptVal optval_from_varp(OptIndex opt_idx, void *varp)
if (option_is_multitype(opt_idx)) {
// Multitype options are stored as OptVal.
return varp == NULL ? NIL_OPTVAL : *(OptVal *)varp;
return *(OptVal *)varp;
}
OptValType type = option_get_type(opt_idx);
@@ -3179,11 +3170,11 @@ OptVal optval_from_varp(OptIndex opt_idx, void *varp)
case kOptValTypeNil:
return NIL_OPTVAL;
case kOptValTypeBoolean:
return BOOLEAN_OPTVAL(varp == NULL ? false : TRISTATE_FROM_INT(*(int *)varp));
return BOOLEAN_OPTVAL(TRISTATE_FROM_INT(*(int *)varp));
case kOptValTypeNumber:
return NUMBER_OPTVAL(varp == NULL ? 0 : *(OptInt *)varp);
return NUMBER_OPTVAL(*(OptInt *)varp);
case kOptValTypeString:
return STRING_OPTVAL(varp == NULL ? (String)STRING_INIT : cstr_as_string(*(char **)varp));
return STRING_OPTVAL(cstr_as_string(*(char **)varp));
}
UNREACHABLE;
}
@@ -3319,7 +3310,10 @@ static char *option_get_valid_types(OptIndex opt_idx)
/// @return True if option is hidden, false otherwise. Returns false if option name is invalid.
bool is_option_hidden(OptIndex opt_idx)
{
return opt_idx == kOptInvalid ? false : get_varp(&options[opt_idx]) == NULL;
// Hidden options are always immutable and point to their default value
return opt_idx == kOptInvalid
? false
: (options[opt_idx].immutable && options[opt_idx].var == &options[opt_idx].def_val.data);
}
static inline bool option_is_global_local(OptIndex opt_idx)
@@ -3459,8 +3453,8 @@ static const char *did_set_option(OptIndex opt_idx, void *varp, OptVal old_value
.os_win = curwin
};
if (direct || opt->hidden) {
// Don't do any extra processing if setting directly or if option is hidden.
if (direct) {
// Don't do any extra processing if setting directly.
}
// Disallow changing immutable options.
else if (opt->immutable && !optval_equal(old_value, new_value)) {
@@ -3489,7 +3483,7 @@ static const char *did_set_option(OptIndex opt_idx, void *varp, OptVal old_value
// If option is hidden or if an error is detected, restore the previous value and don't do any
// further processing.
if (opt->hidden || errmsg != NULL) {
if (errmsg != NULL) {
set_option_varp(opt_idx, varp, old_value, true);
// When resetting some values, need to act on it.
if (restore_chartab) {
@@ -3749,7 +3743,7 @@ void set_option_direct(OptIndex opt_idx, OptVal value, int opt_flags, scid_T set
vimoption_T *opt = get_option(opt_idx);
if (opt->var == NULL) {
if (is_option_hidden(opt_idx)) {
return;
}
@@ -3960,11 +3954,6 @@ int get_option_attrs(OptIndex opt_idx)
vimoption_T *opt = get_option(opt_idx);
// Hidden option
if (opt->var == NULL) {
return 0;
}
int attrs = 0;
if (opt->indir == PV_NONE || (opt->indir & PV_BOTH)) {
@@ -3976,6 +3965,7 @@ int get_option_attrs(OptIndex opt_idx)
attrs |= SOPT_BUF;
}
assert(attrs != 0);
return attrs;
}
@@ -4143,8 +4133,8 @@ static int optval_default(OptIndex opt_idx, void *varp)
{
vimoption_T *opt = &options[opt_idx];
// Hidden or immutable options always use their default value.
if (varp == NULL || opt->hidden || opt->immutable) {
// Hidden options always use their default value.
if (is_option_hidden(opt_idx)) {
return true;
}
@@ -4554,9 +4544,9 @@ void *get_option_varp_scope_from(OptIndex opt_idx, int scope, buf_T *buf, win_T
void *get_varp_from(vimoption_T *p, buf_T *buf, win_T *win)
{
// hidden option, always return NULL
if (p->var == NULL) {
return NULL;
// hidden options always use the same var pointer
if (is_option_hidden(get_opt_idx(p))) {
return p->var;
}
switch ((int)p->indir) {
@@ -5456,7 +5446,7 @@ void set_context_in_set_cmd(expand_T *xp, char *arg, int opt_flags)
}
nextchar = *p;
opt_idx = find_option_len(arg, (size_t)(p - arg));
if (opt_idx == kOptInvalid || options[opt_idx].var == NULL) {
if (opt_idx == kOptInvalid || is_option_hidden(opt_idx)) {
xp->xp_context = EXPAND_NOTHING;
return;
}
@@ -5680,7 +5670,7 @@ int ExpandSettings(expand_T *xp, regmatch_T *regmatch, char *fuzzystr, int *numM
char *str;
for (OptIndex opt_idx = 0; opt_idx < kOptIndexCount; opt_idx++) {
str = options[opt_idx].fullname;
if (options[opt_idx].var == NULL) {
if (is_option_hidden(opt_idx)) {
continue;
}
if (xp->xp_context == EXPAND_BOOL_SETTINGS
@@ -5928,6 +5918,7 @@ int ExpandSettingSubtract(expand_T *xp, regmatch_T *regmatch, int *numMatches, c
static void option_value2string(vimoption_T *opt, int scope)
{
void *varp = get_varp_scope(opt, scope);
assert(varp != NULL);
if (option_has_type(get_opt_idx(opt), kOptValTypeNumber)) {
OptInt wc = 0;
@@ -5944,9 +5935,8 @@ static void option_value2string(vimoption_T *opt, int scope)
}
} else { // string
varp = *(char **)(varp);
if (varp == NULL) { // Just in case.
NameBuff[0] = NUL;
} else if (opt->flags & kOptFlagExpand) {
if (opt->flags & kOptFlagExpand) {
home_replace(NULL, varp, NameBuff, MAXPATHL, false);
} else {
xstrlcpy(NameBuff, varp, MAXPATHL);

View File

@@ -49,7 +49,6 @@ typedef struct {
///< buffer-local option: global value
idopt_T indir; ///< global option: PV_NONE;
///< local option: indirect option index
bool hidden; ///< option is hidden, any attempt to set its value will be ignored.
bool immutable; ///< option is immutable, trying to set its value will give an error.
/// callback function to invoke after an option is modified to validate and

View File

@@ -530,6 +530,7 @@ EXTERN char *p_mef; ///< 'makeef'
EXTERN char *p_mp; ///< 'makeprg'
EXTERN char *p_mps; ///< 'matchpairs'
EXTERN OptInt p_mat; ///< 'matchtime'
EXTERN OptInt p_mco; ///< 'maxcombine'
EXTERN OptInt p_mfd; ///< 'maxfuncdepth'
EXTERN OptInt p_mmd; ///< 'maxmapdepth'
EXTERN OptInt p_mmp; ///< 'maxmempattern'

View File

@@ -13,7 +13,7 @@
--- @field list? 'comma'|'onecomma'|'commacolon'|'onecommacolon'|'flags'|'flagscomma'
--- @field scope vim.option_scope[]
--- @field deny_duplicates? boolean
--- @field enable_if? string|false
--- @field enable_if? string
--- @field defaults? vim.option_defaults
--- @field secure? true
--- @field noglob? true
@@ -87,11 +87,11 @@ return {
{
abbreviation = 'al',
defaults = { if_true = 224 },
enable_if = false,
full_name = 'aleph',
scope = { 'global' },
short_desc = N_('ASCII code of the letter Aleph (Hebrew)'),
type = 'number',
immutable = true,
},
{
abbreviation = 'ari',
@@ -789,11 +789,11 @@ return {
current Use the current directory.
{path} Use the specified directory
]=],
enable_if = false,
full_name = 'browsedir',
scope = { 'global' },
short_desc = N_('which directory to start browsing in'),
type = 'string',
immutable = true,
},
{
abbreviation = 'bh',
@@ -1493,7 +1493,7 @@ return {
cb = 'did_set_completeslash',
defaults = { if_true = '' },
desc = [=[
only for MS-Windows
only modifiable in MS-Windows
When this option is set it overrules 'shellslash' for completion:
- When this option is set to "slash", a forward slash is used for path
completion in insert mode. This is useful when editing HTML tag, or
@@ -3791,12 +3791,12 @@ return {
try to keep 'lines' and 'columns' the same when adding and
removing GUI components.
]=],
enable_if = false,
full_name = 'guioptions',
list = 'flags',
scope = { 'global' },
short_desc = N_('GUI: Which components and options are used'),
type = 'string',
immutable = true,
},
{
abbreviation = 'gtl',
@@ -3816,13 +3816,13 @@ return {
present in 'guioptions'. For the non-GUI tab pages line 'tabline' is
used.
]=],
enable_if = false,
full_name = 'guitablabel',
modelineexpr = true,
redraw = { 'current_window' },
scope = { 'global' },
short_desc = N_('GUI: custom label for a tab page'),
type = 'string',
immutable = true,
},
{
abbreviation = 'gtt',
@@ -3835,12 +3835,12 @@ return {
let &guitabtooltip = "line one\nline two"
<
]=],
enable_if = false,
full_name = 'guitabtooltip',
redraw = { 'current_window' },
scope = { 'global' },
short_desc = N_('GUI: custom tooltip for a tab page'),
type = 'string',
immutable = true,
},
{
abbreviation = 'hf',
@@ -4082,11 +4082,11 @@ return {
English characters directly, e.g., when it's used to type accented
characters with dead keys.
]=],
enable_if = false,
full_name = 'imcmdline',
scope = { 'global' },
short_desc = N_('use IM when starting to edit a command line'),
type = 'boolean',
immutable = true,
},
{
abbreviation = 'imd',
@@ -4100,11 +4100,11 @@ return {
Currently this option is on by default for SGI/IRIX machines. This
may change in later releases.
]=],
enable_if = false,
full_name = 'imdisable',
scope = { 'global' },
short_desc = N_('do not use the IM in any mode'),
type = 'boolean',
immutable = true,
},
{
abbreviation = 'imi',
@@ -5236,7 +5236,7 @@ return {
scope = { 'global' },
short_desc = N_('maximum nr of combining characters displayed'),
type = 'number',
hidden = true,
varname = 'p_mco',
},
{
abbreviation = 'mfd',
@@ -5732,13 +5732,13 @@ return {
indicate no input when the hit-enter prompt is displayed (since
clicking the mouse has no effect in this state.)
]=],
enable_if = false,
full_name = 'mouseshape',
list = 'onecomma',
scope = { 'global' },
short_desc = N_('shape of the mouse pointer in different modes'),
tags = { 'E547' },
type = 'string',
immutable = true,
},
{
abbreviation = 'mouset',
@@ -5898,11 +5898,11 @@ return {
Note that on Windows editing "aux.h", "lpt1.txt" and the like also
result in editing a device.
]=],
enable_if = false,
full_name = 'opendevice',
scope = { 'global' },
short_desc = N_('allow reading/writing devices on MS-Windows'),
type = 'boolean',
immutable = true,
},
{
abbreviation = 'opfunc',
@@ -5975,11 +5975,11 @@ return {
{
abbreviation = 'pt',
defaults = { if_true = '' },
enable_if = false,
full_name = 'pastetoggle',
scope = { 'global' },
short_desc = N_('No description'),
type = 'string',
immutable = true,
},
{
abbreviation = 'pex',
@@ -7270,9 +7270,14 @@ return {
{
abbreviation = 'ssl',
cb = 'did_set_shellslash',
defaults = { if_true = false },
defaults = {
condition = 'MSWIN',
if_true = false,
if_false = true,
doc = 'on, Windows: off',
},
desc = [=[
only for MS-Windows
only modifiable in MS-Windows
When set, a forward slash is used when expanding file names. This is
useful when a Unix-like shell is used instead of cmd.exe. Backward
slashes can still be typed, but they are changed to forward slashes by
@@ -8885,11 +8890,11 @@ return {
{
abbreviation = 'tenc',
defaults = { if_true = '' },
enable_if = false,
full_name = 'termencoding',
scope = { 'global' },
short_desc = N_('Terminal encoding'),
type = 'string',
immutable = true,
},
{
abbreviation = 'tgc',