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) <[email protected]>

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
 		}

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 {