vim-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 vim/vim#12060)

5da901bb68
This commit is contained in:
Lewis Russell 2023-04-26 15:32:48 +01:00 committed by zeertzjq
parent bb7371ad82
commit 5cda9c267a
5 changed files with 175 additions and 53 deletions

View File

@ -2386,7 +2386,7 @@ void langmap_init(void)
/// Called when langmap option is set; the language map can be
/// changed at any time!
const char *did_set_langmap(optset_T *args FUNC_ATTR_UNUSED)
const char *did_set_langmap(optset_T *args)
{
char *p;
char *p2;
@ -2434,11 +2434,10 @@ const char *did_set_langmap(optset_T *args FUNC_ATTR_UNUSED)
}
}
if (to == NUL) {
// TODO(vim): Need to use errbuf argument for this error message
// and return it.
semsg(_("E357: 'langmap': Matching character missing for %s"),
transchar(from));
return NULL;
snprintf(args->os_errbuf, args->os_errbuflen,
_("E357: 'langmap': Matching character missing for %s"),
transchar(from));
return args->os_errbuf;
}
if (from >= 256) {
@ -2456,10 +2455,10 @@ const char *did_set_langmap(optset_T *args FUNC_ATTR_UNUSED)
p = p2;
if (p[0] != NUL) {
if (p[0] != ',') {
// TODO(vim): Need to use errbuf argument for this error
// message and return it.
semsg(_("E358: 'langmap': Extra characters after semicolon: %s"), p);
return NULL;
snprintf(args->os_errbuf, args->os_errbuflen,
_("E358: 'langmap': Extra characters after semicolon: %s"),
p);
return args->os_errbuf;
}
p++;
}

View File

@ -1002,6 +1002,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).

View File

