Add notifications about variable changes

[ZyX-I]

Some thoughts about what can actually be planned:

Notifications about variable changes may be done two ways:

1. Handle g:, b:, … “public” scope dictionaries specially and run a C equivalent of let v:…=…|doautocmd VariableChanged g:foo on each change (== autocmd VariableChanged g:foo to catch changes), but only to these scope dictionaries. This is more consistent with other things that are run automatically, but is rather not flexible. Also this variant is not able to notify about changes in nested structures (e.g. for let g:foo.bar=1) which is not very common in plugin settings, but is still present.
2. Do not use any autocommands, but add functions watch_dict_key(dictionary, key, funcref) and watch_dict_changes(dictionary, funcref) (also watch_list_changes, but without watch_list_index). Both functions will make NeoVim run funcref(kind_of_change, dictionary, key, old_value) on changes, but first is limited to one dictionary key* and intended for use with global scope (watching plugin own configuration variables) and second is intended for watching nested dictionaries that may be used for complex configuration.

First is more consistent, but not flexible. Second has much more use cases.

* Maybe even only to existing so that watchers can be attached directly to dictitem_T, but that should increase memory usage by one pointer per each dictionary item. Current usage: two bytes (v_type, v_lock), alignment, one pointer (== two pointers if you consider the alignment), one byte for flags and at least two bytes for key.

[tarruda]

The second approach(using funcrefs) is clearly more flexible, and faster than using autocmds because we wont be dealing with global events(no need to match against all registered autocmd pattenrs). Instead, each dict will have a ufunc_T* field that if != NULL will be invoked to notify listeners of that dict.

Here are some problems with using JobActivity in the terminal branch:

• Data received from the job needs to have an extra layer of processing before it is delivered to vimscript, not to mention they are handled as deferred events which generate K_EVENT keypresses(data coming from the terminal can result in pending commands being cancelled)
• Cant use has_autocmd in the job callback because somewhere along the path there are os_breakcheck calls, which can result in recursive event loop invocations.

The result is that every terminal needs to pass data to JobActivity even if there are no registered autocmds(which is the common case for terminals) and generates a lot of useless K_EVENT keys.

This commit fixes the problem by changing the job API to use callbacks instead, and the result is not only a cleaner API but a faster and simpler implementation for terminals that don't need to pass data to job handlers. I plan to merge it after #2076.

[ZyX-I]

each dict will have a ufunc_T* field that if != NULL will be invoked to notify listeners of that dict.

I was actually expecting a single linked list:

typedef struct dict_watcher {
    struct dict_watcher *next;
    char *func_name; // Will be proper function pointer later.
} DictWatcher;

(a pointer to such list lives in dictionary). Main question is how to watch for keys (in my opinion this is better then watching for the whole dictionary): either have a hash pointer attached to the dictionary (+1 if operation per each and every setitem/delitem operation in best case which should be negligible, +1 if, 1 hash lookup and a VimL function call in worst case). Or attach watcher to dictitem_T (will not track key additions, so should probably not be done). Or use the first variant, but implemented in VimL via global dict watcher (needs id function which returns some unique object identifier (in Python this is simply PyObject* pointer casted to int)).

[tarruda]

Main question is how to watch for keys (in my opinion this is better then watching for the whole dictionary)

I think that watching the whole dictionary is better, because plugins can simply namespace their configuration options if more granularity is required. For example, say a plugin has options split into three categories(window, buffer and syntax):

let s:plugin_options = {
\ 'window_options': {},
\ 'buffer_options': {},
\ 'syntax_options': {},
\ }
call watch_dict(s:plugin_options.window_options, 's:WindowOptionChanged')
call watch_dict(s:plugin_options.buffer_options, 's:BufferOptionChanged')
call watch_dict(s:plugin_options.syntax_options, 's:SyntaxOptionChanged')

Note that this does not rule out the possibility of using patterns to match variable names, though namespacing is probably better

[ZyX-I]

Problem is that dictionaries can simply be removed. Or replaced:

let s:plugin_options_for_foo = { … }
let s:plugin_options_for_bar = { … }
let g:plugin_options = s:plugin_options_for_foo

. You can’t cope with this without either watching the g: dictionary itself (what I do not like) or specific key in this dictionary.

WTF is s:plugin_option and how it can be used? Especially its buffer_options part?

[tarruda]

WTF is s:plugin_option and how it can be used? Especially its buffer_options part?

Nvm that, I'm just used to script variables but for configuring a plugin its clearly not an option

[ZyX-I]

You can use script variables for configuration: if you create configuration via functions. Not sure why would variable notifications be needed then though.

Currently such notifications regarding configuration are needed just not to waste time at startup on calling loads of functions and loading loads of autoload scripts. They are not needed to receive notification about variable changes: one may just require doing all configuration via autoload functions.

[tarruda]

A variant of your suggestion:

typedef struct dict_watcher {
    struct dict_watcher *next;
    void (*callback)(dict_T *dict, char *key, typval_T value, void *data);
    char *pattern;
} DictWatcher;

If pattern == NULL the callback will simply be invoked for every key. The same function for matching autocmds can be used for matching the patterns.

The advantage is that watchers can be used by C code to implement options decoupled from nvim core.

[tarruda]

Another possibility:

typedef struct dict_watcher {
    struct dict_watcher *next;
    typval_T (*callback)(dict_T *dict, char *key, typval_T value, void *data);
    char *pattern;
} DictWatcher;

This one will use the return value of the callback as the actual option value. If the type is VAR_UNKNOWN the option is not set(failed validation).

[ZyX-I]

I guess this variant is missing void *data field.

[ZyX-I]

And DictEventType argument in function signature.

typedef enum {
    kDictEventAdded,    ///< New key appeared in dictionary, value={VAR_UNKNOWN, {NULL}}
    kDictEventRemoved,  ///< Key was removed from dictionary, value is what was removed
    kDictEventChanged,  ///< Key value changed, value is old value
} DictEventType;

[ZyX-I]

Transformation of values will be rather unexpected. I would suggest

typedef void (*DictWatcherCallback)(void *data, DictEventType change_type, const dict_T *dict, const char *key, const typval_T value);
typedef struct dict_watcher {
    DictWatcherCallback notify;
    void *data;
    char *pattern;
    struct dict_watcher *next;
} DictWatcherDef;

(note const dict_T). If option is invalid it is expected to be ignored with the error message and default value will be used instead. Or, like current plugins do, garbage in-garbage out (not in UIs or other core and near-core components).

[tarruda]

Your'e right. If transformation is required the plugin can simply set a new value in the callback(the watcher needs to be disabled when running the callback to avoid infinite recursion)

[tarruda]

This could very well be used to implement #1913, options could become reserved dicts prefixed with nvim

{g,b,w,t}:nvim_options for global, buffer, window and tabpage options, the syntax for setting options would be kept for compatiblity, but it would essentially be the same as setting keys in those dicts. It would allow options to be decoupled from options.c and implemented in a per-module basis.

[justinmk]

options could become reserved dicts prefixed with nvim
{g,b,w,t}:nvim_options

How about making nvim itself a dictionary? e.g. {g,b,w,t}:nvim.options, {g,b,w,t}:nvim.ui, etc.