diff options
author | Famiu Haque <famiuhaque@proton.me> | 2024-12-28 14:55:22 +0600 |
---|---|---|
committer | Lewis Russell <me@lewisr.dev> | 2025-01-14 09:34:17 +0000 |
commit | c5f93d7ab04f93db1470d58ca1f70e947e716c2b (patch) | |
tree | 27c460ef4b486f91efdb8d89bb0d731c81239273 | |
parent | b192d58284a791c55f5ae000250fc948e9098d47 (diff) | |
download | rneovim-c5f93d7ab04f93db1470d58ca1f70e947e716c2b.tar.gz rneovim-c5f93d7ab04f93db1470d58ca1f70e947e716c2b.tar.bz2 rneovim-c5f93d7ab04f93db1470d58ca1f70e947e716c2b.zip |
refactor(options): remove code for multitype options
Problem: It was decided on Matrix chat that multitype options won't be necessary for Neovim options, and that options should only have a single canonical type. Therefore the code for supporting multitype options is unnecessary.
Solution: Remove the additional code that's used to provide multitype option support.
-rw-r--r-- | src/nvim/eval/vars.c | 11 | ||||
-rw-r--r-- | src/nvim/generators/gen_options.lua | 17 | ||||
-rw-r--r-- | src/nvim/option.c | 65 | ||||
-rw-r--r-- | src/nvim/option_defs.h | 6 |
4 files changed, 11 insertions, 88 deletions
diff --git a/src/nvim/eval/vars.c b/src/nvim/eval/vars.c index b9b5a055fb..012d23b567 100644 --- a/src/nvim/eval/vars.c +++ b/src/nvim/eval/vars.c @@ -843,11 +843,10 @@ static char *ex_let_option(char *arg, typval_T *const tv, const bool is_const, goto theend; } - // Don't assume current and new values are of the same type in order to future-proof the code for - // when an option can have multiple types. - const bool is_num = ((curval.type == kOptValTypeNumber || curval.type == kOptValTypeBoolean) - && (newval.type == kOptValTypeNumber || newval.type == kOptValTypeBoolean)); - const bool is_string = curval.type == kOptValTypeString && newval.type == kOptValTypeString; + // Current value and new value must have the same type. + assert(curval.type == newval.type); + const bool is_num = curval.type == kOptValTypeNumber || curval.type == kOptValTypeBoolean; + const bool is_string = curval.type == kOptValTypeString; if (op != NULL && *op != '=') { if (!hidden && is_num) { // number or bool @@ -1900,8 +1899,6 @@ static void getwinvar(typval_T *argvars, typval_T *rettv, int off) /// /// @return Typval converted to OptVal. Must be freed by caller. /// Returns NIL_OPTVAL for invalid option name. -/// -/// TODO(famiu): Refactor this to support multitype options. static OptVal tv_to_optval(typval_T *tv, OptIndex opt_idx, const char *option, bool *error) { OptVal value = NIL_OPTVAL; diff --git a/src/nvim/generators/gen_options.lua b/src/nvim/generators/gen_options.lua index c79683dc00..0298381ece 100644 --- a/src/nvim/generators/gen_options.lua +++ b/src/nvim/generators/gen_options.lua @@ -314,21 +314,6 @@ end --- @param o vim.option_meta --- @return string -local function get_type_flags(o) - local opt_types = (type(o.type) == 'table') and o.type or { o.type } - local type_flags = '0' - assert(type(opt_types) == 'table') - - for _, opt_type in ipairs(opt_types) do - assert(type(opt_type) == 'string') - type_flags = ('%s | (1 << %s)'):format(type_flags, opt_type_enum(opt_type)) - end - - return type_flags -end - ---- @param o vim.option_meta ---- @return string local function get_scope_flags(o) local scope_flags = '0' @@ -427,8 +412,8 @@ local function dump_option(i, o) if o.abbreviation then w(' .shortname=' .. cstr(o.abbreviation)) end + w(' .type=' .. opt_type_enum(o.type)) w(' .flags=' .. get_flags(o)) - w(' .type_flags=' .. get_type_flags(o)) w(' .scope_flags=' .. get_scope_flags(o)) w(' .scope_idx=' .. get_scope_idx(o)) if o.enable_if then diff --git a/src/nvim/option.c b/src/nvim/option.c index 551ea0be20..593ac62172 100644 --- a/src/nvim/option.c +++ b/src/nvim/option.c @@ -3121,17 +3121,10 @@ bool optval_equal(OptVal o1, OptVal o2) UNREACHABLE; } -/// Get type of option. Does not support multitype options. +/// Get type of option. static OptValType option_get_type(const OptIndex opt_idx) { - assert(!option_is_multitype(opt_idx)); - - // If the option only supports a single type, it means that the index of the option's type flag - // corresponds to the value of the type enum. So get the index of the type flag using xctz() and - // use that as the option's type. - OptValType type = xctz(options[opt_idx].type_flags); - assert(type > kOptValTypeNil && type < kOptValTypeSize); - return type; + return options[opt_idx].type; } /// Create OptVal from var pointer. @@ -3149,11 +3142,6 @@ OptVal optval_from_varp(OptIndex opt_idx, void *varp) return BOOLEAN_OPTVAL(curbufIsChanged()); } - if (option_is_multitype(opt_idx)) { - // Multitype options are stored as OptVal. - return *(OptVal *)varp; - } - OptValType type = option_get_type(opt_idx); switch (type) { @@ -3264,33 +3252,6 @@ OptVal object_as_optval(Object o, bool *error) UNREACHABLE; } -/// Get an allocated string containing a list of valid types for an option. -/// For options with a singular type, it returns the name of the type. For options with multiple -/// possible types, it returns a slash separated list of types. For example, if an option can be a -/// number, boolean or string, the function returns "number/boolean/string" -static char *option_get_valid_types(OptIndex opt_idx) -{ - StringBuilder str = KV_INITIAL_VALUE; - kv_resize(str, 32); - - // Iterate through every valid option value type and check if the option supports that type - for (OptValType type = 0; type < kOptValTypeSize; type++) { - if (option_has_type(opt_idx, type)) { - const char *typename = optval_type_get_name(type); - - if (str.size == 0) { - kv_concat(str, typename); - } else { - kv_printf(str, "/%s", typename); - } - } - } - - // Ensure that the string is NUL-terminated. - kv_push(str, NUL); - return str.items; -} - /// Check if option is hidden. /// /// @param opt_idx Option index in options[] table. @@ -3303,25 +3264,10 @@ bool is_option_hidden(OptIndex opt_idx) && options[opt_idx].var == &options[opt_idx].def_val.data; } -/// Check if option is multitype (supports multiple types). -static bool option_is_multitype(OptIndex opt_idx) -{ - const OptTypeFlags type_flags = get_option(opt_idx)->type_flags; - assert(type_flags != 0); - return !is_power_of_two(type_flags); -} - /// Check if option supports a specific type. bool option_has_type(OptIndex opt_idx, OptValType type) { - // Ensure that type flags variable can hold all types. - STATIC_ASSERT(kOptValTypeSize <= sizeof(OptTypeFlags) * 8, - "Option type_flags cannot fit all option types"); - // Ensure that the type is valid before accessing type_flags. - assert(type > kOptValTypeNil && type < kOptValTypeSize); - // Bitshift 1 by the value of type to get the type's corresponding flag, and check if it's set in - // the type_flags bit field. - return get_option(opt_idx)->type_flags & (1 << type); + return options[opt_idx].type == type; } /// Check if option supports a specific scope. @@ -3658,11 +3604,10 @@ static const char *validate_option_value(const OptIndex opt_idx, OptVal *newval, } } else if (!option_has_type(opt_idx, newval->type)) { char *rep = optval_to_cstr(*newval); - char *valid_types = option_get_valid_types(opt_idx); + const char *type_str = optval_type_get_name(opt->type); snprintf(errbuf, IOSIZE, _("Invalid value for option '%s': expected %s, got %s %s"), - opt->fullname, valid_types, optval_type_get_name(newval->type), rep); + opt->fullname, type_str, optval_type_get_name(newval->type), rep); xfree(rep); - xfree(valid_types); errmsg = errbuf; } else if (newval->type == kOptValTypeNumber) { // Validate and bound check num option values. diff --git a/src/nvim/option_defs.h b/src/nvim/option_defs.h index 832e03148a..2b51547004 100644 --- a/src/nvim/option_defs.h +++ b/src/nvim/option_defs.h @@ -54,9 +54,6 @@ typedef enum { kOptValTypeNumber, kOptValTypeString, } OptValType; -/// Always update this whenever a new option type is added. -#define kOptValTypeSize (kOptValTypeString + 1) -typedef uint32_t OptTypeFlags; /// Scopes that an option can support. typedef enum { @@ -99,7 +96,6 @@ typedef struct { int os_flags; /// Old value of the option. - /// TODO(famiu): Convert `os_oldval` and `os_newval` to `OptVal` to accommodate multitype options. OptValData os_oldval; /// New value of the option. OptValData os_newval; @@ -173,7 +169,7 @@ typedef struct { char *fullname; ///< full option name char *shortname; ///< permissible abbreviation uint32_t flags; ///< see above - OptTypeFlags type_flags; ///< option type flags, see OptValType + OptValType type; ///< option type OptScopeFlags scope_flags; ///< option scope flags, see OptScope void *var; ///< global option: pointer to variable; ///< window-local option: NULL; |