From 6be2bd90311f265e0d6c1f327d42d67535ad37ae Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 7 Oct 2026 19:44:26 +0200 Subject: [PATCH] Resolve the files of the selected diff lines before acting on them The focused diff actions look up the file of a selected line in different places. In the files panel, staging and discarding do it per file while applying the patches, and editing a hunk does it for the first line. For a commit's diff, adding lines to a custom patch or discarding them from the commit does it only after the confirmation prompt and after a new patch has been started. The next commits report a line whose file can't be found as an error. For that, the lookup has to happen once, before we log an action, ask for confirmation or start a patch. So let diffLineSelection group the selected lines by their file, in the order the diff shows the files, and do the same for a commit's diff in selectionByFile. A line whose file can't be found is still skipped, as before. Co-Authored-By: Claude Opus 5.5 (1M context) --- pkg/gui/controllers/commit_diff_actions.go | 65 +++++++++++++------ .../controllers/working_tree_diff_actions.go | 65 ++++++++++--------- 2 files changed, 79 insertions(+), 51 deletions(-) diff --git a/pkg/gui/controllers/commit_diff_actions.go b/pkg/gui/controllers/commit_diff_actions.go index 7063deb9b..0f6bbd01d 100644 --- a/pkg/gui/controllers/commit_diff_actions.go +++ b/pkg/gui/controllers/commit_diff_actions.go @@ -104,6 +104,10 @@ func (self *CommitDiffActions) PrimaryAction(pane types.DiffPaneContext, firstBu if len(lines) == 0 { return nil } + selection := self.selectionByFile(lines) + if len(selection.paths) == 0 { + return nil + } patchBuilder := self.c.Git().Patch.PatchBuilder from, reverse := self.patchEndpoints(target) @@ -121,7 +125,7 @@ func (self *CommitDiffActions) PrimaryAction(pane types.DiffPaneContext, firstBu patchBuilder.Start(from, target.to, reverse, target.canRebase) } - if err := self.togglePatchLines(lines); err != nil { + if err := self.togglePatchLines(selection); err != nil { return err } // Taking the last line back out ends the patch rather than leaving an empty @@ -225,6 +229,10 @@ func (self *CommitDiffActions) DiscardSelection(pane types.DiffPaneContext, firs if commitIndex == -1 { return nil } + selection := self.selectionByFile(lines) + if len(selection.paths) == 0 { + return nil + } patchBuilder := self.c.Git().Patch.PatchBuilder prompt := lo.Ternary(patchBuilder.IsEmpty(), @@ -238,7 +246,7 @@ func (self *CommitDiffActions) DiscardSelection(pane types.DiffPaneContext, firs from, reverse := self.patchEndpoints(target) patchBuilder.Reset() patchBuilder.Start(from, target.to, reverse, target.canRebase) - if err := self.togglePatchLines(lines); err != nil { + if err := self.togglePatchLines(selection); err != nil { return err } if patchBuilder.IsEmpty() { @@ -318,38 +326,53 @@ func (self *CommitDiffActions) PatchInclusion() func(types.DiffLineInfo) bool { } } -// togglePatchLines takes the given lines of the commit's diff into the custom patch, or -// out of it. The first line of the selection decides which of the two happens, once for -// the whole selection: pointing at a line that is already in the patch takes the whole -// selection out of it, as toggling a selection of files in the commit files panel does. -func (self *CommitDiffActions) togglePatchLines(lines []types.DiffLineInfo) error { - patchBuilder := self.c.Git().Patch.PatchBuilder +// commitDiffSelection is the selected lines of a commit's diff, by the file they belong +// to: a patch is built a file at a time, while a selection can span several of them. +type commitDiffSelection struct { + // What the diff says about each of the files it covers. + files commitDiffFiles - // The files the selection covers, in the order the diff shows them, and per file the - // lines of it that are selected: a patch is built a file at a time, while a selection - // can span several of them. - paths := []string{} - linesByPath := map[string][]patch.LineIdentity{} + // The files the selection covers, in the order the diff shows them. + paths []string + + // Per file, the lines of it that are selected. + linesByPath map[string][]patch.LineIdentity +} + +// selectionByFile groups the given lines of the commit's diff by the file they belong +// to. A line whose path is no file of this repo is left out. +func (self *CommitDiffActions) selectionByFile(lines []types.DiffLineInfo) commitDiffSelection { + selection := commitDiffSelection{ + files: self.filesInDiff(), + linesByPath: map[string][]patch.LineIdentity{}, + } for _, line := range lines { path := self.patchBuilderPath(line.Path) if path == "" { continue } - if _, seen := linesByPath[path]; !seen { - paths = append(paths, path) + if _, seen := selection.linesByPath[path]; !seen { + selection.paths = append(selection.paths, path) } - linesByPath[path] = append(linesByPath[path], line.PatchLineIdentity()) - } - if len(paths) == 0 { - return nil + selection.linesByPath[path] = append(selection.linesByPath[path], line.PatchLineIdentity()) } + return selection +} + +// togglePatchLines takes the selected lines of the commit's diff into the custom patch, +// or out of it. The first line of the selection decides which of the two happens, once +// for the whole selection: pointing at a line that is already in the patch takes the +// whole selection out of it, as toggling a selection of files in the commit files panel +// does. +func (self *CommitDiffActions) togglePatchLines(selection commitDiffSelection) error { + patchBuilder := self.c.Git().Patch.PatchBuilder + files, paths := selection.files, selection.paths - files := self.filesInDiff() indicesByPath := map[string][]int{} wholeFileByPath := map[string]bool{} for _, path := range paths { indices, everyChange, err := patchBuilder.PatchLineIndicesForLines( - path, files.previousPath(path), linesByPath[path]) + path, files.previousPath(path), selection.linesByPath[path]) if err != nil { return err } diff --git a/pkg/gui/controllers/working_tree_diff_actions.go b/pkg/gui/controllers/working_tree_diff_actions.go index b9ff5124a..a55e93af1 100644 --- a/pkg/gui/controllers/working_tree_diff_actions.go +++ b/pkg/gui/controllers/working_tree_diff_actions.go @@ -59,14 +59,14 @@ func (self *WorkingTreeDiffActions) PrimaryAction(pane types.DiffPaneContext, fi self.c.UserConfig().Keybinding.Universal.IncreaseContextInDiffView) } - infos, onStagedSide, ok := self.diffLineSelection(pane, firstBufferLine, lastBufferLine) - if !ok { + linesByFile, onStagedSide := self.diffLineSelection(pane, firstBufferLine, lastBufferLine) + if len(linesByFile) == 0 { return nil } // Either way the patch goes to the index: forwards from the unstaged side to stage // it, backwards from the staged side to take it back out. - return self.applyDiffLineSelection(pane, firstBufferLine, infos, onStagedSide, + return self.applyDiffLineSelection(pane, firstBufferLine, linesByFile, onStagedSide, git_commands.ApplyPatchOpts{Reverse: onStagedSide, Cached: true}) } @@ -79,8 +79,8 @@ func (self *WorkingTreeDiffActions) DiscardSelection(pane types.DiffPaneContext, self.c.UserConfig().Keybinding.Universal.IncreaseContextInDiffView) } - infos, onStagedSide, ok := self.diffLineSelection(pane, firstBufferLine, lastBufferLine) - if !ok { + linesByFile, onStagedSide := self.diffLineSelection(pane, firstBufferLine, lastBufferLine) + if len(linesByFile) == 0 { return nil } @@ -93,7 +93,7 @@ func (self *WorkingTreeDiffActions) DiscardSelection(pane types.DiffPaneContext, Title: self.c.Tr.DiscardChangeTitle, Prompt: self.c.Tr.DiscardChangePrompt, HandleConfirm: func() error { - return self.applyDiffLineSelection(pane, firstBufferLine, infos, onStagedSide, + return self.applyDiffLineSelection(pane, firstBufferLine, linesByFile, onStagedSide, git_commands.ApplyPatchOpts{Reverse: true, Cached: onStagedSide}) }, }) @@ -117,18 +117,15 @@ func (self *WorkingTreeDiffActions) DiscardSelectionDisabledReason(types.DiffPan func (self *WorkingTreeDiffActions) EditHunk( pane types.DiffPaneContext, firstBufferLine int, lastBufferLine int, ) error { - infos, onStagedSide, ok := self.diffLineSelection(pane, firstBufferLine, lastBufferLine) - if !ok { - return nil - } - file := self.fileForDiffLinePath(infos[0].Path) - if file == nil { + linesByFile, onStagedSide := self.diffLineSelection(pane, firstBufferLine, lastBufferLine) + if len(linesByFile) == 0 { return nil } + file := linesByFile[0].file parsedPatch := patch.Parse(self.c.Git().WorkingTree.WorktreeFileDiff(file, git_commands.DiffModePlain, onStagedSide)) lineIndices := patch.ChangeLineIndicesForLines(parsedPatch, - []patch.LineIdentity{infos[0].PatchLineIdentity()}) + []patch.LineIdentity{linesByFile[0].lines[0].PatchLineIdentity()}) if len(lineIndices) == 0 { return nil } @@ -193,19 +190,34 @@ func (self *WorkingTreeDiffActions) PatchInclusion() func(types.DiffLineInfo) bo return nil } +// fileDiffLines is the selected change lines of one file of the diff. +type fileDiffLines struct { + file *models.File + lines []types.DiffLineInfo +} + // diffLineSelection resolves what the user has selected in a pane of the focused main // view to the change lines to act on, and reports whether they are the staged side of // the diff — which is a question about the pane, so it is the same for every file of a -// directory's diff. ok is false when the selection holds no change line, in which case +// directory's diff. When the selection holds no change line, it returns no file, and // there is nothing to act on. +// +// A directory's diff spans several files, and a patch is of one file, so the lines are +// grouped by the file they belong to, in the order the diff shows the files. func (self *WorkingTreeDiffActions) diffLineSelection( pane types.DiffPaneContext, firstBufferLine int, lastBufferLine int, -) (infos []types.DiffLineInfo, onStagedSide bool, ok bool) { - infos = self.c.Helpers().DiffLine.ChangeLinesInBufferRange(pane.GetView(), firstBufferLine, lastBufferLine) - if len(infos) == 0 { - return nil, false, false +) (linesByFile []fileDiffLines, onStagedSide bool) { + infos := self.c.Helpers().DiffLine.ChangeLinesInBufferRange(pane.GetView(), firstBufferLine, lastBufferLine) + infosByPath := lo.GroupBy(infos, func(info types.DiffLineInfo) string { return info.Path }) + paths := lo.Uniq(lo.Map(infos, func(info types.DiffLineInfo, _ int) string { return info.Path })) + for _, path := range paths { + file := self.fileForDiffLinePath(path) + if file == nil { + continue + } + linesByFile = append(linesByFile, fileDiffLines{file: file, lines: infosByPath[path]}) } - return infos, self.showsStagedSide(pane), true + return linesByFile, self.showsStagedSide(pane) } // applyDiffLineSelection applies the selected change lines, a patch per file, and @@ -215,25 +227,18 @@ func (self *WorkingTreeDiffActions) diffLineSelection( // from once the diff has changed under it. func (self *WorkingTreeDiffActions) applyDiffLineSelection( pane types.DiffPaneContext, firstBufferLine int, - infos []types.DiffLineInfo, onStagedSide bool, opts git_commands.ApplyPatchOpts, + linesByFile []fileDiffLines, onStagedSide bool, opts git_commands.ApplyPatchOpts, ) error { self.c.LogAction(self.c.Tr.Actions.ApplyPatch) - // A directory's diff spans several files, and a patch is of one file, so the - // selected lines are grouped by the file they belong to and applied file by file. - infosByFile := lo.GroupBy(infos, func(info types.DiffLineInfo) string { return info.Path }) acted := set.New[string]() actedSideRemains := false - for path, fileInfos := range infosByFile { - file := self.fileForDiffLinePath(path) - if file == nil { - continue - } - changesLeft, err := self.applyDiffLines(file, fileInfos, onStagedSide, opts) + for _, fileLines := range linesByFile { + changesLeft, err := self.applyDiffLines(fileLines.file, fileLines.lines, onStagedSide, opts) if err != nil { return err } - acted.Add(file.GetPath()) + acted.Add(fileLines.file.GetPath()) actedSideRemains = actedSideRemains || changesLeft } if !actedSideRemains {