@ -821,7 +821,8 @@ return {
alloced=true,
expand=true,
varname='p_ft',
defaults={if_true=""}
defaults={if_true=""},
cb='did_set_filetype_or_syntax'
},
{
full_name='fillchars', abbreviation='fcs',
@ -1287,7 +1288,8 @@ return {
-- ( and ) are used in text separating fnames */
if_true="@,48-57,/,\\,.,-,_,+,,,#,$,%,{,},[,],:,@-@,!,~,=",
if_false="@,48-57,/,.,-,_,+,,,#,$,%,~,="
}
},
cb='did_set_isopt'
},
{
full_name='isident', abbreviation='isi',
@ -1299,7 +1301,8 @@ return {
condition='MSWIN',
if_true="@,48-57,_,128-167,224-235",
if_false="@,48-57,_,192-255"
}
},
cb='did_set_isopt'
},
{
full_name='iskeyword', abbreviation='isk',
@ -1308,7 +1311,8 @@ return {
deny_duplicates=true,
alloced=true,
varname='p_isk',
defaults={if_true="@,48-57,_,192-255"}
defaults={if_true="@,48-57,_,192-255"},
cb='did_set_isopt'
},
{
full_name='isprint', abbreviation='isp',
@ -1317,7 +1321,8 @@ return {
deny_duplicates=true,
redraw={'all_windows'},
varname='p_isp',
defaults={if_true="@,161-255"}
defaults={if_true="@,161-255"},
cb='did_set_isopt'
},
{
full_name='joinspaces', abbreviation='js',
@ -1344,7 +1349,8 @@ return {
alloced=true,
redraw={'statuslines', 'current_buffer'},
varname='p_keymap', pv_name='p_kmap',
defaults={if_true=""}
defaults={if_true=""},
cb='did_set_keymap'
},
{
full_name='keymodel', abbreviation='km',
@ -1959,7 +1965,8 @@ return {
type='string', scope={'window'},
alloced=true,
redraw={'current_window'},
defaults={if_true="search"}
defaults={if_true="search"},
cb='did_set_rightleftcmd'
},
{
full_name='ruler', abbreviation='ru',
@ -2486,7 +2493,8 @@ return {
normal_fname_chars=true,
alloced=true,
varname='p_syn',
defaults={if_true=""}
defaults={if_true=""},
cb='did_set_filetype_or_syntax'
},
{
full_name='tagfunc', abbreviation='tfu',

View File

@ -727,15 +727,17 @@ const char *did_set_breakindentopt(optset_T *args)
/// The 'isident' or the 'iskeyword' or the 'isprint' or the 'isfname' option is
/// changed.
static void did_set_isopt(buf_T *buf, bool *did_chartab, const char **errmsg)
const char *did_set_isopt(optset_T *args)
{
buf_T *buf = (buf_T *)args->os_buf;
// '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 (buf_init_chartab(buf, true) == FAIL) {
*did_chartab = true; // need to restore it below
*errmsg = e_invarg; // error in value
args->os_restore_chartab = true; // need to restore it below
return e_invarg; // error in value
}
return NULL;
}
/// The 'helpfile' option is changed.
@ -978,12 +980,14 @@ static void did_set_encoding(buf_T *buf, char **varp, char **gvarp, int opt_flag
}
}
static void did_set_keymap(buf_T *buf, char **varp, int opt_flags, int *value_checked,
const char **errmsg)
/// The 'keymap' option has changed.
const char *did_set_keymap(optset_T *args)
{
if (!valid_filetype(*varp)) {
*errmsg = e_invarg;
return;
buf_T *buf = (buf_T *)args->os_buf;
int opt_flags = args->os_flags;
if (!valid_filetype(args->os_varp)) {
return e_invarg;
}
int secure_save = secure;
@ -993,13 +997,13 @@ static void did_set_keymap(buf_T *buf, char **varp, int opt_flags, int *value_ch
secure = 0;
// load or unload key mapping tables
*errmsg = keymap_init();
const char *errmsg = keymap_init();
secure = secure_save;
// 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) {
if (*buf->b_p_keymap != NUL) {
@ -1023,6 +1027,8 @@ static void did_set_keymap(buf_T *buf, char **varp, int opt_flags, int *value_ch
}
status_redraw_buf(buf);
}
return errmsg;
}
/// The 'fileformat' option is changed.
@ -1739,20 +1745,31 @@ const char *did_set_lispoptions(optset_T *args)
return NULL;
}
/// The 'filetype' or the 'syntax' option is changed.
static void did_set_filetype_or_syntax(char **varp, char *oldval, int *value_checked,
bool *value_changed, const char **errmsg)
/// The 'rightleftcmd' option is changed.
const char *did_set_rightleftcmd(optset_T *args)
{
if (!valid_filetype(*varp)) {
*errmsg = e_invarg;
return;
// Currently only "search" is a supported value.
if (*args->os_varp != NUL && strcmp(args->os_varp, "search") != 0) {
return e_invarg;
}
*value_changed = strcmp(oldval, *varp) != 0;
return NULL;
}
/// The 'filetype' or the 'syntax' option is changed.
const char *did_set_filetype_or_syntax(optset_T *args)
{
if (!valid_filetype(args->os_varp)) {
return e_invarg;
}
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;
}
const char *did_set_winhl(optset_T *args)
@ -1955,7 +1972,7 @@ static const char *did_set_string_option_for(buf_T *buf, win_T *win, int opt_idx
size_t errbuflen, int opt_flags, int *value_checked)
{
const char *errmsg = NULL;
bool did_chartab = false;
int restore_chartab = false;
vimoption_T *opt = get_option(opt_idx);
bool free_oldval = (opt->flags & P_ALLOCED);
opt_did_set_cb_T did_set_cb = get_option_did_set_cb(opt_idx);
@ -1970,6 +1987,9 @@ static const char *did_set_string_option_for(buf_T *buf, win_T *win, int opt_idx
.os_flags = opt_flags,
.os_oldval.string = oldval,
.os_newval.string = value,
.os_value_checked = false,
.os_value_changed = false,
.os_restore_chartab = false,
.os_errbuf = errbuf,
.os_errbuflen = errbuflen,
.os_win = curwin,
@ -1991,17 +2011,21 @@ static const char *did_set_string_option_for(buf_T *buf, win_T *win, int opt_idx
if (errmsg == NULL && is_expr_option(curwin, varp, gvarp)) {
*varp = args.os_varp;
}
} else if (varp == &p_isi // 'isident'
|| varp == &buf->b_p_isk // 'iskeyword'
|| varp == &p_isp // 'isprint'
|| varp == &p_isf) { // 'isfname'
did_set_isopt(buf, &did_chartab, &errmsg);
// 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 == &p_shada) { // 'shada'
errmsg = did_set_shada(&opt, &opt_idx, &free_oldval, errbuf, errbuflen);
} else if (varp == &p_enc // 'encoding'
|| gvarp == &p_fenc // 'fileencoding'
|| gvarp == &p_menc) { // 'makeencoding'
did_set_encoding(buf, varp, gvarp, opt_flags, &errmsg);
} else if (varp == &buf->b_p_keymap) { // 'keymap'
did_set_keymap(buf, varp, opt_flags, value_checked, &errmsg);
} else if (varp == &p_lcs // global 'listchars'
|| varp == &p_fcs) { // global 'fillchars'
did_set_global_listfillchars(win, varp, opt_flags, &errmsg);
@ -2009,11 +2033,6 @@ static const char *did_set_string_option_for(buf_T *buf, win_T *win, int opt_idx
errmsg = set_chars_option(win, varp, true);
} else if (varp == &win->w_p_fcs) { // local 'fillchars'
errmsg = set_chars_option(win, varp, true);
} else if (varp == &p_shada) { // 'shada'
errmsg = did_set_shada(&opt, &opt_idx, &free_oldval, errbuf, errbuflen);
} else if (gvarp == &p_ft // 'filetype'
|| gvarp == &p_syn) { // 'syntax'
did_set_filetype_or_syntax(varp, oldval, value_checked, &value_changed, &errmsg);
}
// If an error is detected, restore the previous value.
@ -2021,7 +2040,7 @@ static const char *did_set_string_option_for(buf_T *buf, win_T *win, int opt_idx
free_string_option(*varp);
*varp = oldval;
// When resetting some values, need to act on it.
if (did_chartab) {
if (restore_chartab) {
(void)buf_init_chartab(buf, true);
}
} else {

View File

@ -45,6 +45,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'
@ -1388,6 +1392,7 @@ func Test_string_option_revert_on_failure()
\ ['fileencoding', 'utf-8', 'a123,'],
\ ['fileformat', 'mac', 'a123'],
\ ['fileformats', 'mac', 'a123'],
\ ['filetype', 'abc', 'a^b'],
\ ['fillchars', 'diff:~', 'a123'],
\ ['foldclose', 'all', 'a123'],
\ ['foldmarker', '[[[,]]]', '[[['],
@ -1396,7 +1401,49 @@ func Test_string_option_revert_on_failure()
\ ['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'])
@ -1411,11 +1458,49 @@ func Test_string_option_revert_on_failure()
call add(optlist, ['cscopequickfix', 't-', 'z-'])
endif
if !has('win32') && !has('nvim')
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"