Fix gutter sign on visual-line continuation rows#248
Open
alberti42 wants to merge 3 commits into
Open
Conversation
The loop bound was computed as (point) after forward-line, which lands at the beginning of end-line. When stepping by visual lines with next-visual-line, a single wrapped hunk line would cause the loop to exit after one iteration: the first visual-line step moves point past bol of end-line but still within the same logical line, so the bound check fails and subsequent visual rows are never collected. Using line-end-position ensures the bound covers the full extent of end-line regardless of how the stepping function advances point.
Changed, added, and deleted signs share the same buffer positions as separator and unchanged overlays. Without an explicit priority, overlay display order is undefined, so a separator or unchanged overlay could render on top of a hunk sign. Setting priority 10 on any sign whose text contains a non-whitespace character guarantees hunk signs win when overlays overlap.
- git-gutter:next-visual-line removed — upstream version used next-line with line-move-visual, ours used vertical-motion. Both are replaced by wrap-prefix. - move-fn selection removed from view-for-unchanged and view-set-overlays — always forward-line now. - git-gutter:wrap-prefix-for-sign new function. - put-signs: overlay spans to eol + wrap-prefix set (TTY + visual-line only). - put-signs-linum: same wrap-prefix treatment for parity.
3c919a7 to
c5839dc
Compare
Author
|
Dear Jen-Chieh (@jcs090218), Just a quick check-in on this PR. I wanted to make sure the notification reached you. No urgency at all. The PR solves a real bug but is not critical. It can wait whenever you have some free time and interest to look into it. Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix gutter sign on visual-line continuation rows
Background: visual lines and soft-wrapping
Emacs distinguishes buffer lines (separated by newline characters) from
visual lines (screen rows). When soft-wrapping is active — because text
exceeds the window width, or because a right margin has been set to constrain
line length for readability — a single buffer line is displayed across multiple
visual rows. The extra rows are called continuation rows.
git-gutter:visual-linedoes not enable soft-wrapping; it tells git-gutterthat soft-wrapping is already in use and that gutter indicators should appear
on every visual row of a hunk line, not just the first.
Many modern themes give the left-margin column a distinct background color to
visually separate it from the buffer text — for example the built-in
modus-operanditheme (see https://protesilaos.com/emacs/modus-themes-pictures).This makes a consistent gutter background across all rows, including
continuation rows, visually important.
Problem
With
git-gutter:visual-line t, when a buffer line wraps into multiple visualrows, the gutter sign (
▐or any other indicator) appears only on the firstvisual row. On every continuation row the left margin falls back to the buffer
background instead of the gutter background, producing a color-mismatched
stripe wherever soft-wrap occurs. The problem affects both TTY and GUI frames.
Root cause
Git-gutter renders signs by attaching a
before-stringto a zero-lengthoverlay at the start of each buffer line. A zero-length overlay fires at
exactly one buffer position; it has no mechanism to inject content on
continuation rows produced by the display engine for the same logical line.
The previous approach tried to work around this by enumerating visual row
starts explicitly, using
next-line(upstream) orvertical-motion(earlyattempts in this branch). Both are unreliable:
vertical-motionlands at the last character of the current screen row, notat the start of the next one, so the computed position is off by one.
visual-wrap-prefix-mode(see below) inserts continuation indentation viawrap-prefixtext properties, which shifts where visual rows begin in a waythat
vertical-motiondoes not account for. The result is two overlays onthe same screen row and a gap on the next.
There is no Lisp-accessible hook that fires once per screen row during
redisplay the way
display-line-numbersworks internally inxdisp.c; anyLisp enumeration of visual row positions is inherently a heuristic.
The clean fix:
wrap-prefixInvestigating how
display-line-numbershandles continuation rows inxdisp.cwas the key insight. Line numbers appear correctly on every visualrow because the C display loop runs
maybe_produce_line_numberonce per screenrow with direct access to row geometry. That path is not available from Lisp.
However, the same investigation revealed that
wrap-prefix— the overlayproperty that
visual-wrap-prefix-modeuses to repeat continuationindentation — is driven by exactly the same display-loop mechanism and is
accessible from Lisp.
Glossary for context:
before-string: an overlay property whose text is prepended at the overlaystart position, before the visible buffer text. It applies only to the first
visual row of the logical line. Used by git-gutter to place a sign in the
left margin on that row.
wrap-prefix: an overlay (or text) property whose text is prepended to everycontinuation row of the line the overlay spans, before the visible buffer
text begins on that row. It is explicitly skipped on the first visual row,
making it complementary to
before-string. Used byvisual-wrap-prefix-modeto repeat indentation on wrapped lines.
visual-wrap-prefix-mode: built into Emacs 30+ (formerly the externaladaptive-wrappackage), it makes soft-wrapped lines look indented to thelevel of their content, by setting
wrap-prefixon each line. Seehttps://emacsredux.com/blog/2026/03/01/soft-wrapping-done-right-with-visual-wrap-prefix-mode/
The fix: when
git-gutter:visual-lineis non-nil, one overlay is createdper buffer line in the hunk, spanning from
posto(line-end-position)instead of being zero-length.
before-stringplaces the sign on the firstvisual row as before; since
wrap-prefixis skipped on the first visual row,both properties are needed and cover complementary, non-overlapping sets of
rows.
wrap-prefixis set to the same margin string so the display enginerepeats it automatically on every continuation row of that buffer line. This
replaces the previous approach of one overlay per visual row: instead of
attempting to enumerate visual row positions from Lisp — which is unreliable —
we delegate continuation row rendering entirely to the C display loop, which
is the only place that has precise, authoritative knowledge of screen row
geometry.
When a
wrap-prefixtext property already exists atpos(e.g. fromvisual-wrap-prefix-mode), the gutter sign is prepended to it so thatcontinuation indentation is preserved on wrapped rows. Emacs does not provide
a left-margin compositor that would automatically combine contributions from
multiple sources (see https://www.reddit.com/r/emacs/comments/1rpclmq/), so
this manual prepending is currently the only way to ensure coexistence with
other packages that set
wrap-prefix.The spanning overlay and
wrap-prefixare applied whenevergit-gutter:visual-lineis non-nil, in both TTY and GUI frames. When
git-gutter:visual-lineis nilthe overlay remains zero-length and
wrap-prefixis not set, preserving theexisting behavior.
Related fixes included in this branch
view-set-overlays: useline-end-positionas end positionview-set-overlaysloops over the buffer lines of a hunk to collect thepositions where signs should be placed. The position used to stop the loop was
(point)captured after stepping to the last line of the hunk with the built-inEmacs function
forward-line, which places point at the beginning of the linefollowing the hunk. When stepping by visual lines, a single wrapped hunk line
caused the loop to exit after one iteration: the first visual-line step moves
point past
bolof that following line, but the logical line is still within thehunk, so the stop condition was met too early and sign positions on subsequent
visual rows were never collected. Using
line-end-positionensures the loopcovers the full extent of the last hunk line regardless of how point advances.
With the main fix in place, both
view-set-overlaysandview-for-unchangednow always step using the built-in Emacs function
forward-line, which advancesby logical (buffer) lines — one newline at a time — rather than by visual rows.
Priority 10 on non-whitespace signs
Changed, added, and deleted signs share the same buffer positions as separator
and unchanged overlays. Without an explicit priority, overlay display order is
undefined, so a separator or unchanged overlay can render on top of a hunk
sign. Setting
priority 10on any sign whose text contains a non-whitespacecharacter ensures hunk signs win when overlays overlap at the same position.
Documentation
If this PR is merged I am happy to update the manual accordingly.
Commits