Right-fill follows the painted background, not minus/plus-style#2164
Closed
igrmk wants to merge 1 commit into
Closed
Right-fill follows the painted background, not minus/plus-style#2164igrmk wants to merge 1 commit into
igrmk wants to merge 1 commit into
Conversation
The side-by-side fill for a standalone minus/plus line used minus/plus-style, ignoring a map-styles repaint of the content and leaving a mismatched trailing stripe. Take it from the line's last painted style instead.
Author
|
Superseded by #2166 |
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.
Right-fill follows the painted background, not minus/plus-style
Problem
In side-by-side mode, when a removed (or added) line's content is repainted to a
non-default background — e.g. git's moved-line color remapped through
map-styles— the panel's trailing background fill was still painted withminus-style(resp.plus-style). The code itself was colored correctly, butthe empty space after it carried the wrong color, leaving a stripe.
The shots below render a block git reports as moved, recolored by
map-stylesto a gray background. The repro is self-contained — git's own config is
neutralized, color-moved is turned on explicitly, and delta is driven entirely by
flags — so it doesn't depend on whoever runs it (
4b56fde1is the commit thatrelocated the release-verification block in this repo):
The trailing
sedcrops the output to the relocated block (the left, removedpanel).
Before
The content is gray (the
map-stylesremap), but each line's trailing fill runsout in pink
minus-style— a mismatched stripe after every line.After
Content and fill share the painted background.
Cause
Painter::get_should_right_fill_background_color_and_fill_stylechose the fillfor a standalone minus/plus line — the
HunkMinus(_, None)/HunkPlus(_, None)branches — as
config.minus_style/config.plus_styleunconditionally. Thatignores any
map-styles(or color-moved) remapparse_style_sectionsalreadyapplied to the content, so the fill and the content disagree.
Fix
Derive the fill from the last painted, non-newline style section of the line —
the same "last real style" the color-moved branch already keys on — and fall
back to minus/plus-style only when there is none. The fill then follows whatever
the content was actually painted (
src/paint.rs).Test
fill_style_tests::test_right_fill_follows_painted_content_for_standalone_minusdrives the fill function directly: a standalone removed line whose content is
painted to a non-minus background must yield a fill matching that background, not
minus_style. It fails on the old code and passes on the new.Known limitation (separate issue)
A fully-blank moved line still renders in
minus-stylerather than the movecolor. This is not a fill mismatch — the fill faithfully follows the line's
content — but a parsing-level quirk: git puts the move color only on the
-marker, which delta strips, so the blank line carries no content color left to
map. Out of scope for this change.