[RFC] make del_var and set_var not return the old value. #5336
Conversation
| /// @return The old value or nil if there was no previous value. | ||
| /// | ||
| /// @warning It may return nil if there was no previous value | ||
| /// or if previous value was `v:null`. | ||
| Object nvim_buf_set_var(Buffer buffer, String name, Object value, Error *err) | ||
| Object buffer_set_var(Buffer buffer, String name, Object value, Error *err) |
There was a problem hiding this comment.
mark @deprecated ? (same for the others)
There was a problem hiding this comment.
I thought about it, but ain't functions with these names implicitly deprecated? When implementing the doc generation the plan is to not generate docs for these, or alternatively place them in a "deprecated" section. But anyway, it shouldn't hurt to be extra clear, so I'll add it.
f886653 to
9001d5a
Compare
|
@justinmk Thanks for the comments, updated. |
|
LGTM |
| eq(val2, meths.del_var('lua')) | ||
| eq(NIL, request('vim_set_var', 'lua', val1)) | ||
| eq(val1, request('vim_set_var', 'lua', val2)) | ||
| eq(val2, request('vim_del_var', 'lua')) | ||
| end) |
There was a problem hiding this comment.
Where are tests for new functions? Also AFAIR all *_set_var and *_del_var are tested, not just vim_*.
There was a problem hiding this comment.
The one implementing this to start with apparently thought it was redundant, but adding for the others should be a quick matter of find and replace
As discussed in #4411.
#4568 could be used emulate the old behavior, or any client that relies on the old behavior could continue to use the deprecated functions for now; we are not in a hurry removing deprecated functions.
@ZyX-I sorry for duplicating the change in #5119 where
deprectated_dispatch.luais moved to the right location. I needed to fix the cmake rule (missing dependency) and also wanted to mention the name of the file in an error message.