refactor(options): set option value for non-current context directly

Problem: Currently, we use `switch_option_context` to temporarily switch the current option context before setting an option for a different buffer / window. This is not ideal because we already support getting and setting option values for non-current contexts in the underlying implementation.

Solution: Set option value for non-current context by passing the context directly to the lower level functions. Also introduce a new `OptCtx` struct to store option context information, this will scale much better if we add more option scopes and other context information in the future.

src/nvim/api/deprecated.c

@@ -649,8 +649,8 @@ static Object get_option_from(void *from, OptScope scope, String name, Error *er
   OptVal value = NIL_OPTVAL;
 
   if (option_has_scope(opt_idx, scope)) {
-    value = get_option_value_for(opt_idx, scope == kOptScopeGlobal ? OPT_GLOBAL : OPT_LOCAL,
+    value = get_option_value_from(opt_idx, option_ctx_from(scope, from),
-                                 scope, from, err);
+                                  scope == kOptScopeGlobal ? OPT_GLOBAL : OPT_LOCAL);
     if (ERROR_SET(err)) {
       return (Object)OBJECT_INIT;
     }
@@ -701,7 +701,11 @@ static void set_option_to(uint64_t channel_id, void *to, OptScope scope, String
       : ((scope == kOptScopeGlobal) ? OPT_GLOBAL : OPT_LOCAL);
 
   WITH_SCRIPT_CONTEXT(channel_id, {
-    set_option_value_for(name.data, opt_idx, optval, opt_flags, scope, to, err);
+    const char *errmsg
+      = set_option_value_for(opt_idx, optval, option_ctx_from(scope, to), opt_flags);
+    if (errmsg) {
+      api_set_error(err, kErrorTypeException, "%s", errmsg);
+    }
   });
 }
 

src/nvim/api/options.c

@@ -157,8 +157,8 @@ Object nvim_get_option_value(String name, Dict(option) *opts, Error *err)
   void *from = NULL;
   char *filetype = NULL;
 
-  if (!validate_option_value_args(opts, name.data, &opt_idx, &opt_flags, &scope, &from,
+  if (!validate_option_value_args(opts, name.data, &opt_idx, &opt_flags, &scope, &from, &filetype,
-                                  &filetype, err)) {
+                                  err)) {
     return (Object)OBJECT_INIT;
   }
 
@@ -182,7 +182,7 @@ Object nvim_get_option_value(String name, Dict(option) *opts, Error *err)
     from = ftbuf;
   }
 
-  OptVal value = get_option_value_for(opt_idx, opt_flags, scope, from, err);
+  OptVal value = get_option_value_from(opt_idx, option_ctx_from(scope, from), opt_flags);
 
   if (ftbuf != NULL) {
     // restore curwin/curbuf and a few other things
@@ -257,7 +257,11 @@ void nvim_set_option_value(uint64_t channel_id, String name, Object value, Dict(
   });
 
   WITH_SCRIPT_CONTEXT(channel_id, {
-    set_option_value_for(name.data, opt_idx, optval, opt_flags, scope, to, err);
+    const char *errmsg
+      = set_option_value_for(opt_idx, optval, option_ctx_from(scope, to), opt_flags);
+    if (errmsg) {
+      api_set_error(err, kErrorTypeException, "%s", errmsg);
+    }
   });
 }
 

src/nvim/api/vim.c

@@ -982,10 +982,10 @@ Buffer nvim_create_buf(Boolean listed, Boolean scratch, Error *err)
     buf_copy_options(buf, BCO_ENTER | BCO_NOHELP);
 
     if (scratch) {
-      set_option_direct_for(kOptBufhidden, STATIC_CSTR_AS_OPTVAL("hide"), OPT_LOCAL, 0,
+      set_option_direct_for(kOptBufhidden, STATIC_CSTR_AS_OPTVAL("hide"),
-                            kOptScopeBuf, buf);
+                            option_ctx_from(kOptScopeBuf, buf), OPT_LOCAL, 0);
-      set_option_direct_for(kOptBuftype, STATIC_CSTR_AS_OPTVAL("nofile"), OPT_LOCAL, 0,
+      set_option_direct_for(kOptBuftype, STATIC_CSTR_AS_OPTVAL("nofile"),
-                            kOptScopeBuf, buf);
+                            option_ctx_from(kOptScopeBuf, buf), OPT_LOCAL, 0);
       assert(buf->b_ml.ml_mfp->mf_fd < 0);  // ml_open() should not have opened swapfile already
       buf->b_p_swf = false;
       buf->b_p_ml = false;

src/nvim/diff.c

@@ -1391,8 +1391,8 @@ void diff_win_options(win_T *wp, bool addbuf)
     }
     wp->w_p_fdm_save = xstrdup(wp->w_p_fdm);
   }
-  set_option_direct_for(kOptFoldmethod, STATIC_CSTR_AS_OPTVAL("diff"), OPT_LOCAL, 0,
+  set_option_direct_for(kOptFoldmethod, STATIC_CSTR_AS_OPTVAL("diff"),
-                        kOptScopeWin, wp);
+                        option_ctx_from(kOptScopeWin, wp), OPT_LOCAL, 0);
 
   if (!wp->w_p_diff) {
     wp->w_p_fen_save = wp->w_p_fen;

src/nvim/ex_docmd.c

@@ -5294,7 +5294,7 @@ static char *findfunc_find_file(char *findarg, size_t findarg_len, int count)
 /// Returns NULL on success and an error message on failure.
 const char *did_set_findfunc(optset_T *args)
 {
-  buf_T *buf = (buf_T *)args->os_buf;
+  buf_T *buf = args->os_ctx.buf;
   int retval;
 
   if (args->os_flags & OPT_LOCAL) {

src/nvim/insexpand.c

@@ -2398,7 +2398,7 @@ static void copy_global_to_buflocal_cb(Callback *globcb, Callback *bufcb)
 /// lambda expression.
 const char *did_set_completefunc(optset_T *args)
 {
-  buf_T *buf = (buf_T *)args->os_buf;
+  buf_T *buf = args->os_ctx.buf;
   if (option_set_callback_func(buf->b_p_cfu, &cfu_cb) == FAIL) {
     return e_invarg;
   }
@@ -2419,7 +2419,7 @@ void set_buflocal_cfu_callback(buf_T *buf)
 /// lambda expression.
 const char *did_set_omnifunc(optset_T *args)
 {
-  buf_T *buf = (buf_T *)args->os_buf;
+  buf_T *buf = args->os_ctx.buf;
   if (option_set_callback_func(buf->b_p_ofu, &ofu_cb) == FAIL) {
     return e_invarg;
   }
@@ -2440,7 +2440,7 @@ void set_buflocal_ofu_callback(buf_T *buf)
 /// lambda expression.
 const char *did_set_thesaurusfunc(optset_T *args FUNC_ATTR_UNUSED)
 {
-  buf_T *buf = (buf_T *)args->os_buf;
+  buf_T *buf = args->os_ctx.buf;
   int retval;
 
   if (args->os_flags & OPT_LOCAL) {

src/nvim/option_defs.h

@@ -81,6 +81,12 @@ typedef struct {
   OptValData data;
 } OptVal;
 
+/// Context that an option is being set for.
+typedef struct {
+  win_T *win;
+  buf_T *buf;
+} OptCtx;
+
 /// :set operator types
 typedef enum {
   OP_NONE = 0,
@@ -122,8 +128,7 @@ typedef struct {
   /// length of the error buffer
   size_t os_errbuflen;
 
-  void *os_win;
+  OptCtx os_ctx;
-  void *os_buf;
 } optset_T;
 
 /// Type for the callback function that is invoked after an option value is
@@ -192,3 +197,12 @@ typedef struct {
   OptVal def_val;                    ///< default value
   LastSet last_set;                  ///< script in which the option was last set
 } vimoption_T;
+
+/// Execute code with autocmd context
+#define WITH_AUCMD_CONTEXT(ctx, code) \
+  do { \
+    aco_save_T _aco; \
+    aucmd_prepbuf_win(&_aco, ctx.buf, ctx.win); \
+    code; \
+    aucmd_restbuf(&_aco); \
+  } while (0)

src/nvim/optionstr.c

@@ -567,7 +567,7 @@ int expand_set_backspace(optexpand_T *args, int *numMatches, char ***matches)
 /// The 'backupcopy' option is changed.
 const char *did_set_backupcopy(optset_T *args)
 {
-  buf_T *buf = (buf_T *)args->os_buf;
+  buf_T *buf = args->os_ctx.buf;
   const char *oldval = args->os_oldval.string.data;
   int opt_flags = args->os_flags;
   char *bkc = p_bkc;
@@ -655,7 +655,7 @@ const char *did_set_breakat(optset_T *args FUNC_ATTR_UNUSED)
 /// The 'breakindentopt' option is changed.
 const char *did_set_breakindentopt(optset_T *args)
 {
-  win_T *win = (win_T *)args->os_win;
+  win_T *win = args->os_ctx.win;
   char **varp = (char **)args->os_varp;
 
   if (briopt_check(*varp, varp == &win->w_p_briopt ? win : NULL) == FAIL) {
@@ -682,7 +682,7 @@ int expand_set_breakindentopt(optexpand_T *args, int *numMatches, char ***matche
 /// The 'bufhidden' option is changed.
 const char *did_set_bufhidden(optset_T *args)
 {
-  buf_T *buf = (buf_T *)args->os_buf;
+  buf_T *buf = args->os_ctx.buf;
   return did_set_opt_strings(buf->b_p_bh, opt_bh_values, false);
 }
 
@@ -698,8 +698,8 @@ int expand_set_bufhidden(optexpand_T *args, int *numMatches, char ***matches)
 /// The 'buftype' option is changed.
 const char *did_set_buftype(optset_T *args)
 {
-  buf_T *buf = (buf_T *)args->os_buf;
+  buf_T *buf = args->os_ctx.buf;
-  win_T *win = (win_T *)args->os_win;
+  win_T *win = args->os_ctx.win;
   // When 'buftype' is set, check for valid value.
   if ((buf->terminal && buf->b_p_bt[0] != 't')
       || (!buf->terminal && buf->b_p_bt[0] == 't')
@@ -780,7 +780,7 @@ static const char *did_set_global_chars_option(win_T *win, char *val, CharsOptio
 /// The 'fillchars' option or the 'listchars' option is changed.
 const char *did_set_chars_option(optset_T *args)
 {
-  win_T *win = (win_T *)args->os_win;
+  win_T *win = args->os_ctx.win;
   char **varp = (char **)args->os_varp;
   const char *errmsg = NULL;
 
@@ -815,7 +815,7 @@ int expand_set_chars_option(optexpand_T *args, int *numMatches, char ***matches)
 /// The 'cinoptions' option is changed.
 const char *did_set_cinoptions(optset_T *args)
 {
-  buf_T *buf = (buf_T *)args->os_buf;
+  buf_T *buf = args->os_ctx.buf;
   // TODO(vim): recognize errors
   parse_cino(buf);
 
@@ -840,7 +840,7 @@ int expand_set_clipboard(optexpand_T *args, int *numMatches, char ***matches)
 /// The 'colorcolumn' option is changed.
 const char *did_set_colorcolumn(optset_T *args)
 {
-  win_T *win = (win_T *)args->os_win;
+  win_T *win = args->os_ctx.win;
   char **varp = (char **)args->os_varp;
   return check_colorcolumn(*varp, varp == &win->w_p_cc ? win : NULL);
 }
@@ -985,7 +985,7 @@ const char *did_set_completeitemalign(optset_T *args)
 /// The 'completeopt' option is changed.
 const char *did_set_completeopt(optset_T *args FUNC_ATTR_UNUSED)
 {
-  buf_T *buf = (buf_T *)args->os_buf;
+  buf_T *buf = args->os_ctx.buf;
   char *cot = p_cot;
   unsigned *flags = &cot_flags;
 
@@ -1021,7 +1021,7 @@ int expand_set_completeopt(optexpand_T *args, int *numMatches, char ***matches)
 /// The 'completeslash' option is changed.
 const char *did_set_completeslash(optset_T *args)
 {
-  buf_T *buf = (buf_T *)args->os_buf;
+  buf_T *buf = args->os_ctx.buf;
   if (check_opt_strings(p_csl, opt_csl_values, false) != OK
       || check_opt_strings(buf->b_p_csl, opt_csl_values, false) != OK) {
     return e_invarg;
@@ -1068,7 +1068,7 @@ int expand_set_cpoptions(optexpand_T *args, int *numMatches, char ***matches)
 /// The 'cursorlineopt' option is changed.
 const char *did_set_cursorlineopt(optset_T *args)
 {
-  win_T *win = (win_T *)args->os_win;
+  win_T *win = args->os_ctx.win;
   char **varp = (char **)args->os_varp;
 
   // This could be changed to use opt_strings_flags() instead.
@@ -1176,12 +1176,12 @@ int expand_set_eadirection(optexpand_T *args, int *numMatches, char ***matches)
 /// options is changed.
 const char *did_set_encoding(optset_T *args)
 {
-  buf_T *buf = (buf_T *)args->os_buf;
+  buf_T *buf = args->os_ctx.buf;
   char **varp = (char **)args->os_varp;
   int opt_flags = args->os_flags;
   // Get the global option to compare with, otherwise we would have to check
   // two values for all local options.
-  char **gvarp = (char **)get_option_varp_scope_from(args->os_idx, OPT_GLOBAL, buf, NULL);
+  char **gvarp = (char **)get_varp_scope_from(get_option(args->os_idx), OPT_GLOBAL, args->os_ctx);
 
   if (gvarp == &p_fenc) {
     if (!MODIFIABLE(buf) && opt_flags != OPT_GLOBAL) {
@@ -1246,7 +1246,7 @@ int expand_set_eventignore(optexpand_T *args, int *numMatches, char ***matches)
 /// The 'fileformat' option is changed.
 const char *did_set_fileformat(optset_T *args)
 {
-  buf_T *buf = (buf_T *)args->os_buf;
+  buf_T *buf = args->os_ctx.buf;
   char **varp = (char **)args->os_varp;
   const char *oldval = args->os_oldval.string.data;
   int opt_flags = args->os_flags;
@@ -1347,7 +1347,7 @@ int expand_set_foldcolumn(optexpand_T *args, int *numMatches, char ***matches)
 /// The 'foldexpr' option is changed.
 const char *did_set_foldexpr(optset_T *args)
 {
-  win_T *win = (win_T *)args->os_win;
+  win_T *win = args->os_ctx.win;
   did_set_optexpr(args);
   if (foldmethodIsExpr(win)) {
     foldUpdateAll(win);
@@ -1358,7 +1358,7 @@ const char *did_set_foldexpr(optset_T *args)
 /// The 'foldignore' option is changed.
 const char *did_set_foldignore(optset_T *args)
 {
-  win_T *win = (win_T *)args->os_win;
+  win_T *win = args->os_ctx.win;
   if (foldmethodIsIndent(win)) {
     foldUpdateAll(win);
   }
@@ -1368,7 +1368,7 @@ const char *did_set_foldignore(optset_T *args)
 /// The 'foldmarker' option is changed.
 const char *did_set_foldmarker(optset_T *args)
 {
-  win_T *win = (win_T *)args->os_win;
+  win_T *win = args->os_ctx.win;
   char **varp = (char **)args->os_varp;
   char *p = vim_strchr(*varp, ',');
 
@@ -1390,7 +1390,7 @@ const char *did_set_foldmarker(optset_T *args)
 /// The 'foldmethod' option is changed.
 const char *did_set_foldmethod(optset_T *args)
 {
-  win_T *win = (win_T *)args->os_win;
+  win_T *win = args->os_ctx.win;
   char **varp = (char **)args->os_varp;
   if (check_opt_strings(*varp, opt_fdm_values, false) != OK || **varp == NUL) {
     return e_invarg;
@@ -1536,7 +1536,7 @@ const char *did_set_iskeyword(optset_T *args)
 /// changed.
 const char *did_set_isopt(optset_T *args)
 {
-  buf_T *buf = (buf_T *)args->os_buf;
+  buf_T *buf = args->os_ctx.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
@@ -1565,7 +1565,7 @@ int expand_set_jumpoptions(optexpand_T *args, int *numMatches, char ***matches)
 /// The 'keymap' option has changed.
 const char *did_set_keymap(optset_T *args)
 {
-  buf_T *buf = (buf_T *)args->os_buf;
+  buf_T *buf = args->os_ctx.buf;
   char **varp = (char **)args->os_varp;
   int opt_flags = args->os_flags;
 
@@ -2053,7 +2053,7 @@ int expand_set_showcmdloc(optexpand_T *args, int *numMatches, char ***matches)
 /// The 'signcolumn' option is changed.
 const char *did_set_signcolumn(optset_T *args)
 {
-  win_T *win = (win_T *)args->os_win;
+  win_T *win = args->os_ctx.win;
   char **varp = (char **)args->os_varp;
   const char *oldval = args->os_oldval.string.data;
   if (check_signcolumn(*varp, varp == &win->w_p_scl ? win : NULL) != OK) {
@@ -2079,7 +2079,7 @@ int expand_set_signcolumn(optexpand_T *args, int *numMatches, char ***matches)
 /// The 'spellcapcheck' option is changed.
 const char *did_set_spellcapcheck(optset_T *args)
 {
-  win_T *win = (win_T *)args->os_win;
+  win_T *win = args->os_ctx.win;
   // When 'spellcapcheck' is set compile the regexp program.
   return compile_cap_prog(win->w_s);
 }
@@ -2113,7 +2113,7 @@ const char *did_set_spelllang(optset_T *args)
 /// The 'spelloptions' option is changed.
 const char *did_set_spelloptions(optset_T *args)
 {
-  win_T *win = (win_T *)args->os_win;
+  win_T *win = args->os_ctx.win;
   int opt_flags = args->os_flags;
   const char *val = args->os_newval.string.data;
 
@@ -2189,7 +2189,7 @@ const char *did_set_statusline(optset_T *args)
 static const char *did_set_statustabline_rulerformat(optset_T *args, bool rulerformat,
                                                      bool statuscolumn)
 {
-  win_T *win = (win_T *)args->os_win;
+  win_T *win = args->os_ctx.win;
   char **varp = (char **)args->os_varp;
   if (rulerformat) {       // reset ru_wid first
     ru_wid = 0;
@@ -2264,7 +2264,7 @@ const char *did_set_tabline(optset_T *args)
 /// The 'tagcase' option is changed.
 const char *did_set_tagcase(optset_T *args)
 {
-  buf_T *buf = (buf_T *)args->os_buf;
+  buf_T *buf = args->os_ctx.buf;
   int opt_flags = args->os_flags;
 
   unsigned *flags;
@@ -2337,7 +2337,7 @@ const char *did_set_titlestring(optset_T *args)
 /// The 'varsofttabstop' option is changed.
 const char *did_set_varsofttabstop(optset_T *args)
 {
-  buf_T *buf = (buf_T *)args->os_buf;
+  buf_T *buf = args->os_ctx.buf;
   char **varp = (char **)args->os_varp;
 
   if (!(*varp)[0] || ((*varp)[0] == '0' && !(*varp)[1])) {
@@ -2367,8 +2367,8 @@ const char *did_set_varsofttabstop(optset_T *args)
 /// The 'varstabstop' option is changed.
 const char *did_set_vartabstop(optset_T *args)
 {
-  buf_T *buf = (buf_T *)args->os_buf;
+  buf_T *buf = args->os_ctx.buf;
-  win_T *win = (win_T *)args->os_win;
+  win_T *win = args->os_ctx.win;
   char **varp = (char **)args->os_varp;
 
   if (!(*varp)[0] || ((*varp)[0] == '0' && !(*varp)[1])) {
@@ -2417,7 +2417,7 @@ const char *did_set_viewoptions(optset_T *args FUNC_ATTR_UNUSED)
 /// The 'virtualedit' option is changed.
 const char *did_set_virtualedit(optset_T *args)
 {
-  win_T *win = (win_T *)args->os_win;
+  win_T *win = args->os_ctx.win;
 
   char *ve = p_ve;
   unsigned *flags = &ve_flags;
@@ -2527,7 +2527,7 @@ const char *did_set_winbar(optset_T *args)
 /// The 'winhighlight' option is changed.
 const char *did_set_winhighlight(optset_T *args)
 {
-  win_T *win = (win_T *)args->os_win;
+  win_T *win = args->os_ctx.win;
   char **varp = (char **)args->os_varp;
   if (!parse_winhl_opt(*varp, varp == &win->w_p_winhl ? win : NULL)) {
     return e_invarg;

src/nvim/tag.c

@@ -228,7 +228,7 @@ static Callback tfu_cb;          // 'tagfunc' callback function
 /// a function (string), or function(<name>) or funcref(<name>) or a lambda.
 const char *did_set_tagfunc(optset_T *args)
 {
-  buf_T *buf = (buf_T *)args->os_buf;
+  buf_T *buf = args->os_ctx.buf;
 
   callback_free(&tfu_cb);
   callback_free(&buf->b_tfu_cb);

src/nvim/winfloat.c

@@ -411,8 +411,8 @@ win_T *win_float_create(bool enter, bool new_buf)
       return handle_error_and_cleanup(wp, &err);
     }
     buf->b_p_bl = false;  // unlist
-    set_option_direct_for(kOptBufhidden, STATIC_CSTR_AS_OPTVAL("wipe"), OPT_LOCAL, 0,
+    set_option_direct_for(kOptBufhidden, STATIC_CSTR_AS_OPTVAL("wipe"),
-                          kOptScopeBuf, buf);
+                          option_ctx_from(kOptScopeBuf, buf), OPT_LOCAL, 0);
     win_set_buf(wp, buf, &err);
     if (ERROR_SET(&err)) {
       return handle_error_and_cleanup(wp, &err);

test/functional/lua/with_spec.lua

@@ -882,11 +882,7 @@ describe('vim._with', function()
       eq({
         bo = { cms_cur = '// %s', cms_other = '-- %s', ul_cur = 250, ul_other = -123456 },
         wo = { ve_cur = 'insert', ve_other = 'block', winbl_cur = 25, winbl_other = 10 },
-        -- Global `winbl` inside context ideally should be untouched and equal
+        go = { cms = '-- %s', ul = 0, ve = 'none', winbl = 50, lmap = 'xy,yx' },
-        -- to 50. It seems to be equal to 0 because `context.buf` uses
-        -- `aucmd_prepbuf` C approach which has no guarantees about window or
-        -- window option values inside context.
-        go = { cms = '-- %s', ul = 0, ve = 'none', winbl = 0, lmap = 'xy,yx' },
       }, out.inner)
       eq(out.before, out.after)
     end)