feat: set properties on namespaces #28728
Conversation
altermo
force-pushed
the
ns_scope
branch
3 times, most recently
from
fff7048
to
60665db
Compare
|
It may be better to be able to change a namespace from global scope to window scope in one API call. The API will be like |
zeertzjq
added
marks
altermo
changed the title
altermo
force-pushed
the
ns_scope
branch
6 times, most recently
from
0c96b4a
to
32e9982
Compare
Can we have a generic |
altermo
force-pushed
the
ns_scope
branch
4 times, most recently
from
24cbad6
to
2ca429b
Compare
altermo
force-pushed
the
ns_scope
branch
4 times, most recently
from
8d80e61
to
0c91dab
Compare
altermo
force-pushed
the
ns_scope
branch
2 times, most recently
from
2f2787f
to
1fca1d1
Compare
altermo
changed the title
altermo
force-pushed
the
ns_scope
branch
7 times, most recently
from
ac306b4
to
9ff9697
Compare
altermo
force-pushed
the
ns_scope
branch
3 times, most recently
from
87d940b
to
7bbe030
Compare
altermo
force-pushed
the
ns_scope
branch
7 times, most recently
from
95d9399
to
fcaac92
Compare
src/nvim/api/extmark.h
Outdated
| if (!set_has(uint32_t, &namespace_scoped, ns_id)) { | ||
| return true; | ||
| } | ||
|
|
||
| return set_has(uint32_t, &wp->w_ns_set, ns_id); |
There was a problem hiding this comment.
This seems strange. If ns_id is not in namespace_scoped, this function returns true? Why isn't the decision fully decided by w_ns_set, why is namespace_scoped needed at all?
There was a problem hiding this comment.
namespace_scoped is needed because it currently is equivalent to "namespace is scoped in at least one window" and if it doesn't contain the namespace, then that namespace is "global scoped" (which is what you asked for).
Though namespace_scoped can sometimes contain namespaces that are not scoped in any windows, because if a window is deleted, and that window was the only window where the namespace was scoped, then namespace_scoped isn't updated. (I could change this so that it's always synced, but that would entail that when a window is deleted, a namespace can suddenly become global scope, which is probably not what one wants.)
There was a problem hiding this comment.
right, the docstring and names were confusing.
namespace_scopedis needed because it currently is equivalent to "namespace is scoped in at least one window"
Sounds like an unnecessary optimization. We don't need namespace_scoped, we can iterate through all windows and check w_ns_set. That is simpler and requires less bookkeeping.
oh I missed this:
but that would entail that when a window is deleted, a namespace can suddenly become global scope, which is probably not what one wants.
Ok. namespace_scoped is confusing because "global scope" is also a scope. Maybe namespace_localscope is a better name.
altermo
force-pushed
the
ns_scope
branch
2 times, most recently
from
761d0be
to
acf7866
Compare
src/nvim/api/extmark.h
Outdated
| EXTERN Map(String, int) namespace_ids INIT( = MAP_INIT); | ||
| EXTERN Set(uint32_t) namespace_scoped INIT( = SET_INIT); |
There was a problem hiding this comment.
Is it too early to introduce (pseudocode):
namespaces: Map<id, NamespaceProps>
where NamespaceProps is a struct like:
{ scope: NsScope; ... }
Arguably that is similar to namespace_scope_map which I questioned for other reasons. 😓
altermo
force-pushed
the
ns_scope
branch
2 times, most recently
from
4d9c8e7
to
df9e46b
Compare
Co-authored-by: Justin M. Keyes <justinkz@gmail.com>
Use a singular function that can specify properties on the namespace.
Removed
nvim__win_xx_nsfunctions as they are no longer needed.