Work out which file and line a row of a diff belongs to - #6026
Open
stefanhaller wants to merge 12 commits into
Open
stefanhaller wants to merge 12 commits into
stefanhaller wants to merge 12 commits into
Conversation
stefanhaller
added this pull request to stack #6027
September 19, 2026 19:19
stefanhaller
force-pushed
the
resolve-diff-lines-to-identities
branch
from
September 19, 2026 19:50
029da87 to
0416b76
Compare
stefanhaller
force-pushed
the
resolve-diff-lines-to-identities
branch
from
September 20, 2026 17:51
0416b76 to
1e08dcb
Compare
stefanhaller
force-pushed
the
resolve-diff-lines-to-identities
branch
from
September 21, 2026 12:44
1e08dcb to
8b9ca8e
Compare
stefanhaller
force-pushed
the
resolve-diff-lines-to-identities
branch
2 times, most recently
from
September 25, 2026 09:32
a51c847 to
de8f584
Compare
Base automatically changed from
render-diffs-without-a-pty-on-windows
to
master
September 25, 2026 10:14
The lines a selection covers are asked for by view line, which counts the segments a wrapping view breaks a line into, and then used to index the content, whose lines are unwrapped. The two agree only for a view whose content doesn't wrap. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
A test asks which lines of a view are selected; a wrapping view's cursor and range anchor answer in view lines, which count the segments each line is drawn as. Going through the segment-to-line mapping keeps the answer in the terms the question was asked in, and a line the selection covers several segments of is reported once. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The scroll offset, the cursor and a range's anchor are all view lines, which count the segments each line of the content is wrapped into. A change of width wraps the content differently, so every one of them ends up on a different line than the one it was put on. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Wrapping the content at another width moves every line of it to a different view line. The positions into the view are all view lines: the scroll offset, the cursor, a range's anchor. Each of them is then left pointing at a line it was never on. Committing the last of the staged changes widens the main view by half a screen, and that moves a selected hunk somewhere else entirely. Carry the positions through the lines of content they were on. A position always meant a line of content rather than a view line. The line the cursor is on keeps the row it was drawn on, so it stays in front of the user rather than the view scrolling under it; a view with no cursor on screen keeps its own place instead. A range's ends go on the outermost segments of their lines, since a range covers lines of content and not the segments those lines are drawn as. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Identifying a change line of a diff by its file line number needs both sides: two consecutive deletions sit at the same new-file position, so only their old-file line numbers tell them apart. LineNumberOfLine only answers for the new file, which leaves deletions ambiguous. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Parse is lenient: it takes any text and reads a diff out of it, which is what we want when we hand it a diff, but it has no way to say "this isn't one". We're about to parse the *rendered* contents of a diff view, which a diff renderer is free to restructure — putting the line numbers in a gutter, say, shifts the +/- marker off the start of each body line, so every line reads as context and the parse silently lies about which lines are changes. Comparing each hunk's body against the lengths its header declares catches exactly that, without teaching us anything about any particular renderer's layout: a faithful unified diff agrees with its headers, a restructured one doesn't. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Everything about a diff's content — which file and line a row belongs to, whether it's a change — is a property of the unwrapped buffer line, while the cursor, clicks and the range selection all speak in view lines, which count wrapped segments. Reading the content under the cursor therefore needs the mapping in both directions, and doing it outside gocui isn't possible: the wrapping is internal, and the caller couldn't take the view's lock across the lookup and the read. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ne low A view holds back the newline that ends a write until more content arrives, so that it doesn't end in an empty line. OverwriteLines moves the write cursor to the line it is given without letting go of that pending newline, so the write that follows advances first and lands on the line below. Nothing in lazygit overwrites lines right after such a write today. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
OverwriteLines is asked for a line and writes the one below it when the write before it ended in a newline. The view holds such a newline back until more content arrives, so that it doesn't end in an empty line, and OverwriteLines moved the write cursor without letting go of it, so the write that followed advanced to the next line first. Move the cursor through SetWritePos, which drops the pending newline along with it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A reader that parses a view's content rather than showing it wants the text the writer wrote, and the cells don't always spell it. A tab is expanded into the spaces it fills, so a line read back from the cells ends in one to four spaces where the writer put a tab. A carriage return moves the write cursor back to the start of the line, so the text written after it overwrites what came before. The parser of the main view's diff, which the next commit adds, meets the first case in every header of a file whose path contains a space. git terminates the path field of a "---" or "+++" line with a tab then, and a parser reading the cells takes the spaces the tab became for part of the path. That path names a file that doesn't exist, so the file's lines can't be acted on. Keep the text as written per line, from the first character on that the cells spell differently, so that a line without a tab or a carriage return costs nothing. LinesAsWritten hands it out the way BufferLines hands out the cells' text. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
To act on the line the user is pointing at in a diff view — to stage it, to open it in an editor, to keep the cursor on it while the diff is regenerated — we need to know which file it belongs to and where it sits in that file. Only the diff the view was rendered from knows that, and by the time it's on screen all we have is text. So parse it back: the view's contents are (usually) a unified diff, and running them through the patch parser recovers each row's file, kind and line numbers. "Usually" is why this goes behind a seam, and why it answers "I don't know" rather than guessing: a diff renderer is free to restructure what it prints, and a wrong answer here means acting on the wrong line of the wrong file. Parsing a whole buffer is a separate entry point from parsing a single line, because a caller resolving every row of a large diff must not re-parse a file's section once per line of it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
git doesn't just print the path: it terminates it with a tab when the path contains a space, and C-quotes the whole field when the path contains anything it won't print raw — with core.quotePath, which is on by default, that means any non-ASCII byte, so a file called café shows up as "b/caf\303\251". Taking the field verbatim therefore gives a path that doesn't exist, for a whole class of perfectly ordinary file names. The quoting is Go's own string syntax, octal escapes and all, so decoding it is a call to strconv.Unquote. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
stefanhaller
force-pushed
the
resolve-diff-lines-to-identities
branch
from
September 25, 2026 10:14
de8f584 to
82125cd
Compare
This branch has not been deployed
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
1 participant
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.
This is the first in a series of stacked PRs that implement staging directly in the main view.
To act on the line a user is pointing at in a diff — to stage it, to open it in an editor, to keep the cursor on it while the diff is generated again — we have to know which file it belongs to and where it sits in that file. By the time the diff is on screen, all we have is text. So this PR parses the view's contents back through the patch parser and recovers each row's file, kind and line numbers.
No new functionality here, this just lays the ground for future work by introducing data types and functions for getting information about a diff that is being shown in a view. In this PR we only support this for raw git diffs generated without a diff renderer; support for those will follow in the next PR.
A general note about the whole PR stack: unfortunately this is a lot more AI-generated with a lot less manual review than I would like. But applying the normal scrutiny to the code that I usually do would have taken me months to release this, and I don't want to delay this feature any longer than necessary.