fix(parser): keep gutter one segment per rune when padding blank graph lines - #722
Merged
Merged
Conversation
…h lines A row whose graph column contains a blank line (e.g. the empty "│" line under a commit in `builtin_log_comfortable`) has its gutter padded to the row's indent by re-fattening the last gutter segment's text with spaces. That breaks the one-rune-per-segment invariant `row.go` relies on: `Row.Extend()` builds its mask from the segment count, so a padded gutter that reports 1 segment instead of 3 loses the trailing columns. Parsing `test/single-line-with-description.log` and extending its row: actual "│" (1 segment) expected "│ " (3 segments, row.Indent == 3) The missing columns shift every embedded operation panel (details file list, inline describe, evolog, rebase preview) two columns left of the row's own text under a comfortable template. Pad by appending single-space segments carrying the last gutter segment's style, one per remaining indent column, preserving the invariant. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
baggiiiie
approved these changes
Aug 11, 2026
baggiiiie
left a comment
baggiiiie
left a comment
Collaborator
There was a problem hiding this comment.
lgtm, thanks!
btw for such UI fixes, it'd be better to include screenshots to show what the issue was and the fixed UI
Collaborator
|
merging this, repro and results: beforebefore.mp4afterafter.mp4 |
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
2 participants
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.
Parsing the shipped fixture
test/testdata/single-line-with-description.log(a commit followed by a blank│graph line, asbuiltin_log_comfortableemits) and extending the row drops two gutter columns:Downstream, that shifts every embedded operation panel (details file list, inline describe, evolog, rebase preview) two columns to the left of the row's own text under a comfortable template.
Cause
GraphRowLine.chopfirst splits the gutter into one segment per rune, then pads short lines up to the row indent. The padding step re-fattened the last segment's.Textwithstrings.Repeat(" ", indent), undoing the per-rune split.Row.Extend()derives its mask from the gutter's segment count, so a blank line reporting 1 segment instead of 3 loses the trailing columns.Fix
Pad by appending one single-space
screen.Segmentper remaining indent column, each carrying the last gutter segment'sStyle, so the one-rune-per-segment invariant holds.Test
TestParser_Parse_ExtendBlankGraphLineintest/log_parser_test.goextends the fixture row and asserts the gutter is"│ ". I verified it fails without the source change (actual "│"vsexpected "│ ") and passes with it.go test ./...is green.