Mercurial > vim
changeset 32055:8a3f659c7b5e v9.0.1359
patch 9.0.1359: too many "else if" statements in handling options
Commit: https://github.com/vim/vim/commit/5da901bb68717b2baff6e971c1517219b6ee3a67
Author: Yegappan Lakshmanan <yegappan@yahoo.com>
Date: Mon Feb 27 12:47:47 2023 +0000
patch 9.0.1359: too many "else if" statements in handling options
Problem: Too many "else if" statements in handling options.
Solution: Add more functions for handling option changes. (Yegappan
Lakshmanan, closes #12060)
| author | Bram Moolenaar <Bram@vim.org> |
|---|---|
| date | Mon, 27 Feb 2023 14:00:03 +0100 |
| parents | 2cfcf99476ff |
| children | 3dae9c4395a8 |
| files | src/map.c src/optiondefs.h src/optionstr.c src/proto/optionstr.pro src/structs.h src/testdir/gen_opt_test.vim src/testdir/test_options.vim src/version.c |
| diffstat | 8 files changed, 188 insertions(+), 61 deletions(-) [+] |
line wrap: on
line diff
--- a/src/map.c +++ b/src/map.c @@ -3088,11 +3088,10 @@ did_set_langmap(optset_T *args UNUSED) } if (to == NUL) { - // TODO: Need to use errbuf argument for this error message - // and return it. - semsg(_(e_langmap_matching_character_missing_for_str), - transchar(from)); - return NULL; + sprintf(args->os_errbuf, + _(e_langmap_matching_character_missing_for_str), + transchar(from)); + return args->os_errbuf; } if (from >= 256) @@ -3112,10 +3111,10 @@ did_set_langmap(optset_T *args UNUSED) { if (p[0] != ',') { - // TODO: Need to use errbuf argument for this error - // message and return it. - semsg(_(e_langmap_extra_characters_after_semicolon_str), p); - return NULL; + sprintf(args->os_errbuf, + _(e_langmap_extra_characters_after_semicolon_str), + p); + return args->os_errbuf; } ++p; }
--- a/src/optiondefs.h +++ b/src/optiondefs.h @@ -939,7 +939,8 @@ static struct vimoption options[] = #endif (char_u *)0L} SCTX_INIT}, {"filetype", "ft", P_STRING|P_EXPAND|P_ALLOCED|P_VI_DEF|P_NOGLOB|P_NFNAME, - (char_u *)&p_ft, PV_FT, NULL, + (char_u *)&p_ft, PV_FT, + did_set_filetype_or_syntax, {(char_u *)"", (char_u *)0L} SCTX_INIT}, {"fillchars", "fcs", P_STRING|P_VI_DEF|P_RALL|P_ONECOMMA|P_NODUP, @@ -1411,7 +1412,7 @@ static struct vimoption options[] = (char_u *)&p_im, PV_NONE, did_set_insertmode, {(char_u *)FALSE, (char_u *)0L} SCTX_INIT}, {"isfname", "isf", P_STRING|P_VI_DEF|P_COMMA|P_NODUP, - (char_u *)&p_isf, PV_NONE, NULL, + (char_u *)&p_isf, PV_NONE, did_set_isopt, { #ifdef BACKSLASH_IN_FILENAME // Excluded are: & and ^ are special in cmd.exe @@ -1428,7 +1429,7 @@ static struct vimoption options[] = #endif (char_u *)0L} SCTX_INIT}, {"isident", "isi", P_STRING|P_VI_DEF|P_COMMA|P_NODUP, - (char_u *)&p_isi, PV_NONE, NULL, + (char_u *)&p_isi, PV_NONE, did_set_isopt, { #if defined(MSWIN) (char_u *)"@,48-57,_,128-167,224-235", @@ -1437,7 +1438,7 @@ static struct vimoption options[] = #endif (char_u *)0L} SCTX_INIT}, {"iskeyword", "isk", P_STRING|P_ALLOCED|P_VIM|P_COMMA|P_NODUP, - (char_u *)&p_isk, PV_ISK, NULL, + (char_u *)&p_isk, PV_ISK, did_set_isopt, { (char_u *)"@,48-57,_", #if defined(MSWIN) @@ -1447,7 +1448,7 @@ static struct vimoption options[] = #endif } SCTX_INIT}, {"isprint", "isp", P_STRING|P_VI_DEF|P_RALL|P_COMMA|P_NODUP, - (char_u *)&p_isp, PV_NONE, NULL, + (char_u *)&p_isp, PV_NONE, did_set_isopt, { #if defined(MSWIN) || defined(VMS) (char_u *)"@,~-255", @@ -1469,7 +1470,7 @@ static struct vimoption options[] = SCTX_INIT}, {"keymap", "kmp", P_STRING|P_ALLOCED|P_VI_DEF|P_RBUF|P_RSTAT|P_NFNAME|P_PRI_MKRC, #ifdef FEAT_KEYMAP - (char_u *)&p_keymap, PV_KMAP, NULL, + (char_u *)&p_keymap, PV_KMAP, did_set_keymap, {(char_u *)"", (char_u *)0L} #else (char_u *)NULL, PV_NONE, NULL, @@ -2095,7 +2096,7 @@ static struct vimoption options[] = {(char_u *)FALSE, (char_u *)0L} SCTX_INIT}, {"rightleftcmd", "rlc", P_STRING|P_ALLOCED|P_VI_DEF|P_RWIN, #ifdef FEAT_RIGHTLEFT - (char_u *)VAR_WIN, PV_RLC, NULL, + (char_u *)VAR_WIN, PV_RLC, did_set_rightleftcmd, {(char_u *)"search", (char_u *)NULL} #else (char_u *)NULL, PV_NONE, NULL, @@ -2419,7 +2420,7 @@ static struct vimoption options[] = (char_u *)&p_swf, PV_SWF, did_set_swapfile, {(char_u *)TRUE, (char_u *)0L} SCTX_INIT}, {"swapsync", "sws", P_STRING|P_VI_DEF, - (char_u *)&p_sws, PV_NONE, NULL, + (char_u *)&p_sws, PV_NONE, did_set_swapsync, {(char_u *)"fsync", (char_u *)0L} SCTX_INIT}, {"switchbuf", "swb", P_STRING|P_VI_DEF|P_ONECOMMA|P_NODUP, (char_u *)&p_swb, PV_NONE, did_set_switchbuf, @@ -2435,7 +2436,8 @@ static struct vimoption options[] = SCTX_INIT}, {"syntax", "syn", P_STRING|P_ALLOCED|P_VI_DEF|P_NOGLOB|P_NFNAME, #ifdef FEAT_SYN_HL - (char_u *)&p_syn, PV_SYN, NULL, + (char_u *)&p_syn, PV_SYN, + did_set_filetype_or_syntax, {(char_u *)"", (char_u *)0L} #else (char_u *)NULL, PV_NONE, NULL,
--- a/src/optionstr.c +++ b/src/optionstr.c @@ -97,6 +97,7 @@ static char *(p_scl_values[]) = {"yes", static char *(p_twt_values[]) = {"winpty", "conpty", "", NULL}; #endif static char *(p_sloc_values[]) = {"last", "statusline", "tabline", NULL}; +static char *(p_sws_values[]) = {"fsync", "sync", NULL}; static int check_opt_strings(char_u *val, char **values, int list); static int opt_strings_flags(char_u *val, char **values, unsigned *flagp, int list); @@ -761,15 +762,15 @@ did_set_breakindentopt(optset_T *args UN * The 'isident' or the 'iskeyword' or the 'isprint' or the 'isfname' option is * changed. */ - static char * -did_set_isopt(int *did_chartab) + char * +did_set_isopt(optset_T *args) { // 'isident', 'iskeyword', 'isprint or 'isfname' option: refill g_chartab[] // If the new option is invalid, use old value. // 'lisp' option: refill g_chartab[] for '-' char. if (init_chartab() == FAIL) { - *did_chartab = TRUE; // need to restore it below + args->os_restore_chartab = TRUE;// need to restore the chartab. return e_invalid_argument; // error in value } @@ -929,6 +930,15 @@ did_set_splitkeep(optset_T *args UNUSED) } /* + * The 'swapsync' option is changed. + */ + char * +did_set_swapsync(optset_T *args UNUSED) +{ + return did_set_opt_strings(p_sws, p_sws_values, FALSE); +} + +/* * The 'switchbuf' option is changed. */ char * @@ -1224,16 +1234,16 @@ did_set_imactivatekey(optset_T *args UNU } #endif -#ifdef FEAT_KEYMAP +#if defined(FEAT_KEYMAP) || defined(PROTO) /* * The 'keymap' option is changed. */ - static char * -did_set_keymap(char_u **varp, int opt_flags, int *value_checked) + char * +did_set_keymap(optset_T *args) { char *errmsg = NULL; - if (!valid_filetype(*varp)) + if (!valid_filetype(args->os_varp)) errmsg = e_invalid_argument; else { @@ -1250,7 +1260,7 @@ did_set_keymap(char_u **varp, int opt_fl // Since we check the value, there is no need to set P_INSECURE, // even when the value comes from a modeline. - *value_checked = TRUE; + args->os_value_checked = TRUE; } if (errmsg == NULL) @@ -1270,7 +1280,7 @@ did_set_keymap(char_u **varp, int opt_fl if (curbuf->b_p_imsearch == B_IMODE_LMAP) curbuf->b_p_imsearch = B_IMODE_USE_INSERT; } - if ((opt_flags & OPT_LOCAL) == 0) + if ((args->os_flags & OPT_LOCAL) == 0) { set_iminsert_global(); set_imsearch_global(); @@ -2573,7 +2583,8 @@ did_set_cinoptions(optset_T *args UNUSED char * did_set_lispoptions(optset_T *args) { - if (*args->os_varp != NUL && STRCMP(args->os_varp, "expr:0") != 0 + if (*args->os_varp != NUL + && STRCMP(args->os_varp, "expr:0") != 0 && STRCMP(args->os_varp, "expr:1") != 0) return e_invalid_argument; @@ -2594,24 +2605,36 @@ did_set_renderoptions(optset_T *args UNU } #endif +#if defined(FEAT_RIGHTLEFT) || defined(PROTO) +/* + * The 'rightleftcmd' option is changed. + */ + char * +did_set_rightleftcmd(optset_T *args) +{ + // Currently only "search" is a supported value. + if (*args->os_varp != NUL && STRCMP(args->os_varp, "search") != 0) + return e_invalid_argument; + + return NULL; +} +#endif + /* * The 'filetype' or the 'syntax' option is changed. */ - static char * -did_set_filetype_or_syntax( - char_u **varp, - char_u *oldval, - int *value_checked, - int *value_changed) + char * +did_set_filetype_or_syntax(optset_T *args) { - if (!valid_filetype(*varp)) + if (!valid_filetype(args->os_varp)) return e_invalid_argument; - *value_changed = STRCMP(oldval, *varp) != 0; + args->os_value_changed = + STRCMP(args->os_oldval.string, args->os_varp) != 0; // Since we check the value, there is no need to set P_INSECURE, // even when the value comes from a modeline. - *value_checked = TRUE; + args->os_value_checked = TRUE; return NULL; } @@ -3008,7 +3031,7 @@ did_set_string_option( // need to set P_INSECURE { char *errmsg = NULL; - int did_chartab = FALSE; + int restore_chartab = FALSE; char_u **gvarp; long_u free_oldval = (get_option_flags(opt_idx) & P_ALLOCED); int value_changed = FALSE; @@ -3037,31 +3060,37 @@ did_set_string_option( args.os_flags = opt_flags; args.os_oldval.string = oldval; args.os_newval.string = value; + args.os_value_checked = FALSE; + args.os_value_changed = FALSE; + args.os_restore_chartab = FALSE; args.os_errbuf = errbuf; + // Invoke the option specific callback function to validate and apply + // the new option value. errmsg = did_set_cb(&args); + #ifdef FEAT_EVAL - // When processing the '*expr' options (e.g. diffexpr, foldexpr, etc.), - // the did_set_cb() function may modify '*varp'. + // The '*expr' option (e.g. diffexpr, foldexpr, etc.), callback + // functions may modify '*varp'. if (errmsg == NULL && is_expr_option(varp, gvarp)) *varp = args.os_varp; #endif + // The 'filetype' and 'syntax' option callback functions may change + // the os_value_changed field. + value_changed = args.os_value_changed; + // The 'keymap', 'filetype' and 'syntax' option callback functions + // may change the os_value_checked field. + *value_checked = args.os_value_checked; + // The 'isident', 'iskeyword', 'isprint' and 'isfname' options may + // change the character table. On failure, this needs to be restored. + restore_chartab = args.os_restore_chartab; } else if (varp == &T_NAME) // 'term' errmsg = did_set_term(&opt_idx, &free_oldval); - else if ( varp == &p_isi // 'isident' - || varp == &(curbuf->b_p_isk) // 'iskeyword' - || varp == &p_isp // 'isprint' - || varp == &p_isf) // 'isfname' - errmsg = did_set_isopt(&did_chartab); else if ( varp == &p_enc // 'encoding' || gvarp == &p_fenc // 'fileencoding' || varp == &p_tenc // 'termencoding' || gvarp == &p_menc) // 'makeencoding' errmsg = did_set_encoding(varp, gvarp, opt_flags); -#ifdef FEAT_KEYMAP - else if (varp == &curbuf->b_p_keymap) // 'keymap' - errmsg = did_set_keymap(varp, opt_flags, value_checked); -#endif else if ( varp == &p_lcs // global 'listchars' || varp == &p_fcs) // global 'fillchars' errmsg = did_set_global_listfillchars(varp, opt_flags); @@ -3072,14 +3101,6 @@ did_set_string_option( // terminal options else if (istermoption_idx(opt_idx) && full_screen) did_set_term_option(varp, &did_swaptcap); - else if (gvarp == &p_ft) // 'filetype' - errmsg = did_set_filetype_or_syntax(varp, oldval, value_checked, - &value_changed); -#ifdef FEAT_SYN_HL - else if (gvarp == &p_syn) // 'syntax' - errmsg = did_set_filetype_or_syntax(varp, oldval, value_checked, - &value_changed); -#endif // If an error is detected, restore the previous value. if (errmsg != NULL) @@ -3087,7 +3108,7 @@ did_set_string_option( free_string_option(*varp); *varp = oldval; // When resetting some values, need to act on it. - if (did_chartab) + if (restore_chartab) (void)init_chartab(); if (varp == &p_hl) (void)highlight_changed();
--- a/src/proto/optionstr.pro +++ b/src/proto/optionstr.pro @@ -12,6 +12,7 @@ char *set_string_option(int opt_idx, cha char *did_set_backupcopy(optset_T *args); char *did_set_backupext_or_patchmode(optset_T *args); char *did_set_breakindentopt(optset_T *args); +char *did_set_isopt(optset_T *args); char *did_set_helpfile(optset_T *args); char *did_set_colorcolumn(optset_T *args); char *did_set_cursorlineopt(optset_T *args); @@ -23,6 +24,7 @@ char *did_set_scrollopt(optset_T *args); char *did_set_selectmode(optset_T *args); char *did_set_showcmdloc(optset_T *args); char *did_set_splitkeep(optset_T *args); +char *did_set_swapsync(optset_T *args); char *did_set_switchbuf(optset_T *args); char *did_set_sessionoptions(optset_T *args); char *did_set_viewoptions(optset_T *args); @@ -36,6 +38,7 @@ char *did_set_eadirection(optset_T *args char *did_set_eventignore(optset_T *args); char *did_set_printencoding(optset_T *args); char *did_set_imactivatekey(optset_T *args); +char *did_set_keymap(optset_T *args); char *did_set_fileformat(optset_T *args); char *did_set_fileformats(optset_T *args); char *did_set_cryptkey(optset_T *args); @@ -96,6 +99,8 @@ char *did_set_cscopequickfix(optset_T *a char *did_set_cinoptions(optset_T *args); char *did_set_lispoptions(optset_T *args); char *did_set_renderoptions(optset_T *args); +char *did_set_rightleftcmd(optset_T *args); +char *did_set_filetype_or_syntax(optset_T *args); char *did_set_termwinkey(optset_T *args); char *did_set_termwinsize(optset_T *args); char *did_set_termwintype(optset_T *args);
--- a/src/structs.h +++ b/src/structs.h @@ -4820,6 +4820,17 @@ typedef struct // Currently only used for boolean options. int os_doskip; + // 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; + // Option value changed. Used for the 'filetype' and 'syntax' options. + int os_value_changed; + + // Used by the 'isident', 'iskeyword', 'isprint' and 'isfname' options. + // Set to TRUE if the character table is modified when processing the + // option and need to be restored because of a failure. + int os_restore_chartab; + // If the value specified for an option is not valid and the error message // is parameterized, then the "os_errbuf" buffer is used to store the error // message (when it is not NULL).
--- a/src/testdir/gen_opt_test.vim +++ b/src/testdir/gen_opt_test.vim @@ -130,6 +130,7 @@ let test_values = { \ 'printoptions': [['', 'header:0', 'left:10pc,top:5pc'], ['xxx']], \ 'scrollopt': [['', 'ver', 'ver,hor'], ['xxx']], \ 'renderoptions': [[''], ['xxx']], + \ 'rightleftcmd': [['search'], ['xxx']], \ 'selection': [['old', 'inclusive'], ['', 'xxx']], \ 'selectmode': [['', 'mouse', 'key,cmd'], ['xxx']], \ 'sessionoptions': [['', 'blank', 'help,options,slash'], ['xxx']], @@ -140,6 +141,7 @@ let test_values = { \ 'spelloptions': [['', 'camel'], ['xxx']], \ 'spellsuggest': [['', 'best', 'double,33'], ['xxx']], \ 'splitkeep': [['cursor', 'screen', 'topline'], ['xxx']], + \ 'swapsync': [['', 'sync', 'fsync'], ['xxx']], \ 'switchbuf': [['', 'useopen', 'split,newtab'], ['xxx']], \ 'tagcase': [['smart', 'match'], ['', 'xxx', 'smart,match']], \ 'term': [[], []],
--- a/src/testdir/test_options.vim +++ b/src/testdir/test_options.vim @@ -46,6 +46,10 @@ func Test_isfname() set isfname= call assert_equal("~X", expand("~X")) set isfname& + " Test for setting 'isfname' to an unsupported character + let save_isfname = &isfname + call assert_fails('exe $"set isfname+={"\u1234"}"', 'E474:') + call assert_equal(save_isfname, &isfname) endfunc " Test for getting the value of 'pastetoggle' @@ -1548,6 +1552,7 @@ func Test_string_option_revert_on_failur \ ['fileencoding', 'utf-8', 'a123,'], \ ['fileformat', 'mac', 'a123'], \ ['fileformats', 'mac', 'a123'], + \ ['filetype', 'abc', 'a^b'], \ ['fillchars', 'diff:~', 'a123'], \ ['foldclose', 'all', 'a123'], \ ['foldmarker', '[[[,]]]', '[[['], @@ -1556,7 +1561,49 @@ func Test_string_option_revert_on_failur \ ['formatoptions', 'an', '*'], \ ['guicursor', 'n-v-c:block-Cursor/lCursor', 'n-v-c'], \ ['helplang', 'en', 'a'], - \ ['highlight', '!:CursorColumn', '8:'] + \ ['highlight', '!:CursorColumn', '8:'], + \ ['keymodel', 'stopsel', 'a123'], + \ ['keyprotocol', 'kitty:kitty', 'kitty:'], + \ ['lispoptions', 'expr:1', 'a123'], + \ ['listchars', 'tab:->', 'tab:'], + \ ['matchpairs', '<:>', '<:'], + \ ['mkspellmem', '100000,1000,100', '100000'], + \ ['mouse', 'nvi', 'z'], + \ ['mousemodel', 'extend', 'a123'], + \ ['nrformats', 'alpha', 'a123'], + \ ['omnifunc', 'MyOmniFunc', '1a-'], + \ ['operatorfunc', 'MyOpFunc', '1a-'], + \ ['previewpopup', 'width:20', 'a123'], + \ ['printoptions', 'paper:A4', 'a123:'], + \ ['quickfixtextfunc', 'MyQfFunc', '1a-'], + \ ['rulerformat', '%l', '%['], + \ ['scrollopt', 'hor,jump', 'a123'], + \ ['selection', 'exclusive', 'a123'], + \ ['selectmode', 'cmd', 'a123'], + \ ['sessionoptions', 'options', 'a123'], + \ ['shortmess', 'w', '2'], + \ ['showbreak', '>>', "\x01"], + \ ['showcmdloc', 'statusline', 'a123'], + \ ['signcolumn', 'no', 'a123'], + \ ['spellcapcheck', '[.?!]\+', '%\{'], + \ ['spellfile', 'MySpell.en.add', "\x01"], + \ ['spelllang', 'en', "#"], + \ ['spelloptions', 'camel', 'a123'], + \ ['spellsuggest', 'double', 'a123'], + \ ['splitkeep', 'topline', 'a123'], + \ ['statusline', '%f', '%['], + \ ['swapsync', 'sync', 'a123'], + \ ['switchbuf', 'usetab', 'a123'], + \ ['syntax', 'abc', 'a^b'], + \ ['tabline', '%f', '%['], + \ ['tagcase', 'ignore', 'a123'], + \ ['tagfunc', 'MyTagFunc', '1a-'], + \ ['thesaurusfunc', 'MyThesaurusFunc', '1a-'], + \ ['viewoptions', 'options', 'a123'], + \ ['virtualedit', 'onemore', 'a123'], + \ ['whichwrap', '<,>', '{,}'], + \ ['wildmode', 'list', 'a123'], + \ ['wildoptions', 'pum', 'a123'] \ ] if has('gui') call add(optlist, ['browsedir', 'buffer', 'a123']) @@ -1571,11 +1618,49 @@ func Test_string_option_revert_on_failur call add(optlist, ['cscopequickfix', 't-', 'z-']) endif if !has('win32') - call add(optlist, ['imactivatefunc', 'MyCmplFunc', '1a-']) + call add(optlist, ['imactivatefunc', 'MyActFunc', '1a-']) + call add(optlist, ['imstatusfunc', 'MyStatusFunc', '1a-']) + endif + if has('keymap') + call add(optlist, ['keymap', 'greek', '[]']) + endif + if has('mouseshape') + call add(optlist, ['mouseshape', 'm:no', 'a123:']) + endif + if has('win32') && has('gui') + call add(optlist, ['renderoptions', 'type:directx', 'type:directx,a123']) + endif + if has('rightleft') + call add(optlist, ['rightleftcmd', 'search', 'a123']) + endif + if has('terminal') + call add(optlist, ['termwinkey', '<C-L>', '<C']) + call add(optlist, ['termwinsize', '24x80', '100']) + endif + if has('win32') && has('terminal') + call add(optlist, ['termwintype', 'winpty', 'a123']) + endif + if has('+toolbar') + call add(optlist, ['toolbar', 'text', 'a123']) + call add(optlist, ['toolbariconsize', 'medium', 'a123']) + endif + if has('+mouse') + call add(optlist, ['ttymouse', 'xterm', 'a123']) + endif + if has('+vartabs') + call add(optlist, ['varsofttabstop', '12', 'a123']) + call add(optlist, ['vartabstop', '4,20', '4,']) + endif + if has('gui') + call add(optlist, ['winaltkeys', 'no', 'a123']) endif for opt in optlist exe $"let save_opt = &{opt[0]}" - exe $"let &{opt[0]} = '{opt[1]}'" + try + exe $"let &{opt[0]} = '{opt[1]}'" + catch + call assert_report($"Caught {v:exception} with {opt->string()}") + endtry call assert_fails($"let &{opt[0]} = '{opt[2]}'", '', opt[0]) call assert_equal(opt[1], eval($"&{opt[0]}"), opt[0]) exe $"let &{opt[0]} = save_opt"