refactor(options): replace p_force_(on|off) with immutable (#26209)

Problem: We use the `p_force_on` and `p_force_off` variables to check if a variable is immutable and what its default value is. This is not only hacky and unintuitive, but also is limited to only boolean options.

Solution: Replace `p_force_on` and `p_force_off` with an `immutable` property for options, which indicates if an option is immutable. Immutable options cannot be changed from their default value.

Ref: #25672.
This commit is contained in:
Famiu Haque 2023-11-28 06:15:26 +06:00 committed by GitHub
parent a314703cf1
commit 3a3474371b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 53 additions and 50 deletions

View File

@ -143,9 +143,13 @@ local function dump_option(i, o)
end
if o.varname then
w(' .var=&' .. o.varname)
-- Immutable options should directly point to the default value
elseif o.immutable then
w((' .var=&options[%u].def_val'):format(i - 1))
elseif #o.scope == 1 and o.scope[1] == 'window' then
w(' .var=VAR_WIN')
end
w(' .immutable=' .. (o.immutable and 'true' or 'false'))
if #o.scope == 1 and o.scope[1] == 'global' then
w(' .indir=PV_NONE')
else

View File

@ -1913,26 +1913,6 @@ static void apply_optionset_autocmd(int opt_idx, int opt_flags, OptVal oldval, O
reset_v_option_vars();
}
/// Ensure that options set to p_force_on cannot be disabled.
static const char *did_set_force_on(optset_T *args FUNC_ATTR_UNUSED)
{
if (p_force_on == false) {
p_force_on = true;
return e_unsupportedoption;
}
return NULL;
}
/// Ensure that options set to p_force_off cannot be enabled.
static const char *did_set_force_off(optset_T *args FUNC_ATTR_UNUSED)
{
if (p_force_off == true) {
p_force_off = false;
return e_unsupportedoption;
}
return NULL;
}
/// Process the updated 'arabic' option value.
static const char *did_set_arabic(optset_T *args)
{
@ -3195,6 +3175,7 @@ int findoption(const char *const arg)
return findoption_len(arg, strlen(arg));
}
/// Free an allocated OptVal.
void optval_free(OptVal o)
{
switch (o.type) {
@ -3211,6 +3192,7 @@ void optval_free(OptVal o)
}
}
/// Copy an OptVal.
OptVal optval_copy(OptVal o)
{
switch (o.type) {
@ -3224,6 +3206,27 @@ OptVal optval_copy(OptVal o)
UNREACHABLE;
}
/// Check if two option values are equal.
bool optval_equal(OptVal o1, OptVal o2)
{
if (o1.type != o2.type) {
return false;
}
switch (o1.type) {
case kOptValTypeNil:
return true;
case kOptValTypeBoolean:
return o1.data.boolean == o2.data.boolean;
case kOptValTypeNumber:
return o1.data.number == o2.data.number;
case kOptValTypeString:
return o1.data.string.size == o2.data.string.size
&& strequal(o1.data.string.data, o2.data.string.data);
}
UNREACHABLE;
}
/// Match type of OptVal with the type of the target option. Returns true if the types match and
/// false otherwise.
static bool optval_match_type(OptVal o, int opt_idx)
@ -3480,14 +3483,16 @@ OptVal get_option_value(const char *name, uint32_t *flagsp, int scope, bool *hid
return NIL_OPTVAL;
}
void *varp = get_varp_scope(&(options[opt_idx]), scope);
vimoption_T *opt = &options[opt_idx];
void *varp = get_varp_scope(opt, scope);
if (hidden != NULL) {
*hidden = varp == NULL;
}
if (flagsp != NULL) {
// Return the P_xxxx option flags.
*flagsp = options[opt_idx].flags;
*flagsp = opt->flags;
}
return optval_copy(optval_from_varp(opt_idx, varp));
@ -3542,7 +3547,6 @@ static const char *did_set_option(int opt_idx, void *varp, OptVal old_value, Opt
bool free_oldval = (opt->flags & P_ALLOCED);
bool value_changed = false;
opt_did_set_cb_T did_set_cb;
optset_T did_set_cb_args = {
.os_varp = varp,
.os_idx = opt_idx,
@ -3558,24 +3562,21 @@ static const char *did_set_option(int opt_idx, void *varp, OptVal old_value, Opt
.os_win = curwin
};
if ((int *)varp == &p_force_on) {
did_set_cb = did_set_force_on;
} else if ((int *)varp == &p_force_off) {
did_set_cb = did_set_force_off;
} else {
did_set_cb = opt->opt_did_set_cb;
// Disallow changing immutable options.
if (opt->immutable && !optval_equal(old_value, new_value)) {
errmsg = e_unsupportedoption;
}
// Disallow changing some options from secure mode
if ((secure || sandbox != 0) && (opt->flags & P_SECURE)) {
// Disallow changing some options from secure mode.
else if ((secure || sandbox != 0) && (opt->flags & P_SECURE)) {
errmsg = e_secure;
// Check for a "normal" directory or file name in some string options.
} else if (new_value.type == kOptValTypeString
&& check_illegal_path_names(*(char **)varp, opt->flags)) {
}
// Check for a "normal" directory or file name in some string options.
else if (new_value.type == kOptValTypeString
&& check_illegal_path_names(*(char **)varp, opt->flags)) {
errmsg = e_invarg;
} else if (did_set_cb != NULL) {
} else if (opt->opt_did_set_cb != NULL) {
// Invoke the option specific callback function to validate and apply the new value.
errmsg = did_set_cb(&did_set_cb_args);
errmsg = opt->opt_did_set_cb(&did_set_cb_args);
// The 'filetype' and 'syntax' option callback functions may change the os_value_changed field.
value_changed = did_set_cb_args.os_value_changed;
// The 'keymap', 'filetype' and 'syntax' option callback functions may change the

View File

@ -48,6 +48,7 @@ typedef struct vimoption {
///< local option: indirect option index
///< callback function to invoke after an option is modified to validate and
///< apply the new value.
bool immutable; ///< option value cannot be changed from the default value.
/// callback function to invoke after an option is modified to validate and
/// apply the new value.

View File

@ -456,7 +456,6 @@ EXTERN unsigned dy_flags;
#define DY_UHEX 0x004
// legacy flag, not used
#define DY_MSGSEP 0x008
EXTERN int p_ed; ///< 'edcompatible'
EXTERN char *p_ead; ///< 'eadirection'
EXTERN int p_emoji; ///< 'emoji'
EXTERN int p_ea; ///< 'equalalways'
@ -786,9 +785,6 @@ EXTERN int p_wb; ///< 'writebackup'
EXTERN OptInt p_wd; ///< 'writedelay'
EXTERN int p_cdh; ///< 'cdhome'
EXTERN int p_force_on; ///< options that cannot be turned off.
EXTERN int p_force_off; ///< options that cannot be turned on.
/// "indir" values for buffer-local options.
/// These need to be defined globally, so that the BV_COUNT can be used with
/// b_p_script_stx[].

View File

@ -6,6 +6,7 @@
--- @field varname? string
--- @field pv_name? string
--- @field type 'bool'|'number'|'string'
--- @field immutable? boolean
--- @field list? 'comma'|'onecomma'|'commacolon'|'onecommacolon'|'flags'|'flagscomma'
--- @field scope vim.option_scope[]
--- @field deny_duplicates? boolean
@ -1341,7 +1342,7 @@ return {
scope = { 'global' },
short_desc = N_('No description'),
type = 'bool',
varname = 'p_force_off',
immutable = true,
},
{
abbreviation = 'cpt',
@ -2299,7 +2300,7 @@ return {
scope = { 'global' },
short_desc = N_('No description'),
type = 'bool',
varname = 'p_force_off',
immutable = true,
},
{
abbreviation = 'emo',
@ -3885,7 +3886,7 @@ return {
scope = { 'global' },
short_desc = N_('No description'),
type = 'bool',
varname = 'p_force_off',
immutable = true,
},
{
abbreviation = 'hkp',
@ -3894,7 +3895,7 @@ return {
scope = { 'global' },
short_desc = N_('No description'),
type = 'bool',
varname = 'p_force_off',
immutable = true,
},
{
abbreviation = 'hls',
@ -4292,7 +4293,7 @@ return {
scope = { 'global' },
short_desc = N_('No description'),
type = 'bool',
varname = 'p_force_off',
immutable = true,
},
{
abbreviation = 'isf',
@ -6042,7 +6043,7 @@ return {
scope = { 'global' },
short_desc = N_('enable prompt in Ex mode'),
type = 'bool',
varname = 'p_force_on',
immutable = true,
},
{
abbreviation = 'pb',
@ -6299,7 +6300,7 @@ return {
scope = { 'global' },
short_desc = N_('No description'),
type = 'bool',
varname = 'p_force_on',
immutable = true,
},
{
defaults = { if_true = 2 },
@ -8824,7 +8825,7 @@ return {
scope = { 'global' },
short_desc = N_('No description'),
type = 'bool',
varname = 'p_force_off',
immutable = true,
},
{
abbreviation = 'tw',
@ -9069,7 +9070,7 @@ return {
scope = { 'global' },
short_desc = N_('No description'),
type = 'bool',
varname = 'p_force_on',
immutable = true,
},
{
abbreviation = 'udir',