RemoteMap returns copies instead of references, leading to unintuitive/unfriendly behaviour

[phodge]

If I have a vim global variable that contains a dict, e.g. g:foo = {"a": "A"}, then the current implementation of RemoteMap leads to some unexpected, unintuitive behaviours when I try to manipulate it in python. For example:

foo = nvim.vars["foo"]
foo["b"] = "B"
print(nvim.vars["foo"].keys())  # -> ["a"]

In python, dicts are normally assigned by reference so a python programmer would expect a change to the local variable foo to also affect the Vim variable. But instead, they are only changing a copy. This is particularly confusing when you write a line of code like this:

nvim.vars["foo"]["b"] = "B"

This doesn't raise an exception when you run it, and it's such a simple line of code that programmers familiar with python won't suspect that it contains a bug. It's just assigning a value to a dict, how can it possibly be broken? But it is broken because when python assigns "B" to the key "b" it is only modifying a copy of g:foo, and the copy is immediately garbage collected and the change never makes it back to neovim.

---

I can think of three separate actions which will address this in different ways:

1) Instead of using __get__() and __set__() for RemoteMap key access, have .read() and .write() methods that A) make it obvious that you're getting a copy of the variable and B) make it obvious how many RPC calls are happening. This isn't as slick as __get__(), but at least you intuitively know what's going on.

2) Bring back the __get__() and __set__() for RemoteMap, but have them return pointers to variables that are generally assigned by reference (vim and python both conveniently have the same opinions on what should be copied vs referenced). This has the disadvantage that a RemoteMap gets returned in place of a simple dict, meaning every key lookup is a separate RPC call, but this might be what the users want.

3) Document how the .vars RemoteMaps work. Obviously documentation is already a big TODO for this project, but we need to make sure this behaviour is specifically covered if we want to make this API friendly to newcomers.

---

This issue was also discussed in the neovim gitter starting here: https://gitter.im/neovim/neovim?at=591e0de4631b8e4e61ed6a20

[justinmk]

My understanding is that the current behavior matches the legacy Vim python API (before bindeval). That's why the behavior is "expected". cc @ZyX-I

[phodge]

@justinmk I did some more digging and have found that Vim and Neovim are not consistent with how they handle var references. Consider this snippet:

python << EOF
import vim
vim.vars["foo"] = {"a": "A"}
vim.vars["foo"]["b"] = "B"
foo = vim.vars["foo"]
foo["c"] = "C"
EOF
let g:foo

When I run this in vim 8.0.503, g:foo is a dict with 3 keys. When I run the same code in neovim v0.2.0, g:foo is a dict with only 1 key.

[ZyX-I]

@justinmk Before bindeval there was no such thing as vim.vars; more AFAIR vim.vars was introduced in a sequence of patches when bindeval was already there for a while and I definitely never coded Neovim behaviour in. So no, it does not.

[justinmk]

@phodge That's because Vim 8 has bindeval. Neovim's support of the Vim legacy API only meets Vim 7.4.

In any case it sounds like the current behavior is unacceptable :)

[bfredl]

The behavior is expected as long as one is aware that it is an rpc interface; the problem is the that it is so well integrated that one often does not think of it as an rpc api. Python does not have frozendict or frozenlist, only frozenset (tuple can be abused as frozenlist, but it often carries the indication of different semantics than a list, (1,2) != [1,2] goes along with this), so the mutable types are expected to be used even in a value context (like serialization/deserialization). But the surronding vars definitely has mutable mapping sematics, so dict syntax should be used. Personally I'm leaning towards option 3, but if someone wants to implement option 2 (vim compat is nice of course) that would be fine as long as one could still get a deep copy in a single rpc roundtrip and simple syntax.