From: Illia Bobyr Date: Wed, 22 Jul 2026 09:48:19 +0000 (+0000) Subject: patch 9.2.0829: Sessions do not preserve script version for expression options X-Git-Tag: v9.2.0829^0 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=4b767b99951a9f9c193dc4d45ad89803f2e82add;p=thirdparty%2Fvim.git patch 9.2.0829: Sessions do not preserve script version for expression options Problem: Sessions do not preserve script version for option holding expressions Solution: Remember script version for string options, and add a ":legacy" prefix for ":set"/":setlocal" calls when necessary Since patch v9.2.0579 (":mksession, :mkview and :mkvimrc emit legacy Vim script") session files are marked as Vim9 script. A few options have Vim expressions as their values. When inserted their values literally into a Vim9 script, these expressions are now evaluated as Vim9 expressions. But the original value might have been set by a legacy script. In order to be backward compatible and not break most of the existing file type plugin scripts, we need to consider the script version of the option value. If an option was set in a legacy script, or via "legacy set"/"legacy setlocal", we need to restore it using the "legacy" prefix as well. Options are not marked as expression options, vs string options, vs bool options. We only know if an option is a bool, number or a string option. It seems safe to set bool and number options using Vim9 semantics. But for string options we do not know if an option value is a Vim expression. And so, if it was set by a legacy script we just use a conservative approach and prefix the "set" or "setlocal" command with "legacy" for all string options set in a legacy context. related: #20152 closes: #20696 Signed-off-by: Illia Bobyr Signed-off-by: Christian Brabandt --- diff --git a/src/option.c b/src/option.c index 9a869fdb9b..d4f63203dd 100644 --- a/src/option.c +++ b/src/option.c @@ -56,7 +56,7 @@ static int find_key_option(char_u *arg_arg, int has_lt); static void showoptions(int all, int opt_flags); static int optval_default(struct vimoption *, char_u *varp, int compatible); static void showoneopt(struct vimoption *, int opt_flags); -static int put_setstring(FILE *fd, char *cmd, char *name, char_u **valuep, long_u flags); +static int put_setstring(FILE *fd, bool legacy, char *cmd, char *name, char_u **valuep, long_u flags); static int put_setnum(FILE *fd, char *cmd, char *name, long *valuep); static int put_setbool(FILE *fd, char *cmd, char *name, int value); static int istermoption(struct vimoption *p); @@ -3525,6 +3525,36 @@ set_option_sctx_idx(int opt_idx, int opt_flags, sctx_T script_ctx) } } +/* + * Returns true if an option value will be evaluated as a Vim9 script. + * Checks stored script context for the option. For options that have values in + * multiple contexts, opt_flags selects the desired context with OPT_GLOBAL, or + * OPT_LOCAL, selecting global or buffer/window context, respectively. + */ + bool +is_option_value_vim9(int opt_idx, int opt_flags) +{ + int indir = (int)options[opt_idx].indir; + sctx_T *sctx = NULL; + + if ((opt_flags & OPT_GLOBAL) || (indir & (PV_BUF|PV_WIN)) == 0) + sctx = &options[opt_idx].script_ctx; + + if ((opt_flags & OPT_LOCAL) || (indir & (PV_BUF|PV_WIN))) + { + if (indir & PV_BUF) + sctx = &curbuf->b_p_script_ctx[indir & PV_MASK]; + else if (indir & PV_WIN) + sctx = &curwin->w_p_script_ctx[indir & PV_MASK]; + } + + // If "sc_sid" is not set, it means the option value was not modified from + // the default yet. As we move towards Vim9 all the default values should + // be valid Vim9. + return !SCRIPT_ID_VALID(sctx->sc_sid) || + sctx->sc_version >= SCRIPT_VERSION_VIM9; +} + /* * Get the script context of global option "name". * @@ -6507,6 +6537,19 @@ makeset(FILE *fd, int opt_flags, int local_only) else // P_STRING { int do_endif = FALSE; + bool legacy; + +#ifdef FEAT_EVAL + legacy = !is_option_value_vim9(p - &options[0], + round == 1 ? opt_flags | OPT_GLOBAL : OPT_LOCAL); +#else + // I think none of the options that require expression + // evaluation will be present if expression evaluation is + // disabled. For example, 'includeexpr' is set to NULL if + // FEAT_EVAL is not present. So it should be safe to use + // normal "set" and "setlocal" in the session file. + legacy = false; +#endif // Don't set 'syntax' and 'filetype' again if the value is // already right, avoids reloading the syntax file. @@ -6522,8 +6565,8 @@ makeset(FILE *fd, int opt_flags, int local_only) return FAIL; do_endif = TRUE; } - if (put_setstring(fd, cmd, p->fullname, (char_u **)varp, - p->flags) == FAIL) + if (put_setstring(fd, legacy, cmd, p->fullname, + (char_u **)varp, p->flags) == FAIL) return FAIL; if (do_endif) { @@ -6545,14 +6588,23 @@ makeset(FILE *fd, int opt_flags, int local_only) int makefoldset(FILE *fd) { - if (put_setstring(fd, "setlocal", "fdm", &curwin->w_p_fdm, 0) == FAIL # ifdef FEAT_EVAL - || put_setstring(fd, "setlocal", "fde", &curwin->w_p_fde, 0) + sctx_T *fde_script_ctx = &curwin->w_p_script_ctx[WV_FDE]; + // Similarly to the check in is_option_value_vim9(), if "sc_id" is not set + // we have a default value, so it is safe to avoid the ":legacy" prefix. + bool fde_is_legacy = SCRIPT_ID_VALID(fde_script_ctx->sc_sid) + && fde_script_ctx->sc_version < SCRIPT_VERSION_VIM9; +# endif + + if (put_setstring(fd, false, "setlocal", "fdm", &curwin->w_p_fdm, 0) == FAIL +# ifdef FEAT_EVAL + || put_setstring(fd, fde_is_legacy, "setlocal", "fde", + &curwin->w_p_fde, 0) == FAIL # endif - || put_setstring(fd, "setlocal", "fmr", &curwin->w_p_fmr, 0) + || put_setstring(fd, false, "setlocal", "fmr", &curwin->w_p_fmr, 0) == FAIL - || put_setstring(fd, "setlocal", "fdi", &curwin->w_p_fdi, 0) + || put_setstring(fd, false, "setlocal", "fdi", &curwin->w_p_fdi, 0) == FAIL || put_setnum(fd, "setlocal", "fdl", &curwin->w_p_fdl) == FAIL || put_setnum(fd, "setlocal", "fml", &curwin->w_p_fml) == FAIL @@ -6568,6 +6620,7 @@ makefoldset(FILE *fd) static int put_setstring( FILE *fd, + bool legacy, char *cmd, char *name, char_u **valuep, @@ -6578,6 +6631,8 @@ put_setstring( char_u *part = NULL; char_u *p; + if (legacy && fprintf(fd, "legacy ") < 0) + return FAIL; if (fprintf(fd, "%s %s=", cmd, name) < 0) return FAIL; if (*valuep != NULL) @@ -6620,6 +6675,8 @@ put_setstring( p = buf; while (*p != NUL) { + if (legacy && fprintf(fd, "legacy ") < 0) + return FAIL; // for each comma separated option part, append value to // the option, :set rtp+=value if (fprintf(fd, "%s %s+=", cmd, name) < 0) diff --git a/src/proto/option.pro b/src/proto/option.pro index ae586ea978..c6945d80fe 100644 --- a/src/proto/option.pro +++ b/src/proto/option.pro @@ -22,6 +22,7 @@ int was_set_insecurely(win_T *wp, char_u *opt, int opt_flags); void redraw_titles(void); int valid_name(char_u *val, char *allowed); void set_option_sctx_idx(int opt_idx, int opt_flags, sctx_T script_ctx); +bool is_option_value_vim9(int opt_idx, int opt_flags); sctx_T *get_option_sctx(char *name); void set_term_option_sctx_idx(char *name, int opt_idx); char *did_set_arabic(optset_T *args); diff --git a/src/testdir/test_mksession.vim b/src/testdir/test_mksession.vim index f7384f601e..c11e8b849c 100644 --- a/src/testdir/test_mksession.vim +++ b/src/testdir/test_mksession.vim @@ -246,8 +246,8 @@ func Test_mksession_rtp() " determine expected value let expected=split(&rtp, ',') - let expected = map(expected, '"set runtimepath+=".v:val') - let expected = ['set runtimepath='] + expected + let expected = map(expected, '"legacy set runtimepath+=".v:val') + let expected = ['legacy set runtimepath='] + expected let expected = map(expected, {v,w -> substitute(w, $HOME, "~", "g")}) mksession! Xtest_mks.out @@ -1297,9 +1297,10 @@ func Test_mkvimrc() set wildcharm= call assert_fails('mkvimrc Xtestvimrc', 'E189: "Xtestvimrc" exists') mkvimrc! Xtestvimrc - call assert_notequal(-1, index(readfile('Xtestvimrc'), 'set pastetoggle=')) - call assert_notequal(-1, index(readfile('Xtestvimrc'), 'set wildchar=')) - call assert_notequal(-1, index(readfile('Xtestvimrc'), 'set wildcharm=')) + let content = readfile('Xtestvimrc') + call assert_notequal(-1, index(content, 'legacy set pastetoggle=')) + call assert_notequal(-1, index(content, 'set wildchar=')) + call assert_notequal(-1, index(content, 'set wildcharm=')) set pastetoggle& wildchar& wildcharm& call delete('Xtestvimrc') @@ -1810,6 +1811,139 @@ func Test_mksession_vim9_duplicate_import() endfunc +" Make sure options are marked as legacy in the session file when they are set +" by the user from the command execution interface. Only applies to string +" options. +func Test_mksession_preserve_option_script_version_set_manually() + + let orig_includeexpr = &includeexpr + defer execute('let &includeexpr = orig_includeexpr') + + " Using "execute()" to simulate input from the user, rather than the option + " being set directly by the test script itself. + execute('set includeexpr=FooDefault(v:fname)') + + " Try a few options, to make sure they all work correctly. + new + execute('legacy set includeexpr=FooLegacy(v:fname)') + + new + execute('vim9 set includeexpr=FooVim9(v:fname)') + + mksession! XDummySession.vim + defer delete('XDummySession.vim') + + let session_content = readfile('XDummySession.vim') + " We expect that without an explicit script version specification the command + " line input is treated as legacy input. + call assert_notequal(-1, index(session_content, + \ 'legacy setlocal includeexpr=FooDefault(v:fname)')) + call assert_notequal(-1, index(session_content, + \ 'legacy setlocal includeexpr=FooLegacy(v:fname)')) + call assert_notequal(-1, index(session_content, + \ 'setlocal includeexpr=FooVim9(v:fname)')) +endfunc + +" Make sure options preserve legacy/Vim9 script version when written into the +" session file based on the original script that set those options. Only +" applies to string options. +func Test_mksession_preserve_option_script_version_set_from_script() + + CheckFeature packages + + const base = getcwd() . '/rtdir' + " clean up later + defer delete(base, 'rf') + let orig_packpath = &packpath + let &packpath .= ',' . base + defer execute('let &packpath = orig_packpath') + + " Is used to disable file type plugins. + let g:Global_run_ftplugins = 1 + defer execute('unlet g:Global_run_ftplugins') + + " We are going to create two simple file type plugins - one Vim9 and one + " legacy. Each will just set the "includeexpr" locally in the buffer to some + " value. + const root = base . '/pack/test/opt/test_option_script_version' + call mkdir(root . '/ftplugin', 'p') + + let vim9_ftplugin_sources =<< trim END + vim9script + + if !get(g:, 'Global_run_ftplugins') + finish + endif + + &l:include = 'vim9include' + &l:includeexpr = 'vim9fn(v:fname, 1)' + END + call writefile(vim9_ftplugin_sources, root . '/ftplugin/vim9test_lang.vim') + + let legacy_ftplugin_sources =<< trim END + if !get(g:, 'Global_run_ftplugins') + finish + endif + + setlocal include=legacyimport + setlocal includeexpr=legacyfn(v:fname,\".\") + END + call writefile(legacy_ftplugin_sources, root . '/ftplugin/legacytest_lang.vim') + + let orig_runtimepath = &runtimepath + packadd test_option_script_version + defer execute('let &runtimepath = orig_runtimepath') + + " Next we will create two buffers, and set "filetype" in each to match our two + " file type plugin names. The expectation is that "includeexpr" will be set + " to the correct value and a ":legacy" prefix will be used only when the + " original value was set by a legacy script. + filetype plugin on + set filetype=vim9test_lang + + new + set filetype=legacytest_lang + + mksession! XDummySession1.vim + defer delete('XDummySession1.vim') + + " Check that the session file indeed contains correctly set "includeexpr" + " values. + let session1_content = readfile('XDummySession1.vim') + call assert_notequal(-1, index(session1_content, + \ 'setlocal include=vim9include')) + call assert_notequal(-1, index(session1_content, + \ 'setlocal includeexpr=vim9fn(v:fname,\ 1)')) + call assert_notequal(-1, index(session1_content, + \ 'legacy setlocal include=legacyimport')) + call assert_notequal(-1, index(session1_content, + \ 'legacy setlocal includeexpr=legacyfn(v:fname,\".\")')) + + " Now disable our filetype plugins, restore the session file, save session + " again, and make sure that the options are still stored with correct Vim + " script version prefixes. + + let g:Global_run_ftplugins = 0 + %bwipe! + + source XDummySession1.vim + + mksession! XDummySession2.vim + defer delete('XDummySession2.vim') + + " Check that the session file indeed contains correctly set "includeexpr" + " values. + let session2_content = readfile('XDummySession2.vim') + call assert_notequal(-1, index(session2_content, + \ 'setlocal include=vim9include')) + call assert_notequal(-1, index(session2_content, + \ 'setlocal includeexpr=vim9fn(v:fname,\ 1)')) + call assert_notequal(-1, index(session2_content, + \ 'legacy setlocal include=legacyimport')) + call assert_notequal(-1, index(session2_content, + \ 'legacy setlocal includeexpr=legacyfn(v:fname,\".\")')) +endfunc + " 'winminwidth' restore must not fail when the session's saved 'winwidth' is " smaller than the sourcing context's 'winminwidth'. func Test_mksession_winminwidth() diff --git a/src/version.c b/src/version.c index 182dd1d240..7efb39e2a5 100644 --- a/src/version.c +++ b/src/version.c @@ -759,6 +759,8 @@ static char *(features[]) = static int included_patches[] = { /* Add new patch number below this line */ +/**/ + 829, /**/ 828, /**/