mirror of
				https://github.com/neovim/neovim.git
				synced 2025-10-26 12:27:24 +00:00 
			
		
		
		
	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.
This commit is contained in:
		 Famiu Haque
					Famiu Haque
				
			
				
					committed by
					
						 Lewis Russell
						Lewis Russell
					
				
			
			
				
	
			
			
			 Lewis Russell
						Lewis Russell
					
				
			
						parent
						
							b192d58284
						
					
				
				
					commit
					c5f93d7ab0
				
			| @@ -843,11 +843,10 @@ static char *ex_let_option(char *arg, typval_T *const tv, const bool is_const, | |||||||
|     goto theend; |     goto theend; | ||||||
|   } |   } | ||||||
|  |  | ||||||
|   // Don't assume current and new values are of the same type in order to future-proof the code for |   // Current value and new value must have the same type. | ||||||
|   // when an option can have multiple types. |   assert(curval.type == newval.type); | ||||||
|   const bool is_num = ((curval.type == kOptValTypeNumber || curval.type == kOptValTypeBoolean) |   const bool is_num = curval.type == kOptValTypeNumber || curval.type == kOptValTypeBoolean; | ||||||
|                        && (newval.type == kOptValTypeNumber || newval.type == kOptValTypeBoolean)); |   const bool is_string = curval.type == kOptValTypeString; | ||||||
|   const bool is_string = curval.type == kOptValTypeString && newval.type == kOptValTypeString; |  | ||||||
|  |  | ||||||
|   if (op != NULL && *op != '=') { |   if (op != NULL && *op != '=') { | ||||||
|     if (!hidden && is_num) {  // number or bool |     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. | /// @return  Typval converted to OptVal. Must be freed by caller. | ||||||
| ///          Returns NIL_OPTVAL for invalid option name. | ///          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) | static OptVal tv_to_optval(typval_T *tv, OptIndex opt_idx, const char *option, bool *error) | ||||||
| { | { | ||||||
|   OptVal value = NIL_OPTVAL; |   OptVal value = NIL_OPTVAL; | ||||||
|   | |||||||
| @@ -312,21 +312,6 @@ local function opt_scope_enum(scope) | |||||||
|   return ('kOptScope%s'):format(lowercase_to_titlecase(scope)) |   return ('kOptScope%s'):format(lowercase_to_titlecase(scope)) | ||||||
| end | 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 | --- @param o vim.option_meta | ||||||
| --- @return string | --- @return string | ||||||
| local function get_scope_flags(o) | local function get_scope_flags(o) | ||||||
| @@ -427,8 +412,8 @@ local function dump_option(i, o) | |||||||
|   if o.abbreviation then |   if o.abbreviation then | ||||||
|     w('    .shortname=' .. cstr(o.abbreviation)) |     w('    .shortname=' .. cstr(o.abbreviation)) | ||||||
|   end |   end | ||||||
|  |   w('    .type=' .. opt_type_enum(o.type)) | ||||||
|   w('    .flags=' .. get_flags(o)) |   w('    .flags=' .. get_flags(o)) | ||||||
|   w('    .type_flags=' .. get_type_flags(o)) |  | ||||||
|   w('    .scope_flags=' .. get_scope_flags(o)) |   w('    .scope_flags=' .. get_scope_flags(o)) | ||||||
|   w('    .scope_idx=' .. get_scope_idx(o)) |   w('    .scope_idx=' .. get_scope_idx(o)) | ||||||
|   if o.enable_if then |   if o.enable_if then | ||||||
|   | |||||||
| @@ -3121,17 +3121,10 @@ bool optval_equal(OptVal o1, OptVal o2) | |||||||
|   UNREACHABLE; |   UNREACHABLE; | ||||||
| } | } | ||||||
|  |  | ||||||
| /// Get type of option. Does not support multitype options. | /// Get type of option. | ||||||
| static OptValType option_get_type(const OptIndex opt_idx) | static OptValType option_get_type(const OptIndex opt_idx) | ||||||
| { | { | ||||||
|   assert(!option_is_multitype(opt_idx)); |   return options[opt_idx].type; | ||||||
|  |  | ||||||
|   // 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; |  | ||||||
| } | } | ||||||
|  |  | ||||||
| /// Create OptVal from var pointer. | /// Create OptVal from var pointer. | ||||||
| @@ -3149,11 +3142,6 @@ OptVal optval_from_varp(OptIndex opt_idx, void *varp) | |||||||
|     return BOOLEAN_OPTVAL(curbufIsChanged()); |     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); |   OptValType type = option_get_type(opt_idx); | ||||||
|  |  | ||||||
|   switch (type) { |   switch (type) { | ||||||
| @@ -3264,33 +3252,6 @@ OptVal object_as_optval(Object o, bool *error) | |||||||
|   UNREACHABLE; |   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. | /// Check if option is hidden. | ||||||
| /// | /// | ||||||
| /// @param  opt_idx  Option index in options[] table. | /// @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; |          && 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. | /// Check if option supports a specific type. | ||||||
| bool option_has_type(OptIndex opt_idx, OptValType type) | bool option_has_type(OptIndex opt_idx, OptValType type) | ||||||
| { | { | ||||||
|   // Ensure that type flags variable can hold all types. |   return options[opt_idx].type == type; | ||||||
|   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); |  | ||||||
| } | } | ||||||
|  |  | ||||||
| /// Check if option supports a specific scope. | /// 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)) { |   } else if (!option_has_type(opt_idx, newval->type)) { | ||||||
|     char *rep = optval_to_cstr(*newval); |     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"), |     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(rep); | ||||||
|     xfree(valid_types); |  | ||||||
|     errmsg = errbuf; |     errmsg = errbuf; | ||||||
|   } else if (newval->type == kOptValTypeNumber) { |   } else if (newval->type == kOptValTypeNumber) { | ||||||
|     // Validate and bound check num option values. |     // Validate and bound check num option values. | ||||||
|   | |||||||
| @@ -54,9 +54,6 @@ typedef enum { | |||||||
|   kOptValTypeNumber, |   kOptValTypeNumber, | ||||||
|   kOptValTypeString, |   kOptValTypeString, | ||||||
| } OptValType; | } 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. | /// Scopes that an option can support. | ||||||
| typedef enum { | typedef enum { | ||||||
| @@ -99,7 +96,6 @@ typedef struct { | |||||||
|   int os_flags; |   int os_flags; | ||||||
|  |  | ||||||
|   /// Old value of the option. |   /// Old value of the option. | ||||||
|   /// TODO(famiu): Convert `os_oldval` and `os_newval` to `OptVal` to accommodate multitype options. |  | ||||||
|   OptValData os_oldval; |   OptValData os_oldval; | ||||||
|   /// New value of the option. |   /// New value of the option. | ||||||
|   OptValData os_newval; |   OptValData os_newval; | ||||||
| @@ -173,7 +169,7 @@ typedef struct { | |||||||
|   char *fullname;                    ///< full option name |   char *fullname;                    ///< full option name | ||||||
|   char *shortname;                   ///< permissible abbreviation |   char *shortname;                   ///< permissible abbreviation | ||||||
|   uint32_t flags;                    ///< see above |   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 |   OptScopeFlags scope_flags;         ///< option scope flags, see OptScope | ||||||
|   void *var;                         ///< global option: pointer to variable; |   void *var;                         ///< global option: pointer to variable; | ||||||
|                                      ///< window-local option: NULL; |                                      ///< window-local option: NULL; | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user