From 4b0b798ebae8f4f618c02b3adf30077c9dcc48b0 Mon Sep 17 00:00:00 2001 From: Lewis Russell Date: Tue, 11 Jul 2023 10:26:11 +0100 Subject: refactor(option.c): move validation logic to function --- src/nvim/option.c | 146 ++++++++++++++++++++++++++++-------------------------- 1 file changed, 77 insertions(+), 69 deletions(-) (limited to 'src') diff --git a/src/nvim/option.c b/src/nvim/option.c index 716fd3775a..ffceeb6a5a 100644 --- a/src/nvim/option.c +++ b/src/nvim/option.c @@ -2981,181 +2981,189 @@ static const char *check_num_option_bounds(long *pp, long old_value, long old_Ro return errmsg; } -/// Set the value of a number option, taking care of side effects -/// -/// @param[in] opt_idx Option index in options[] table. -/// @param[out] varp Pointer to the option variable. -/// @param[in] value New value. -/// @param errbuf Buffer for error messages. -/// @param[in] errbuflen Length of `errbuf`. -/// @param[in] opt_flags OPT_LOCAL, OPT_GLOBAL or OPT_MODELINE. -/// -/// @return NULL on success, error message on error. -static const char *set_num_option(int opt_idx, void *varp, long value, char *errbuf, - size_t errbuflen, int opt_flags) +/// Options that need some validation. +static const char *validate_num_option(const long *pp, long *valuep) { - const char *errmsg = NULL; - long old_value = *(long *)varp; - long old_global_value = 0; // only used when setting a local and global option - long old_Rows = Rows; // remember old Rows - long *pp = (long *)varp; - - // Disallow changing some options from secure mode. - if ((secure || sandbox != 0) && (options[opt_idx].flags & P_SECURE)) { - return e_secure; - } - - // Save the global value before changing anything. This is needed as for - // a global-only option setting the "local value" in fact sets the global - // value (since there is only one value). - if ((opt_flags & (OPT_LOCAL | OPT_GLOBAL)) == 0) { - old_global_value = *(long *)get_varp_scope(&(options[opt_idx]), OPT_GLOBAL); - } + long value = *valuep; // Many number options assume their value is in the signed int range. if (value < INT_MIN || value > INT_MAX) { return e_invarg; } - // Options that need some validation. if (pp == &p_wh) { if (value < 1) { - errmsg = e_positive; + return e_positive; } else if (p_wmh > value) { - errmsg = e_winheight; + return e_winheight; } } else if (pp == &p_hh) { if (value < 0) { - errmsg = e_positive; + return e_positive; } } else if (pp == &p_wmh) { if (value < 0) { - errmsg = e_positive; + return e_positive; } else if (value > p_wh) { - errmsg = e_winheight; + return e_winheight; } } else if (pp == &p_wiw) { if (value < 1) { - errmsg = e_positive; + return e_positive; } else if (p_wmw > value) { - errmsg = e_winwidth; + return e_winwidth; } } else if (pp == &p_wmw) { if (value < 0) { - errmsg = e_positive; + return e_positive; } else if (value > p_wiw) { - errmsg = e_winwidth; + return e_winwidth; } } else if (pp == &p_mco) { - value = MAX_MCO; + *valuep = MAX_MCO; } else if (pp == &p_titlelen) { if (value < 0) { - errmsg = e_positive; + return e_positive; } } else if (pp == &p_uc) { if (value < 0) { - errmsg = e_positive; + return e_positive; } } else if (pp == &p_ch) { if (value < 0) { - errmsg = e_positive; + return e_positive; } else { p_ch_was_zero = value == 0; } } else if (pp == &p_tm) { if (value < 0) { - errmsg = e_positive; + return e_positive; } } else if (pp == &p_hi) { if (value < 0) { - errmsg = e_positive; + return e_positive; } else if (value > 10000) { - errmsg = e_invarg; + return e_invarg; } } else if (pp == &p_pyx) { if (value == 0) { - value = 3; + *valuep = 3; } else if (value != 3) { - errmsg = e_invarg; + return e_invarg; } } else if (pp == &p_re) { if (value < 0 || value > 2) { - errmsg = e_invarg; + return e_invarg; } } else if (pp == &p_report) { if (value < 0) { - errmsg = e_positive; + return e_positive; } } else if (pp == &p_so) { if (value < 0 && full_screen) { - errmsg = e_positive; + return e_positive; } } else if (pp == &p_siso) { if (value < 0 && full_screen) { - errmsg = e_positive; + return e_positive; } } else if (pp == &p_cwh) { if (value < 1) { - errmsg = e_positive; + return e_positive; } } else if (pp == &p_ut) { if (value < 0) { - errmsg = e_positive; + return e_positive; } } else if (pp == &p_ss) { if (value < 0) { - errmsg = e_positive; + return e_positive; } } else if (pp == &curwin->w_p_fdl || pp == &curwin->w_allbuf_opt.wo_fdl) { if (value < 0) { - errmsg = e_positive; + return e_positive; } } else if (pp == &curwin->w_p_cole || pp == &curwin->w_allbuf_opt.wo_cole) { if (value < 0) { - errmsg = e_positive; + return e_positive; } else if (value > 3) { - errmsg = e_invarg; + return e_invarg; } } else if (pp == &curwin->w_p_nuw || pp == &curwin->w_allbuf_opt.wo_nuw) { if (value < 1) { - errmsg = e_positive; + return e_positive; } else if (value > MAX_NUMBERWIDTH) { - errmsg = e_invarg; + return e_invarg; } } else if (pp == &curbuf->b_p_iminsert || pp == &p_iminsert) { if (value < 0 || value > B_IMODE_LAST) { - errmsg = e_invarg; + return e_invarg; } } else if (pp == &curbuf->b_p_imsearch || pp == &p_imsearch) { if (value < -1 || value > B_IMODE_LAST) { - errmsg = e_invarg; + return e_invarg; } } else if (pp == &curbuf->b_p_channel || pp == &p_channel) { - errmsg = e_invarg; + return e_invarg; } else if (pp == &curbuf->b_p_scbk || pp == &p_scbk) { if (value < -1 || value > SB_MAX) { - errmsg = e_invarg; + return e_invarg; } } else if (pp == &curbuf->b_p_sw || pp == &p_sw) { if (value < 0) { - errmsg = e_positive; + return e_positive; } } else if (pp == &curbuf->b_p_ts || pp == &p_ts) { if (value < 1) { - errmsg = e_positive; + return e_positive; } else if (value > TABSTOP_MAX) { - errmsg = e_invarg; + return e_invarg; } } else if (pp == &curbuf->b_p_tw || pp == &p_tw) { if (value < 0) { - errmsg = e_positive; + return e_positive; } } else if (pp == &p_wd) { if (value < 0) { - errmsg = e_positive; + return e_positive; } } + return NULL; +} + +/// Set the value of a number option, taking care of side effects +/// +/// @param[in] opt_idx Option index in options[] table. +/// @param[out] varp Pointer to the option variable. +/// @param[in] value New value. +/// @param errbuf Buffer for error messages. +/// @param[in] errbuflen Length of `errbuf`. +/// @param[in] opt_flags OPT_LOCAL, OPT_GLOBAL or OPT_MODELINE. +/// +/// @return NULL on success, error message on error. +static const char *set_num_option(int opt_idx, void *varp, long value, char *errbuf, + size_t errbuflen, int opt_flags) +{ + long old_value = *(long *)varp; + long old_global_value = 0; // only used when setting a local and global option + long old_Rows = Rows; // remember old Rows + long *pp = (long *)varp; + + // Disallow changing some options from secure mode. + if ((secure || sandbox != 0) && (options[opt_idx].flags & P_SECURE)) { + return e_secure; + } + + // Save the global value before changing anything. This is needed as for + // a global-only option setting the "local value" in fact sets the global + // value (since there is only one value). + if ((opt_flags & (OPT_LOCAL | OPT_GLOBAL)) == 0) { + old_global_value = *(long *)get_varp_scope(&(options[opt_idx]), OPT_GLOBAL); + } + + const char *errmsg = validate_num_option(pp, &value); + // Don't change the value and return early if validation failed. if (errmsg != NULL) { return errmsg; -- cgit From 3b02e1281ad9fff8f8817868996e83e9f69cb8a1 Mon Sep 17 00:00:00 2001 From: Lewis Russell Date: Wed, 12 Jul 2023 13:56:33 +0100 Subject: refactor(option.c): remove did_set_string_option alias --- src/nvim/option.c | 2 +- src/nvim/optionstr.c | 17 +++++------------ 2 files changed, 6 insertions(+), 13 deletions(-) (limited to 'src') diff --git a/src/nvim/option.c b/src/nvim/option.c index ffceeb6a5a..ad0d6a0266 100644 --- a/src/nvim/option.c +++ b/src/nvim/option.c @@ -1172,7 +1172,7 @@ static void do_set_option_string(int opt_idx, int opt_flags, char **argp, int ne // Handle side effects, and set the global value for ":set" on local // options. Note: when setting 'syntax' or 'filetype' autocommands may // be triggered that can cause havoc. - *errmsg = did_set_string_option(opt_idx, (char **)varp, oldval, newval, + *errmsg = did_set_string_option(curbuf, curwin, opt_idx, (char **)varp, oldval, newval, errbuf, errbuflen, opt_flags, value_checked); diff --git a/src/nvim/optionstr.c b/src/nvim/optionstr.c index 07e2e5eed1..60ad246293 100644 --- a/src/nvim/optionstr.c +++ b/src/nvim/optionstr.c @@ -459,8 +459,8 @@ const char *set_string_option(const int opt_idx, const char *const value, const char *const saved_newval = xstrdup(s); int value_checked = false; - const char *const errmsg = did_set_string_option(opt_idx, varp, oldval, s, errbuf, errbuflen, - opt_flags, &value_checked); + const char *const errmsg = did_set_string_option(curbuf, curwin, opt_idx, varp, oldval, s, errbuf, + errbuflen, opt_flags, &value_checked); if (errmsg == NULL) { did_set_option(opt_idx, opt_flags, true, value_checked); } @@ -2066,9 +2066,9 @@ static void do_spelllang_source(win_T *win) /// @param value_checked value was checked to be safe, no need to set P_INSECURE /// /// @return NULL for success, or an untranslated error message for an error -static const char *did_set_string_option_for(buf_T *buf, win_T *win, int opt_idx, char **varp, - char *oldval, const char *value, char *errbuf, - size_t errbuflen, int opt_flags, int *value_checked) +const char *did_set_string_option(buf_T *buf, win_T *win, int opt_idx, char **varp, char *oldval, + const char *value, char *errbuf, size_t errbuflen, int opt_flags, + int *value_checked) { const char *errmsg = NULL; int restore_chartab = false; @@ -2186,13 +2186,6 @@ static const char *did_set_string_option_for(buf_T *buf, win_T *win, int opt_idx return errmsg; } -const char *did_set_string_option(int opt_idx, char **varp, char *oldval, char *value, char *errbuf, - size_t errbuflen, int opt_flags, int *value_checked) -{ - return did_set_string_option_for(curbuf, curwin, opt_idx, varp, oldval, value, errbuf, - errbuflen, opt_flags, value_checked); -} - /// Check an option that can be a range of string values. /// /// @param list when true: accept a list of values -- cgit From 9fdc4cdb64ee405c3d742b0397a7be6c963d09a0 Mon Sep 17 00:00:00 2001 From: Lewis Russell Date: Wed, 12 Jul 2023 14:16:29 +0100 Subject: refactor(optionstr.c): remove redundant argument --- src/nvim/option.c | 2 +- src/nvim/optionstr.c | 12 +++++------- 2 files changed, 6 insertions(+), 8 deletions(-) (limited to 'src') diff --git a/src/nvim/option.c b/src/nvim/option.c index ad0d6a0266..fb7c04d36b 100644 --- a/src/nvim/option.c +++ b/src/nvim/option.c @@ -1172,7 +1172,7 @@ static void do_set_option_string(int opt_idx, int opt_flags, char **argp, int ne // Handle side effects, and set the global value for ":set" on local // options. Note: when setting 'syntax' or 'filetype' autocommands may // be triggered that can cause havoc. - *errmsg = did_set_string_option(curbuf, curwin, opt_idx, (char **)varp, oldval, newval, + *errmsg = did_set_string_option(curbuf, curwin, opt_idx, (char **)varp, oldval, errbuf, errbuflen, opt_flags, value_checked); diff --git a/src/nvim/optionstr.c b/src/nvim/optionstr.c index 60ad246293..18065f05d4 100644 --- a/src/nvim/optionstr.c +++ b/src/nvim/optionstr.c @@ -437,7 +437,6 @@ const char *set_string_option(const int opt_idx, const char *const value, const return NULL; } - char *const s = xstrdup(value); char **const varp = (char **)get_varp_scope(opt, ((opt_flags & (OPT_LOCAL | OPT_GLOBAL)) == 0 ? ((opt->indir & PV_BOTH) ? OPT_GLOBAL : OPT_LOCAL) @@ -451,15 +450,15 @@ const char *set_string_option(const int opt_idx, const char *const value, const oldval_g = *(char **)get_varp_scope(opt, OPT_GLOBAL); } - *varp = s; + *varp = xstrdup(value); char *const saved_oldval = xstrdup(oldval); char *const saved_oldval_l = (oldval_l != NULL) ? xstrdup(oldval_l) : 0; char *const saved_oldval_g = (oldval_g != NULL) ? xstrdup(oldval_g) : 0; - char *const saved_newval = xstrdup(s); + char *const saved_newval = xstrdup(*varp); int value_checked = false; - const char *const errmsg = did_set_string_option(curbuf, curwin, opt_idx, varp, oldval, s, errbuf, + const char *const errmsg = did_set_string_option(curbuf, curwin, opt_idx, varp, oldval, errbuf, errbuflen, opt_flags, &value_checked); if (errmsg == NULL) { did_set_option(opt_idx, opt_flags, true, value_checked); @@ -2067,8 +2066,7 @@ static void do_spelllang_source(win_T *win) /// /// @return NULL for success, or an untranslated error message for an error const char *did_set_string_option(buf_T *buf, win_T *win, int opt_idx, char **varp, char *oldval, - const char *value, char *errbuf, size_t errbuflen, int opt_flags, - int *value_checked) + char *errbuf, size_t errbuflen, int opt_flags, int *value_checked) { const char *errmsg = NULL; int restore_chartab = false; @@ -2082,7 +2080,7 @@ const char *did_set_string_option(buf_T *buf, win_T *win, int opt_idx, char **va .os_idx = opt_idx, .os_flags = opt_flags, .os_oldval.string = oldval, - .os_newval.string = value, + .os_newval.string = *varp, .os_value_checked = false, .os_value_changed = false, .os_restore_chartab = false, -- cgit From 804c828e681ec3e55f19614d78c2b098b2672f86 Mon Sep 17 00:00:00 2001 From: Lewis Russell Date: Wed, 12 Jul 2023 14:21:27 +0100 Subject: fix(optionstr.c): incorrect use of curbuf/curwin --- src/nvim/optionstr.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/nvim/optionstr.c b/src/nvim/optionstr.c index 18065f05d4..9e5226a46f 100644 --- a/src/nvim/optionstr.c +++ b/src/nvim/optionstr.c @@ -2086,8 +2086,8 @@ const char *did_set_string_option(buf_T *buf, win_T *win, int opt_idx, char **va .os_restore_chartab = false, .os_errbuf = errbuf, .os_errbuflen = errbuflen, - .os_win = curwin, - .os_buf = curbuf, + .os_win = win, + .os_buf = buf, }; // Disallow changing some options from secure mode -- cgit From 354a1154423fc381dfcd7b045963e8076288e777 Mon Sep 17 00:00:00 2001 From: Lewis Russell Date: Wed, 12 Jul 2023 14:54:13 +0100 Subject: refactor(option.c): call did_set_option for all types set_option_value() only called did_set_option() for string options, whereas do_set_option_value() called it for all types. This change makes set_option_value() call did_set_option() for all types and thus makes it more consistent with do_set_option_value(). --- src/nvim/option.c | 10 ++++++++-- src/nvim/optionstr.c | 9 ++------- 2 files changed, 10 insertions(+), 9 deletions(-) (limited to 'src') diff --git a/src/nvim/option.c b/src/nvim/option.c index fb7c04d36b..e433dc5639 100644 --- a/src/nvim/option.c +++ b/src/nvim/option.c @@ -1598,7 +1598,7 @@ int do_set(char *arg, int opt_flags) /// @param opt_flags possibly with OPT_MODELINE /// @param new_value value was replaced completely /// @param value_checked value was checked to be safe, no need to set P_INSECURE -void did_set_option(int opt_idx, int opt_flags, int new_value, int value_checked) +void did_set_option(int opt_idx, int opt_flags, bool new_value, bool value_checked) { options[opt_idx].flags |= P_WAS_SET; @@ -3793,6 +3793,8 @@ const char *set_option_value(const char *const name, const OptVal value, int opt goto end; } + int value_checked = false; + switch (v.type) { case kOptValTypeNil: abort(); // This will never happen. @@ -3825,11 +3827,15 @@ const char *set_option_value(const char *const name, const OptVal value, int opt if (s == NULL || opt_flags & OPT_CLEAR) { s = ""; } - errmsg = set_string_option(opt_idx, s, opt_flags, errbuf, sizeof(errbuf)); + errmsg = set_string_option(opt_idx, s, opt_flags, &value_checked, errbuf, sizeof(errbuf)); break; } } + if (errmsg != NULL) { + did_set_option(opt_idx, opt_flags, true, value_checked); + } + end: optval_free(v); // Free the copied OptVal. return errmsg; diff --git a/src/nvim/optionstr.c b/src/nvim/optionstr.c index 9e5226a46f..d5ab47cc84 100644 --- a/src/nvim/optionstr.c +++ b/src/nvim/optionstr.c @@ -428,7 +428,7 @@ void set_string_option_direct_in_buf(buf_T *buf, const char *name, int opt_idx, /// /// @return NULL on success, an untranslated error message on error. const char *set_string_option(const int opt_idx, const char *const value, const int opt_flags, - char *const errbuf, const size_t errbuflen) + int *value_checked, char *const errbuf, const size_t errbuflen) FUNC_ATTR_NONNULL_ARG(2) FUNC_ATTR_WARN_UNUSED_RESULT { vimoption_T *opt = get_option(opt_idx); @@ -457,13 +457,8 @@ const char *set_string_option(const int opt_idx, const char *const value, const char *const saved_oldval_g = (oldval_g != NULL) ? xstrdup(oldval_g) : 0; char *const saved_newval = xstrdup(*varp); - int value_checked = false; const char *const errmsg = did_set_string_option(curbuf, curwin, opt_idx, varp, oldval, errbuf, - errbuflen, opt_flags, &value_checked); - if (errmsg == NULL) { - did_set_option(opt_idx, opt_flags, true, value_checked); - } - + errbuflen, opt_flags, value_checked); // call autocommand after handling side effects if (errmsg == NULL) { if (!starting) { -- cgit From 95c880ce310a6ab3e5b68d4b1d81d81da6786f00 Mon Sep 17 00:00:00 2001 From: Lewis Russell Date: Thu, 13 Jul 2023 10:22:18 +0100 Subject: refactor(option): change some int to bool --- src/nvim/option.c | 6 +++--- src/nvim/option_defs.h | 4 ++-- src/nvim/optionstr.c | 5 +++-- 3 files changed, 8 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/nvim/option.c b/src/nvim/option.c index e433dc5639..f108b4d8d9 100644 --- a/src/nvim/option.c +++ b/src/nvim/option.c @@ -1098,7 +1098,7 @@ static char *stropt_get_newval(int nextchar, int opt_idx, char **argp, void *var /// Part of do_set() for string options. static void do_set_option_string(int opt_idx, int opt_flags, char **argp, int nextchar, set_op_T op_arg, uint32_t flags, void *varp_arg, char *errbuf, - size_t errbuflen, int *value_checked, const char **errmsg) + size_t errbuflen, bool *value_checked, const char **errmsg) { char *arg = *argp; set_op_T op = op_arg; @@ -1337,7 +1337,7 @@ static void do_set_option_value(int opt_idx, int opt_flags, char **argp, int pre set_op_T op, uint32_t flags, void *varp, char *errbuf, size_t errbuflen, const char **errmsg) { - int value_checked = false; + bool value_checked = false; if (flags & P_BOOL) { // boolean do_set_bool(opt_idx, opt_flags, prefix, nextchar, varp, errmsg); } else if (flags & P_NUM) { // numeric @@ -3793,7 +3793,7 @@ const char *set_option_value(const char *const name, const OptVal value, int opt goto end; } - int value_checked = false; + bool value_checked = false; switch (v.type) { case kOptValTypeNil: diff --git a/src/nvim/option_defs.h b/src/nvim/option_defs.h index 35687a19b7..313d282cef 100644 --- a/src/nvim/option_defs.h +++ b/src/nvim/option_defs.h @@ -1009,9 +1009,9 @@ typedef struct { // Option value was checked to be safe, no need to set P_INSECURE // Used for the 'keymap', 'filetype' and 'syntax' options. - int os_value_checked; + bool os_value_checked; // Option value changed. Used for the 'filetype' and 'syntax' options. - int os_value_changed; + bool os_value_changed; // Used by the 'isident', 'iskeyword', 'isprint' and 'isfname' options. // Set to true if the character table is modified when processing the diff --git a/src/nvim/optionstr.c b/src/nvim/optionstr.c index d5ab47cc84..f82919d77a 100644 --- a/src/nvim/optionstr.c +++ b/src/nvim/optionstr.c @@ -428,7 +428,7 @@ void set_string_option_direct_in_buf(buf_T *buf, const char *name, int opt_idx, /// /// @return NULL on success, an untranslated error message on error. const char *set_string_option(const int opt_idx, const char *const value, const int opt_flags, - int *value_checked, char *const errbuf, const size_t errbuflen) + bool *value_checked, char *const errbuf, const size_t errbuflen) FUNC_ATTR_NONNULL_ARG(2) FUNC_ATTR_WARN_UNUSED_RESULT { vimoption_T *opt = get_option(opt_idx); @@ -2061,7 +2061,8 @@ static void do_spelllang_source(win_T *win) /// /// @return NULL for success, or an untranslated error message for an error const char *did_set_string_option(buf_T *buf, win_T *win, int opt_idx, char **varp, char *oldval, - char *errbuf, size_t errbuflen, int opt_flags, int *value_checked) + char *errbuf, size_t errbuflen, int opt_flags, + bool *value_checked) { const char *errmsg = NULL; int restore_chartab = false; -- cgit From 9a6b399cadef1cf5cd65851f76c9cf600b7e0aa1 Mon Sep 17 00:00:00 2001 From: Lewis Russell Date: Thu, 13 Jul 2023 11:48:00 +0100 Subject: refactor(option): remove redundant local --- src/nvim/option.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'src') diff --git a/src/nvim/option.c b/src/nvim/option.c index f108b4d8d9..164abe2c80 100644 --- a/src/nvim/option.c +++ b/src/nvim/option.c @@ -1100,7 +1100,6 @@ static void do_set_option_string(int opt_idx, int opt_flags, char **argp, int ne set_op_T op_arg, uint32_t flags, void *varp_arg, char *errbuf, size_t errbuflen, bool *value_checked, const char **errmsg) { - char *arg = *argp; set_op_T op = op_arg; void *varp = varp_arg; char *origval_l = NULL; @@ -1138,7 +1137,7 @@ static void do_set_option_string(int opt_idx, int opt_flags, char **argp, int ne } // Get the new value for the option - char *newval = stropt_get_newval(nextchar, opt_idx, &arg, varp, origval, &op, flags); + char *newval = stropt_get_newval(nextchar, opt_idx, argp, varp, origval, &op, flags); // Set the new value. *(char **)(varp) = newval; @@ -1193,8 +1192,6 @@ static void do_set_option_string(int opt_idx, int opt_flags, char **argp, int ne xfree(saved_origval_l); xfree(saved_origval_g); xfree(saved_newval); - - *argp = arg; } static set_op_T get_op(const char *arg) -- cgit From 9edd0f077eacde69512dcfbe8b58312036d02be4 Mon Sep 17 00:00:00 2001 From: Lewis Russell Date: Thu, 13 Jul 2023 12:00:06 +0100 Subject: refactor(option): remove hidden option check --- src/nvim/optionstr.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/nvim/optionstr.c b/src/nvim/optionstr.c index f82919d77a..26a5ed8563 100644 --- a/src/nvim/optionstr.c +++ b/src/nvim/optionstr.c @@ -419,7 +419,9 @@ void set_string_option_direct_in_buf(buf_T *buf, const char *name, int opt_idx, curbuf = save_curbuf; unblock_autocmds(); } + /// Set a string option to a new value, handling the effects +/// Must not be called with a hidden option! /// /// @param[in] opt_idx Option to set. /// @param[in] value New value. @@ -433,10 +435,6 @@ const char *set_string_option(const int opt_idx, const char *const value, const { vimoption_T *opt = get_option(opt_idx); - if (opt->var == NULL) { // don't set hidden option - return NULL; - } - char **const varp = (char **)get_varp_scope(opt, ((opt_flags & (OPT_LOCAL | OPT_GLOBAL)) == 0 ? ((opt->indir & PV_BOTH) ? OPT_GLOBAL : OPT_LOCAL) -- cgit From ec0ca51886271b54e8776c17a9b06f3271b8c4f9 Mon Sep 17 00:00:00 2001 From: Lewis Russell Date: Thu, 13 Jul 2023 12:08:43 +0100 Subject: refactor(option): further align set_string_option with do_set_option_string --- src/nvim/option.c | 69 +++++++++++++++++++++++++--------------------------- src/nvim/optionstr.c | 23 +++++++++++------- 2 files changed, 47 insertions(+), 45 deletions(-) (limited to 'src') diff --git a/src/nvim/option.c b/src/nvim/option.c index 164abe2c80..e52d5ac484 100644 --- a/src/nvim/option.c +++ b/src/nvim/option.c @@ -1100,6 +1100,8 @@ static void do_set_option_string(int opt_idx, int opt_flags, char **argp, int ne set_op_T op_arg, uint32_t flags, void *varp_arg, char *errbuf, size_t errbuflen, bool *value_checked, const char **errmsg) { + vimoption_T *opt = get_option(opt_idx); + set_op_T op = op_arg; void *varp = varp_arg; char *origval_l = NULL; @@ -1109,20 +1111,20 @@ static void do_set_option_string(int opt_idx, int opt_flags, char **argp, int ne // with a local value the local value will be // reset, use the global value here. if ((opt_flags & (OPT_LOCAL | OPT_GLOBAL)) == 0 - && ((int)options[opt_idx].indir & PV_BOTH)) { - varp = options[opt_idx].var; + && ((int)opt->indir & PV_BOTH)) { + varp = opt->var; } // The old value is kept until we are sure that the new value is valid. char *oldval = *(char **)varp; if ((opt_flags & (OPT_LOCAL | OPT_GLOBAL)) == 0) { - origval_l = *(char **)get_varp_scope(&(options[opt_idx]), OPT_LOCAL); - origval_g = *(char **)get_varp_scope(&(options[opt_idx]), OPT_GLOBAL); + origval_l = *(char **)get_varp_scope(opt, OPT_LOCAL); + origval_g = *(char **)get_varp_scope(opt, OPT_GLOBAL); // A global-local string option might have an empty option as value to // indicate that the global value should be used. - if (((int)options[opt_idx].indir & PV_BOTH) && origval_l == empty_option) { + if (((int)opt->indir & PV_BOTH) && origval_l == empty_option) { origval_l = origval_g; } } @@ -1130,8 +1132,8 @@ static void do_set_option_string(int opt_idx, int opt_flags, char **argp, int ne char *origval; // When setting the local value of a global option, the old value may be // the global value. - if (((int)options[opt_idx].indir & PV_BOTH) && (opt_flags & OPT_LOCAL)) { - origval = *(char **)get_varp(&options[opt_idx]); + if (((int)opt->indir & PV_BOTH) && (opt_flags & OPT_LOCAL)) { + origval = *(char **)get_varp(opt); } else { origval = oldval; } @@ -1140,43 +1142,38 @@ static void do_set_option_string(int opt_idx, int opt_flags, char **argp, int ne char *newval = stropt_get_newval(nextchar, opt_idx, argp, varp, origval, &op, flags); // Set the new value. - *(char **)(varp) = newval; - if (newval == NULL) { - *(char **)(varp) = empty_option; - } + *(char **)(varp) = newval != NULL ? newval : empty_option; // origval may be freed by did_set_string_option(), make a copy. - char *saved_origval = (origval != NULL) ? xstrdup(origval) : NULL; - char *saved_origval_l = (origval_l != NULL) ? xstrdup(origval_l) : NULL; - char *saved_origval_g = (origval_g != NULL) ? xstrdup(origval_g) : NULL; + char *const saved_origval = (origval != NULL) ? xstrdup(origval) : NULL; + char *const saved_origval_l = (origval_l != NULL) ? xstrdup(origval_l) : NULL; + char *const saved_origval_g = (origval_g != NULL) ? xstrdup(origval_g) : NULL; // newval (and varp) may become invalid if the buffer is closed by // autocommands. - char *saved_newval = (newval != NULL) ? xstrdup(newval) : NULL; + char *const saved_newval = (newval != NULL) ? xstrdup(newval) : NULL; - { - uint32_t *p = insecure_flag(curwin, opt_idx, opt_flags); - const int secure_saved = secure; + uint32_t *p = insecure_flag(curwin, opt_idx, opt_flags); + const int secure_saved = secure; - // When an option is set in the sandbox, from a modeline or in secure - // mode, then deal with side effects in secure mode. Also when the - // value was set with the P_INSECURE flag and is not completely - // replaced. - if ((opt_flags & OPT_MODELINE) - || sandbox != 0 - || (op != OP_NONE && (*p & P_INSECURE))) { - secure = 1; - } + // When an option is set in the sandbox, from a modeline or in secure + // mode, then deal with side effects in secure mode. Also when the + // value was set with the P_INSECURE flag and is not completely + // replaced. + if ((opt_flags & OPT_MODELINE) + || sandbox != 0 + || (op != OP_NONE && (*p & P_INSECURE))) { + secure = 1; + } - // Handle side effects, and set the global value for ":set" on local - // options. Note: when setting 'syntax' or 'filetype' autocommands may - // be triggered that can cause havoc. - *errmsg = did_set_string_option(curbuf, curwin, opt_idx, (char **)varp, oldval, - errbuf, errbuflen, - opt_flags, value_checked); + // Handle side effects, and set the global value for ":set" on local + // options. Note: when setting 'syntax' or 'filetype' autocommands may + // be triggered that can cause havoc. + *errmsg = did_set_string_option(curbuf, curwin, opt_idx, (char **)varp, oldval, + errbuf, errbuflen, + opt_flags, value_checked); - secure = secure_saved; - } + secure = secure_saved; if (*errmsg == NULL) { if (!starting) { @@ -1184,7 +1181,7 @@ static void do_set_option_string(int opt_idx, int opt_flags, char **argp, int ne saved_origval_g, saved_newval); } if (options[opt_idx].flags & P_UI_OPTION) { - ui_call_option_set(cstr_as_string(options[opt_idx].fullname), + ui_call_option_set(cstr_as_string(opt->fullname), CSTR_AS_OBJ(saved_newval)); } } diff --git a/src/nvim/optionstr.c b/src/nvim/optionstr.c index 26a5ed8563..40118a36d4 100644 --- a/src/nvim/optionstr.c +++ b/src/nvim/optionstr.c @@ -439,20 +439,25 @@ const char *set_string_option(const int opt_idx, const char *const value, const = (char **)get_varp_scope(opt, ((opt_flags & (OPT_LOCAL | OPT_GLOBAL)) == 0 ? ((opt->indir & PV_BOTH) ? OPT_GLOBAL : OPT_LOCAL) : opt_flags)); + char *origval_l = NULL; + char *origval_g = NULL; + + // The old value is kept until we are sure that the new value is valid. char *const oldval = *varp; - char *oldval_l = NULL; - char *oldval_g = NULL; if ((opt_flags & (OPT_LOCAL | OPT_GLOBAL)) == 0) { - oldval_l = *(char **)get_varp_scope(opt, OPT_LOCAL); - oldval_g = *(char **)get_varp_scope(opt, OPT_GLOBAL); + origval_l = *(char **)get_varp_scope(opt, OPT_LOCAL); + origval_g = *(char **)get_varp_scope(opt, OPT_GLOBAL); } *varp = xstrdup(value); char *const saved_oldval = xstrdup(oldval); - char *const saved_oldval_l = (oldval_l != NULL) ? xstrdup(oldval_l) : 0; - char *const saved_oldval_g = (oldval_g != NULL) ? xstrdup(oldval_g) : 0; + char *const saved_oldval_l = (origval_l != NULL) ? xstrdup(origval_l) : 0; + char *const saved_oldval_g = (origval_g != NULL) ? xstrdup(origval_g) : 0; + + // newval (and varp) may become invalid if the buffer is closed by + // autocommands. char *const saved_newval = xstrdup(*varp); const char *const errmsg = did_set_string_option(curbuf, curwin, opt_idx, varp, oldval, errbuf, @@ -460,12 +465,12 @@ const char *set_string_option(const int opt_idx, const char *const value, const // call autocommand after handling side effects if (errmsg == NULL) { if (!starting) { - trigger_optionset_string(opt_idx, opt_flags, saved_oldval, saved_oldval_l, saved_oldval_g, - saved_newval); + trigger_optionset_string(opt_idx, opt_flags, saved_oldval, saved_oldval_l, + saved_oldval_g, saved_newval); } if (opt->flags & P_UI_OPTION) { ui_call_option_set(cstr_as_string(opt->fullname), - STRING_OBJ(cstr_as_string(saved_newval))); + CSTR_AS_OBJ(saved_newval)); } } xfree(saved_oldval); -- cgit From 3a45a0db4d5a3981f4fbd575d405a9c6e382b930 Mon Sep 17 00:00:00 2001 From: Lewis Russell Date: Thu, 13 Jul 2023 12:21:54 +0100 Subject: refactor(option): further align set_string_option with do_set_option_string (2) --- src/nvim/option.c | 3 ++- src/nvim/optionstr.c | 50 +++++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 45 insertions(+), 8 deletions(-) (limited to 'src') diff --git a/src/nvim/option.c b/src/nvim/option.c index e52d5ac484..c68355f5b1 100644 --- a/src/nvim/option.c +++ b/src/nvim/option.c @@ -1175,6 +1175,7 @@ static void do_set_option_string(int opt_idx, int opt_flags, char **argp, int ne secure = secure_saved; + // call autocommand after handling side effects if (*errmsg == NULL) { if (!starting) { trigger_optionset_string(opt_idx, opt_flags, saved_origval, saved_origval_l, @@ -4485,7 +4486,7 @@ void *get_option_varp_scope_from(int opt_idx, int scope, buf_T *buf, win_T *win) return get_varp_scope_from(&(options[opt_idx]), scope, buf, win); } -static void *get_varp_from(vimoption_T *p, buf_T *buf, win_T *win) +void *get_varp_from(vimoption_T *p, buf_T *buf, win_T *win) { // hidden option, always return NULL if (p->var == NULL) { diff --git a/src/nvim/optionstr.c b/src/nvim/optionstr.c index 40118a36d4..c0a7bfa3f7 100644 --- a/src/nvim/optionstr.c +++ b/src/nvim/optionstr.c @@ -435,24 +435,46 @@ const char *set_string_option(const int opt_idx, const char *const value, const { vimoption_T *opt = get_option(opt_idx); - char **const varp - = (char **)get_varp_scope(opt, ((opt_flags & (OPT_LOCAL | OPT_GLOBAL)) == 0 - ? ((opt->indir & PV_BOTH) ? OPT_GLOBAL : OPT_LOCAL) - : opt_flags)); + char **varp = (char **)get_varp_scope(opt, ((opt_flags & (OPT_LOCAL | OPT_GLOBAL)) == 0 + ? ((opt->indir & PV_BOTH) ? OPT_GLOBAL : OPT_LOCAL) + : opt_flags)); char *origval_l = NULL; char *origval_g = NULL; + // When using ":set opt=val" for a global option + // with a local value the local value will be + // reset, use the global value here. + if ((opt_flags & (OPT_LOCAL | OPT_GLOBAL)) == 0 + && ((int)opt->indir & PV_BOTH)) { + varp = (char **)opt->var; + } + // The old value is kept until we are sure that the new value is valid. char *const oldval = *varp; if ((opt_flags & (OPT_LOCAL | OPT_GLOBAL)) == 0) { origval_l = *(char **)get_varp_scope(opt, OPT_LOCAL); origval_g = *(char **)get_varp_scope(opt, OPT_GLOBAL); + + // A global-local string option might have an empty option as value to + // indicate that the global value should be used. + if (((int)opt->indir & PV_BOTH) && origval_l == empty_option) { + origval_l = origval_g; + } + } + + char *origval; + // When setting the local value of a global option, the old value may be + // the global value. + if (((int)opt->indir & PV_BOTH) && (opt_flags & OPT_LOCAL)) { + origval = *(char **)get_varp_from(opt, curbuf, curwin); + } else { + origval = oldval; } *varp = xstrdup(value); - char *const saved_oldval = xstrdup(oldval); + char *const saved_origval = (origval != NULL) ? xstrdup(origval) : NULL; char *const saved_oldval_l = (origval_l != NULL) ? xstrdup(origval_l) : 0; char *const saved_oldval_g = (origval_g != NULL) ? xstrdup(origval_g) : 0; @@ -460,12 +482,26 @@ const char *set_string_option(const int opt_idx, const char *const value, const // autocommands. char *const saved_newval = xstrdup(*varp); + const int secure_saved = secure; + + // When an option is set in the sandbox, from a modeline or in secure + // mode, then deal with side effects in secure mode. Also when the + // value was set with the P_INSECURE flag and is not completely + // replaced. + if ((opt_flags & OPT_MODELINE) + || sandbox != 0) { + secure = 1; + } + const char *const errmsg = did_set_string_option(curbuf, curwin, opt_idx, varp, oldval, errbuf, errbuflen, opt_flags, value_checked); + + secure = secure_saved; + // call autocommand after handling side effects if (errmsg == NULL) { if (!starting) { - trigger_optionset_string(opt_idx, opt_flags, saved_oldval, saved_oldval_l, + trigger_optionset_string(opt_idx, opt_flags, saved_origval, saved_oldval_l, saved_oldval_g, saved_newval); } if (opt->flags & P_UI_OPTION) { @@ -473,7 +509,7 @@ const char *set_string_option(const int opt_idx, const char *const value, const CSTR_AS_OBJ(saved_newval)); } } - xfree(saved_oldval); + xfree(saved_origval); xfree(saved_oldval_l); xfree(saved_oldval_g); xfree(saved_newval); -- cgit From 6a449a892bdc25f4984b1cd4dcbe4e7157142a46 Mon Sep 17 00:00:00 2001 From: Lewis Russell Date: Thu, 13 Jul 2023 15:53:07 +0100 Subject: refactor(option): remove OPT_CLEAR --- src/nvim/api/options.c | 3 --- src/nvim/option.c | 15 +++++++-------- src/nvim/option_defs.h | 1 - 3 files changed, 7 insertions(+), 12 deletions(-) (limited to 'src') diff --git a/src/nvim/api/options.c b/src/nvim/api/options.c index e33cb72e8d..eb80683365 100644 --- a/src/nvim/api/options.c +++ b/src/nvim/api/options.c @@ -465,9 +465,6 @@ OptVal get_option_value_for(const char *const name, uint32_t *flagsp, int scope, /// @param[in] name Option name. /// @param[in] value Option value. /// @param[in] opt_flags Flags: OPT_LOCAL, OPT_GLOBAL, or 0 (both). -/// If OPT_CLEAR is set, the value of the option -/// is cleared (the exact semantics of this depend -/// on the option). /// @param[in] opt_type Option type. See SREQ_* in option_defs.h. /// @param[in] from Target buffer/window. /// @param[out] err Error message, if any. diff --git a/src/nvim/option.c b/src/nvim/option.c index c68355f5b1..6c869c26f0 100644 --- a/src/nvim/option.c +++ b/src/nvim/option.c @@ -3722,9 +3722,6 @@ vimoption_T *get_option(int opt_idx) /// @param[in] name Option name. /// @param[in] value Option value. If NIL_OPTVAL, the option value is cleared. /// @param[in] opt_flags Flags: OPT_LOCAL, OPT_GLOBAL, or 0 (both). -/// If OPT_CLEAR is set, the value of the option -/// is cleared (the exact semantics of this depend -/// on the option). /// /// @return NULL on success, an untranslated error message on error. const char *set_option_value(const char *const name, const OptVal value, int opt_flags) @@ -3765,9 +3762,11 @@ const char *set_option_value(const char *const name, const OptVal value, int opt // Copy the value so we can modify the copy. OptVal v = optval_copy(value); - if (v.type == kOptValTypeNil) { - opt_flags |= OPT_CLEAR; + // Clear an option. For global-local options clear the local value + // (the exact semantics of this depend on the option). + bool clear = v.type == kOptValTypeNil; + if (v.type == kOptValTypeNil) { // Change the type of the OptVal to the type used by the option so that it can be cleared. // TODO(famiu): Clean up all of this after set_(num|bool|string)_option() is unified. if (flags & P_BOOL) { @@ -3794,7 +3793,7 @@ const char *set_option_value(const char *const name, const OptVal value, int opt case kOptValTypeNil: abort(); // This will never happen. case kOptValTypeBoolean: { - if (opt_flags & OPT_CLEAR) { + if (clear) { if ((int *)varp == &curbuf->b_p_ar) { v.data.boolean = kNone; } else { @@ -3805,7 +3804,7 @@ const char *set_option_value(const char *const name, const OptVal value, int opt break; } case kOptValTypeNumber: { - if (opt_flags & OPT_CLEAR) { + if (clear) { if ((long *)varp == &curbuf->b_p_ul) { v.data.number = NO_LOCAL_UNDOLEVEL; } else if ((long *)varp == &curwin->w_p_so || (long *)varp == &curwin->w_p_siso) { @@ -3819,7 +3818,7 @@ const char *set_option_value(const char *const name, const OptVal value, int opt } case kOptValTypeString: { const char *s = v.data.string.data; - if (s == NULL || opt_flags & OPT_CLEAR) { + if (s == NULL || clear) { s = ""; } errmsg = set_string_option(opt_idx, s, opt_flags, &value_checked, errbuf, sizeof(errbuf)); diff --git a/src/nvim/option_defs.h b/src/nvim/option_defs.h index 313d282cef..1007925ccb 100644 --- a/src/nvim/option_defs.h +++ b/src/nvim/option_defs.h @@ -70,7 +70,6 @@ typedef enum { OPT_ONECOLUMN = 0x40, ///< list options one per line OPT_NO_REDRAW = 0x80, ///< ignore redraw flags on option OPT_SKIPRTP = 0x100, ///< "skiprtp" in 'sessionoptions' - OPT_CLEAR = 0x200, ///< Clear local value of an option. } OptionFlags; // Return value from get_option_value_strict -- cgit From af3c667ac13e23b5ff838e720c8b26fa4a12644a Mon Sep 17 00:00:00 2001 From: Lewis Russell Date: Thu, 13 Jul 2023 16:31:11 +0100 Subject: refactor(option): option clearing --- src/nvim/option.c | 76 +++++++++++++++++++++++++++++----------------------- src/nvim/optionstr.c | 8 ++++-- 2 files changed, 48 insertions(+), 36 deletions(-) (limited to 'src') diff --git a/src/nvim/option.c b/src/nvim/option.c index 6c869c26f0..077aef052b 100644 --- a/src/nvim/option.c +++ b/src/nvim/option.c @@ -3717,6 +3717,45 @@ vimoption_T *get_option(int opt_idx) return &options[opt_idx]; } +/// Clear an option +/// +/// The exact semantics of this depend on the option. +static OptVal clear_optval(const char *name, uint32_t flags, void *varp, buf_T *buf, win_T *win) +{ + OptVal v = NIL_OPTVAL; + + // Change the type of the OptVal to the type used by the option so that it can be cleared. + // TODO(famiu): Clean up all of this after set_(num|bool|string)_option() is unified. + + if (flags & P_BOOL) { + v.type = kOptValTypeBoolean; + if ((int *)varp == &buf->b_p_ar) { + // TODO(lewis6991): replace this with a more general condition that + // indicates we are setting the local value of a global-local option + v.data.boolean = kNone; + } else { + v = get_option_value(name, NULL, OPT_GLOBAL, NULL); + } + } else if (flags & P_NUM) { + v.type = kOptValTypeNumber; + if ((long *)varp == &curbuf->b_p_ul) { + // The one true special case + v.data.number = NO_LOCAL_UNDOLEVEL; + } else if ((long *)varp == &win->w_p_so || (long *)varp == &win->w_p_siso) { + // TODO(lewis6991): replace this with a more general condition that + // indicates we are setting the local value of a global-local option + v.data.number = -1; + } else { + v = get_option_value(name, NULL, OPT_GLOBAL, NULL); + } + } else if (flags & P_STRING) { + v.type = kOptValTypeString; + v.data.string.data = NULL; + } + + return v; +} + /// Set the value of an option /// /// @param[in] name Option name. @@ -3762,20 +3801,8 @@ const char *set_option_value(const char *const name, const OptVal value, int opt // Copy the value so we can modify the copy. OptVal v = optval_copy(value); - // Clear an option. For global-local options clear the local value - // (the exact semantics of this depend on the option). - bool clear = v.type == kOptValTypeNil; - if (v.type == kOptValTypeNil) { - // Change the type of the OptVal to the type used by the option so that it can be cleared. - // TODO(famiu): Clean up all of this after set_(num|bool|string)_option() is unified. - if (flags & P_BOOL) { - v.type = kOptValTypeBoolean; - } else if (flags & P_NUM) { - v.type = kOptValTypeNumber; - } else if (flags & P_STRING) { - v.type = kOptValTypeString; - } + v = clear_optval(name, flags, varp, curbuf, curwin); } else if (!optval_match_type(v, opt_idx)) { char *rep = optval_to_cstr(v); char *valid_types = option_get_valid_types(opt_idx); @@ -3793,35 +3820,16 @@ const char *set_option_value(const char *const name, const OptVal value, int opt case kOptValTypeNil: abort(); // This will never happen. case kOptValTypeBoolean: { - if (clear) { - if ((int *)varp == &curbuf->b_p_ar) { - v.data.boolean = kNone; - } else { - v = get_option_value(name, NULL, OPT_GLOBAL, NULL); - } - } errmsg = set_bool_option(opt_idx, varp, (int)v.data.boolean, opt_flags); break; } case kOptValTypeNumber: { - if (clear) { - if ((long *)varp == &curbuf->b_p_ul) { - v.data.number = NO_LOCAL_UNDOLEVEL; - } else if ((long *)varp == &curwin->w_p_so || (long *)varp == &curwin->w_p_siso) { - v.data.number = -1; - } else { - v = get_option_value(name, NULL, OPT_GLOBAL, NULL); - } - } errmsg = set_num_option(opt_idx, varp, (long)v.data.number, errbuf, sizeof(errbuf), opt_flags); break; } case kOptValTypeString: { - const char *s = v.data.string.data; - if (s == NULL || clear) { - s = ""; - } - errmsg = set_string_option(opt_idx, s, opt_flags, &value_checked, errbuf, sizeof(errbuf)); + errmsg = set_string_option(opt_idx, v.data.string.data, opt_flags, &value_checked, errbuf, + sizeof(errbuf)); break; } } diff --git a/src/nvim/optionstr.c b/src/nvim/optionstr.c index c0a7bfa3f7..c68ee65fcf 100644 --- a/src/nvim/optionstr.c +++ b/src/nvim/optionstr.c @@ -429,12 +429,16 @@ void set_string_option_direct_in_buf(buf_T *buf, const char *name, int opt_idx, /// #OPT_GLOBAL. /// /// @return NULL on success, an untranslated error message on error. -const char *set_string_option(const int opt_idx, const char *const value, const int opt_flags, +const char *set_string_option(const int opt_idx, const char *value, const int opt_flags, bool *value_checked, char *const errbuf, const size_t errbuflen) - FUNC_ATTR_NONNULL_ARG(2) FUNC_ATTR_WARN_UNUSED_RESULT + FUNC_ATTR_WARN_UNUSED_RESULT { vimoption_T *opt = get_option(opt_idx); + if (value == NULL) { + value = ""; + } + char **varp = (char **)get_varp_scope(opt, ((opt_flags & (OPT_LOCAL | OPT_GLOBAL)) == 0 ? ((opt->indir & PV_BOTH) ? OPT_GLOBAL : OPT_LOCAL) : opt_flags)); -- cgit From 038ac39b8e3be4a41e763442c306680633806170 Mon Sep 17 00:00:00 2001 From: Lewis Russell Date: Thu, 13 Jul 2023 16:46:05 +0100 Subject: refactor(option): pass varp to set_string_option --- src/nvim/option.c | 2 +- src/nvim/optionstr.c | 21 ++++++++------------- 2 files changed, 9 insertions(+), 14 deletions(-) (limited to 'src') diff --git a/src/nvim/option.c b/src/nvim/option.c index 077aef052b..5a582d2d64 100644 --- a/src/nvim/option.c +++ b/src/nvim/option.c @@ -3828,7 +3828,7 @@ const char *set_option_value(const char *const name, const OptVal value, int opt break; } case kOptValTypeString: { - errmsg = set_string_option(opt_idx, v.data.string.data, opt_flags, &value_checked, errbuf, + errmsg = set_string_option(opt_idx, varp, v.data.string.data, opt_flags, &value_checked, errbuf, sizeof(errbuf)); break; } diff --git a/src/nvim/optionstr.c b/src/nvim/optionstr.c index c68ee65fcf..17afd10ee3 100644 --- a/src/nvim/optionstr.c +++ b/src/nvim/optionstr.c @@ -429,19 +429,14 @@ void set_string_option_direct_in_buf(buf_T *buf, const char *name, int opt_idx, /// #OPT_GLOBAL. /// /// @return NULL on success, an untranslated error message on error. -const char *set_string_option(const int opt_idx, const char *value, const int opt_flags, - bool *value_checked, char *const errbuf, const size_t errbuflen) +const char *set_string_option(const int opt_idx, void *varp_arg, const char *value, + const int opt_flags, bool *value_checked, char *const errbuf, + const size_t errbuflen) FUNC_ATTR_WARN_UNUSED_RESULT { vimoption_T *opt = get_option(opt_idx); - if (value == NULL) { - value = ""; - } - - char **varp = (char **)get_varp_scope(opt, ((opt_flags & (OPT_LOCAL | OPT_GLOBAL)) == 0 - ? ((opt->indir & PV_BOTH) ? OPT_GLOBAL : OPT_LOCAL) - : opt_flags)); + void *varp = (char **)varp_arg; char *origval_l = NULL; char *origval_g = NULL; @@ -450,11 +445,11 @@ const char *set_string_option(const int opt_idx, const char *value, const int op // reset, use the global value here. if ((opt_flags & (OPT_LOCAL | OPT_GLOBAL)) == 0 && ((int)opt->indir & PV_BOTH)) { - varp = (char **)opt->var; + varp = opt->var; } // The old value is kept until we are sure that the new value is valid. - char *const oldval = *varp; + char *oldval = *(char **)varp; if ((opt_flags & (OPT_LOCAL | OPT_GLOBAL)) == 0) { origval_l = *(char **)get_varp_scope(opt, OPT_LOCAL); @@ -476,7 +471,7 @@ const char *set_string_option(const int opt_idx, const char *value, const int op origval = oldval; } - *varp = xstrdup(value); + *(char **)varp = xstrdup(value != NULL ? value : empty_option); char *const saved_origval = (origval != NULL) ? xstrdup(origval) : NULL; char *const saved_oldval_l = (origval_l != NULL) ? xstrdup(origval_l) : 0; @@ -484,7 +479,7 @@ const char *set_string_option(const int opt_idx, const char *value, const int op // newval (and varp) may become invalid if the buffer is closed by // autocommands. - char *const saved_newval = xstrdup(*varp); + char *const saved_newval = xstrdup(*(char **)varp); const int secure_saved = secure; -- cgit From 90fd0864c4794e711cb6c28c41ec05b4d0187954 Mon Sep 17 00:00:00 2001 From: Lewis Russell Date: Thu, 13 Jul 2023 17:08:52 +0100 Subject: refactor(option): add set_option() --- src/nvim/option.c | 47 ++++++++++++++++++++++++----------------------- 1 file changed, 24 insertions(+), 23 deletions(-) (limited to 'src') diff --git a/src/nvim/option.c b/src/nvim/option.c index 5a582d2d64..cc3a9c181d 100644 --- a/src/nvim/option.c +++ b/src/nvim/option.c @@ -3756,6 +3756,29 @@ static OptVal clear_optval(const char *name, uint32_t flags, void *varp, buf_T * return v; } +static const char *set_option(int opt_idx, void *varp, OptVal *v, int opt_flags, char *errbuf, + size_t errbuflen) +{ + const char *errmsg = NULL; + + bool value_checked = false; + + if (v->type == kOptValTypeBoolean) { + errmsg = set_bool_option(opt_idx, varp, (int)v->data.boolean, opt_flags); + } else if (v->type == kOptValTypeNumber) { + errmsg = set_num_option(opt_idx, varp, (long)v->data.number, errbuf, errbuflen, opt_flags); + } else if (v->type == kOptValTypeString) { + errmsg = set_string_option(opt_idx, varp, v->data.string.data, opt_flags, &value_checked, + errbuf, errbuflen); + } + + if (errmsg != NULL) { + did_set_option(opt_idx, opt_flags, true, value_checked); + } + + return errmsg; +} + /// Set the value of an option /// /// @param[in] name Option name. @@ -3814,29 +3837,7 @@ const char *set_option_value(const char *const name, const OptVal value, int opt goto end; } - bool value_checked = false; - - switch (v.type) { - case kOptValTypeNil: - abort(); // This will never happen. - case kOptValTypeBoolean: { - errmsg = set_bool_option(opt_idx, varp, (int)v.data.boolean, opt_flags); - break; - } - case kOptValTypeNumber: { - errmsg = set_num_option(opt_idx, varp, (long)v.data.number, errbuf, sizeof(errbuf), opt_flags); - break; - } - case kOptValTypeString: { - errmsg = set_string_option(opt_idx, varp, v.data.string.data, opt_flags, &value_checked, errbuf, - sizeof(errbuf)); - break; - } - } - - if (errmsg != NULL) { - did_set_option(opt_idx, opt_flags, true, value_checked); - } + errmsg = set_option(opt_idx, varp, &v, opt_flags, errbuf, sizeof(errbuf)); end: optval_free(v); // Free the copied OptVal. -- cgit