Problem: Numberwidth may depend on number of signs with text in the
buffer and is not handled correctly for extmark signs.
Solution: Move legacy sign code for changed numberwidth so that it is
handled properly for legacy and extmark signs alike.
Problem:
Higher-priority signs may be hidden by lower-priority signs.
Solution:
Place higher-priority signs from the left.
Example:
nvim_buf_set_extmark(0, ns, 0, -1, {sign_text='H', priority=1})
nvim_buf_set_extmark(0, ns, 0, -1, {sign_text='W', priority=2})
nvim_buf_set_extmark(0, ns, 0, -1, {sign_text='E', priority=3})
Before:
| |
H | W E |
^ | |
Not visible
After:
| |
| E W | H
| | ^
Not visible
Fixes#16632
This reverts commit 15e77a56b7.
Subpriorities were added in https://github.com/neovim/neovim/pull/27131
as a mechanism for enforcing query order when using iter_matches in the
Tree-sitter highlighter. However, iter_matches proved to have too many
complications to use in the highlighter so we eventually reverted back
to using iter_captures (https://github.com/neovim/neovim/pull/27901).
Thus, subpriorities are no longer needed and can be removed.
Problem: Invalid assert for empty signcols range. The empty range
should already be removed from "b_signcols" at this point.
The "clear" == kTrue call before the splice that made the
range empty will have removed it, and the "clear" == kNone
call after the splice already ignores the empty range.
Solution: Return early when "row2" < "row1".
A lot of functions in move.c only worked for curwin, alternatively
took a `wp` arg but still only work if that happens to be curwin.
Refactor those that are needed for update_topline(wp) to work
for any window.
fixes#27723fixes#27720
Problem: #25826 added a (duplicate) sign comparison function, which was
modified and strayed from the original in #27418.
Solution: Merge the two functions and add a display test that actually
tests for this order in addition to the legacy tests.
Implement api_keydict_to_dict as the complement to api_dict_to_keydict
Fix a conversion error when nvim_get_win_config gets called from lua,
where Float values "x" and "y" didn't get converted to lua numbers.
The "priority" field of extmarks can be used to set priorities of
extmarks which dictates which highlight group a range will actually have
when there are multiple extmarks applied. However, when multiple
extmarks have the same priority, the only way to enforce an actual
priority is through the order in which the extmarks are set.
It is not always possible or desirable to set extmarks in a specific
order, however, so we add a new "subpriority" field that explicitly
enforces the ordering of extmarks that have the same priority.
For now this will be used only to enforce priority of treesitter
highlights. A single node in a treesitter tree may match multiple
captures, in which case that node will have multiple extmarks set. The
order in which captures are returned from the treesitter API is not
_necessarily_ in the same order they are defined in a query file, so we
use the new subpriority field to force that ordering.
For now subpriorites are not documented and are not meant to be used by
external code, and it only applies to ephemeral extmarks. We indicate
the "private" nature of subpriorities by prefixing the field name with
an "_".
The data.sh.url field is valid only when item.kind is
kDecorKindHighlight. The `if` block just before this line already does
that check (as well as checking `active`) so move the access of
`data.sh.url` into that block.
Problem: Marks moved by undo may be lost to "b_signcols.count".
Solution: Count signs for each undo object separately instead of
once for the entire undo.
Extmarks can contain URLs which can then be drawn in any supporting UI.
In the TUI, for example, URLs are "drawn" by emitting the OSC 8 control
sequence to the TTY. On terminals which support the OSC 8 sequence this
will create clickable hyperlinks.
URLs are treated as inline highlights in the decoration subsystem, so
are included in the `DecorSignHighlight` structure. However, unlike
other inline highlights they use allocated memory which must be freed,
so they set the `ext` flag in `DecorInline` so that their lifetimes are
managed along with other allocated memory like virtual text.
The decoration subsystem then adds the URLs as a new highlight
attribute. The highlight subsystem maintains a set of unique URLs to
avoid duplicating allocations for the same string. To attach a URL to an
existing highlight attribute we call `hl_add_url` which finds the URL in
the set (allocating and adding it if it does not exist) and sets the
`url` highlight attribute to the index of the URL in the set (using an
index helps keep the size of the `HlAttrs` struct small).
This has the potential to lead to an increase in highlight attributes
if a URL is used over a range that contains many different highlight
attributes, because now each existing attribute must be combined with
the URL. In practice, however, URLs typically span a range containing a
single highlight (e.g. link text in Markdown), so this is likely just a
pathological edge case.
When a new highlight attribute is defined with a URL it is copied to all
attached UIs with the `hl_attr_define` UI event. The TUI manages its own
set of URLs (just like the highlight subsystem) to minimize allocations.
The TUI keeps track of which URL is "active" for the cell it is
printing. If no URL is active and a cell containing a URL is printed,
the opening OSC 8 sequence is emitted and that URL becomes the actively
tracked URL. If the cursor is moved while in the middle of a URL span,
we emit the terminating OSC sequence to prevent the hyperlink from
spanning multiple lines.
This does not support nested hyperlinks, but that is a rare (and,
frankly, bizarre) use case. If a valid use case for nested hyperlinks
ever presents itself we can address that issue then.
This expands on the global "don't pay for what you don't use" rules for
these special extmark decorations:
- inline virtual text, which needs to be processed in plines.c when we
calculate the size of text on screen
- virtual lines, which are needed when calculating "filler" lines
- signs, with text and/or highlights, both of which needs to be
processed for the entire line already at the beginning of a line.
This adds a count to each node of the marktree, for how many special
marks of each kind can be found in the subtree for this node. This makes
it possible to quickly skip over these extra checks, when working in
regions of the buffer not containing these kind of marks, instead of
before where this could just be skipped if the entire _buffer_
didn't contain such marks.
Problem: Wrong "clear" argument passed to buf_signcols_count_range
when initializing "b_signcols.count" for the first time.
Solution: Pass kFalse so that the "nested" counter is not incorrectly
decremented.
Problem: Some edge cases to the old (pre-#26406) and current "b_signcols"
structure result in an incorrectly sized "auto" 'signcolumn'.
Solution: * Implement a simpler 'signcolumn' validation strategy by immediately
counting the number of signs in a range upon sign insertion and
deletion. Decrease in performance here but there is a clear path
forward to decreasing this performance hit by moving signs to a
dedicated marktree, or by adding meta-data to the existing
marktree which may be queried more efficiently?
* Also replace "max_count" and keep track of the number of lines with
a certain number of signs. This makes it so that it is no longer
necessary to scan the entire buffer when the maximum number of signs
decreases. This likely makes the commit a net increase in performance.
* To ensure correctness we also have re-initialize the count for an
edited region that spans multiple lines. Such an edit may move the
signs within it. Thus we count and decrement before splicing the
marktree and count and increment after.
Problem: Decor range is being redrawn unconditionally, negating the
more conservative line redrawing that follows.
Solution: Get rid of unconditional redraw_buf_range_later() call.
Remove `export` pramgas from defs headers as it causes IWYU to believe
that the definitions from the defs headers comes from main header, which
is not what we really want.
Problem: Unable to predict which byte-offset to place virtual text to
make it repeat visually in the wrapped part of a line.
Solution: Add a flag to nvim_buf_set_extmark() that causes virtual
text to repeat in wrapped lines.
We do not need an enum to keep track of what place in win_line() we
currently are at. We already have a variable which keeps track where
in the code we currently are (and thus what part of the line we are
currently rendering), it is called the _program counter_. When we need
non-linear or self-referential control-flow anyway for a laugh, we
have a mechanism for that, it is called _function calls_.
Do not "save" and "restore" the wlv->n_extra state every time the
columns are to be drawn. This sort of thing needs to go away. Instead of
setting the n_extra variables and then going to the outer while loop,
the text in the columns can be rendered by just simply putting the text
into the cells of the screen line, right away. Even in nvim this can be
tricky sometimes, luckily we can use function calls to abstract this
logic, which means that this handy data structure called the _call
stack_ is handling saving away state temporarily, and restoring it back
when we need it again.
Lastly, but not least, as we now have direct control how signs
are rendered, these can be stored as schar_T[2] and be directly
put on screen as such.
buf_signcols_validate_range() is only called in a buffer with signs, in
which the marktree is thus non-empty. Also remove a redundant comment,
condition and variable.
*** CID 470331: Error handling issues (CHECKED_RETURN)
/src/nvim/decoration.c: 824 in buf_signcols_validate_range()
818
819 // Allocate an array of integers holding the overlapping signs in the range.
820 assert(row2 >= row1);
821 int *overlap = xcalloc(sizeof(int), (size_t)(row2 + 1 - row1));
822
823 // First find the number of overlapping signs at "row1".
>>> CID 470331: Error handling issues (CHECKED_RETURN)
>>> Calling "marktree_itr_get_overlap" without checking return value (as is done elsewhere 4 out of 5 times).
824 marktree_itr_get_overlap(buf->b_marktree, currow, 0, itr);
825 while (marktree_itr_step_overlap(buf->b_marktree, itr, &pair)) {
826 if (!mt_invalid(pair.start) && pair.start.flags & MT_FLAG_DECOR_SIGNTEXT) {
827 overlap[0]++;
828 }
829 }
Problem: The entire marktree needs to be traversed each time a sign is
removed from the sentinel line.
Solution: Remove sentinel line and instead keep track of the number of
lines that hold up the 'signcolumn' in "max_count". Adjust this
number for added/removed signs, and set it to 0 when the
maximum number of signs on a line changes. Only when
"max_count" is decremented to 0 due to sign removal do we need
to check the entire buffer.
Also replace "invalid_top" and "invalid_bot" with a map of
invalid ranges, further reducing the number of lines to be
checked.
Also improve tree traversal when counting the number of signs.
Instead of looping over the to be checked range and counting
the overlap for each row, keep track of the overlap in an
array and add this to the count.
Problem: Signcolumn width does not increase when ranged sign does not
start at sentinel line.
Solution: Handle paired range of added sign when checking signcols.
decor->text.str pointer must go. This removes it for conceal char,
in preparation for a larger PR which will also handle the sign case.
By actually allowing composing chars for a conceal chars, this
becomes a feature and not just a refactor, as a bonus.