From 56e234e94c7e6ac0e4ef6ffae5d4475c9f922990 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 7 Oct 2026 19:46:21 +0200 Subject: [PATCH] Show an error if the selected diff lines can't be found in the diff When we stage, discard or edit lines of the working tree's diff, we read the file's diff again and look for the selected lines in it. If none of them are there, nothing happens. The same goes for adding lines of a commit's diff to a custom patch, or taking lines out of it. This can happen if the file changed since its diff was shown, or if we read the line numbers out of the diff on screen wrongly. Show an error that names the file instead. As with a file that can't be found, don't act on any file if the lines of one are missing. Unlike the file check, this one happens after a new custom patch has been started. If the error leaves that patch without any lines, reset it, as we do when the last line is taken out of a patch. Co-Authored-By: Claude Opus 5.5 (1M context) --- pkg/gui/controllers/commit_diff_actions.go | 21 +++++--- pkg/gui/controllers/diff_paths.go | 7 +++ .../controllers/working_tree_diff_actions.go | 34 ++++++++----- pkg/i18n/english.go | 2 + ...eport_lines_missing_from_a_commits_diff.go | 51 +++++++++++++++++++ ...lines_missing_from_a_file_changed_since.go | 47 +++++++++++++++++ pkg/integration/tests/test_list.go | 2 + 7 files changed, 144 insertions(+), 20 deletions(-) create mode 100644 pkg/integration/tests/main_view/report_lines_missing_from_a_commits_diff.go create mode 100644 pkg/integration/tests/main_view/report_lines_missing_from_a_file_changed_since.go diff --git a/pkg/gui/controllers/commit_diff_actions.go b/pkg/gui/controllers/commit_diff_actions.go index 54b88eb84..a68a55d94 100644 --- a/pkg/gui/controllers/commit_diff_actions.go +++ b/pkg/gui/controllers/commit_diff_actions.go @@ -125,14 +125,16 @@ func (self *CommitDiffActions) PrimaryAction(pane types.DiffPaneContext, firstBu patchBuilder.Start(from, target.to, reverse, target.canRebase) } - if err := self.togglePatchLines(selection); err != nil { - return err - } + err := self.togglePatchLines(selection) // Taking the last line back out ends the patch rather than leaving an empty // one, so that the pane previewing it and the marks over the diff go with it. + // The same goes for a patch just started that no line could be put into. if patchBuilder.IsEmpty() { patchBuilder.Reset() } + if err != nil { + return err + } // The diff on screen is the one the marks belong to, so they can be brought up // to date at once rather than waiting for the render below. @@ -189,7 +191,7 @@ func (self *CommitDiffActions) removePatchLines( } } if len(indices) == 0 { - continue + return diffLinesNotFoundError(self.c, filename) } indicesByFilename[filename] = indices } @@ -254,6 +256,9 @@ func (self *CommitDiffActions) DiscardSelection(pane types.DiffPaneContext, firs patchBuilder.Reset() patchBuilder.Start(from, target.to, reverse, target.canRebase) if err := self.togglePatchLines(selection); err != nil { + // This patch exists only to be removed from the commit, so don't leave it + // behind. + patchBuilder.Reset() return err } if patchBuilder.IsEmpty() { @@ -385,6 +390,9 @@ func (self *CommitDiffActions) togglePatchLines(selection commitDiffSelection) e if err != nil { return err } + if len(indices) == 0 { + return diffLinesNotFoundError(self.c, path) + } indicesByPath[path] = indices // Selecting every change of a file the commit adds or deletes is selecting the @@ -397,12 +405,9 @@ func (self *CommitDiffActions) togglePatchLines(selection commitDiffSelection) e if err != nil { return err } - removing := len(indicesByPath[paths[0]]) > 0 && lo.Contains(included, indicesByPath[paths[0]][0]) + removing := lo.Contains(included, indicesByPath[paths[0]][0]) for _, path := range paths { - if len(indicesByPath[path]) == 0 { - continue - } previousPath := files.previousPath(path) var err error switch { diff --git a/pkg/gui/controllers/diff_paths.go b/pkg/gui/controllers/diff_paths.go index 017286c2d..3f18c5628 100644 --- a/pkg/gui/controllers/diff_paths.go +++ b/pkg/gui/controllers/diff_paths.go @@ -146,6 +146,13 @@ func diffLinesFileNotFoundError(c *ControllerCommon, absolutePath string) error map[string]string{"path": path})) } +// diffLinesNotFoundError reports that selected diff lines can't be found in the diff of +// the file at the given path in the repo. +func diffLinesNotFoundError(c *ControllerCommon, path string) error { + return errors.New(utils.ResolvePlaceholderString(c.Tr.DiffLinesNotFound, + map[string]string{"path": path})) +} + // repoRelativePath turns the absolute path a diff line carries into the one git speaks // of the file: relative to the worktree, with forward slashes. It is "" for a path that // is no file of this repo, which a diff renderer's own naming of a line can produce. diff --git a/pkg/gui/controllers/working_tree_diff_actions.go b/pkg/gui/controllers/working_tree_diff_actions.go index f4c0f6334..6739d80a2 100644 --- a/pkg/gui/controllers/working_tree_diff_actions.go +++ b/pkg/gui/controllers/working_tree_diff_actions.go @@ -136,7 +136,7 @@ func (self *WorkingTreeDiffActions) EditHunk( lineIndices := patch.ChangeLineIndicesForLines(parsedPatch, []patch.LineIdentity{linesByFile[0].lines[0].PatchLineIdentity()}) if len(lineIndices) == 0 { - return nil + return diffLinesNotFoundError(self.c, file.GetPath()) } hunkIdx := parsedPatch.HunkContainingLine(lineIndices[0]) @@ -240,9 +240,16 @@ func (self *WorkingTreeDiffActions) applyDiffLineSelection( pane types.DiffPaneContext, firstBufferLine int, linesByFile []fileDiffLines, onStagedSide bool, opts git_commands.ApplyPatchOpts, ) error { - patches := lo.Map(linesByFile, func(fileLines fileDiffLines, _ int) diffLinesPatch { - return self.buildDiffLinesPatch(fileLines.file, fileLines.lines, onStagedSide, opts) - }) + // If the lines of one file can't be found, no file is acted on, so every file's + // patch is built before any is applied. + patches := make([]diffLinesPatch, 0, len(linesByFile)) + for _, fileLines := range linesByFile { + p, err := self.buildDiffLinesPatch(fileLines.file, fileLines.lines, onStagedSide, opts) + if err != nil { + return err + } + patches = append(patches, p) + } self.c.LogAction(self.c.Tr.Actions.ApplyPatch) @@ -295,8 +302,7 @@ type diffLinesPatch struct { // acting on the file itself, which is staged or unstaged as a whole. wholeFile bool - // The patch of the lines, where they are not the whole file. Empty when none of - // them could be found in the file's diff. + // The patch of the lines, where they are not the whole file. patch string // Whether the diff the lines were found in holds changes they don't cover. @@ -322,15 +328,22 @@ type diffLinesPatch struct { // It records whether the diff it read holds changes the selection didn't cover. The // caller uses this to tell whether the side acted on still has anything of this file // in it once we are done. +// +// It fails if none of the lines are in the diff. The diff on screen is then not the +// one it read; either the file changed since the diff was shown, or we read the +// lines wrongly. func (self *WorkingTreeDiffActions) buildDiffLinesPatch( file *models.File, infos []types.DiffLineInfo, sourceCached bool, opts git_commands.ApplyPatchOpts, -) diffLinesPatch { +) (diffLinesPatch, error) { parsedPatch := patch.Parse(self.c.Git().WorkingTree.WorktreeFileDiff(file, git_commands.DiffModePlain, sourceCached)) patchLineIndices := patch.ChangeLineIndicesForLines(parsedPatch, lo.Map(infos, func(info types.DiffLineInfo, _ int) patch.LineIdentity { return info.PatchLineIdentity() })) + if len(patchLineIndices) == 0 { + return diffLinesPatch{}, diffLinesNotFoundError(self.c, file.GetPath()) + } changesLeft := len(patchLineIndices) < changeLineCount(parsedPatch) @@ -341,7 +354,7 @@ func (self *WorkingTreeDiffActions) buildDiffLinesPatch( // and taking that back out leaves an empty file in the index rather than an // untracked one. if !changesLeft && opts.Cached { - return diffLinesPatch{file: file, wholeFile: true} + return diffLinesPatch{file: file, wholeFile: true}, nil } patchToApply := parsedPatch. @@ -351,7 +364,7 @@ func (self *WorkingTreeDiffActions) buildDiffLinesPatch( FileNameOverride: file.GetPath(), }). FormatPlain() - return diffLinesPatch{file: file, patch: patchToApply, changesLeft: changesLeft} + return diffLinesPatch{file: file, patch: patchToApply, changesLeft: changesLeft}, nil } // applyDiffLinesPatch applies what buildDiffLinesPatch built, as opts says. @@ -362,9 +375,6 @@ func (self *WorkingTreeDiffActions) applyDiffLinesPatch(p diffLinesPatch, opts g } return self.c.Git().WorkingTree.StageFile(p.file.GetPath()) } - if p.patch == "" { - return nil - } return self.c.Git().Patch.ApplyPatch(p.patch, opts) } diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index 522f9cbbb..e2179d5a0 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -749,6 +749,7 @@ type TranslationSet struct { SelectedDiffLinesCopiedToast string SelectionNotFoundInDiffToast string DiffLinesFileNotFound string + DiffLinesNotFound string NoFilesStagedTitle string NoFilesStagedPrompt string BranchNotFoundTitle string @@ -1940,6 +1941,7 @@ func EnglishTranslationSet() *TranslationSet { SelectedDiffLinesCopiedToast: "Selected diff lines copied to clipboard", SelectionNotFoundInDiffToast: "Nothing in the selection could be found in the diff", DiffLinesFileNotFound: "Can't find the file '{{.path}}' that the selected lines belong to", + DiffLinesNotFound: "Can't find the selected lines in the diff of '{{.path}}'", CommitPrefixPatternError: "Error in commitPrefix pattern", NoFilesStagedTitle: "No files staged", NoFilesStagedPrompt: "You have not staged any files. Commit all files?", diff --git a/pkg/integration/tests/main_view/report_lines_missing_from_a_commits_diff.go b/pkg/integration/tests/main_view/report_lines_missing_from_a_commits_diff.go new file mode 100644 index 000000000..e61aab103 --- /dev/null +++ b/pkg/integration/tests/main_view/report_lines_missing_from_a_commits_diff.go @@ -0,0 +1,51 @@ +package main_view + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var ReportLinesMissingFromACommitsDiff = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Adding lines to a custom patch that the diff puts where the commit's diff has none shows an error", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(cfg *config.AppConfig) { + cfg.GetUserConfig().Gui.UseHunkModeInDiffView = false + // A renderer that announces the metadata protocol and passes the diff through, + // except that its hunk header puts the hunk ten lines further down. + cfg.GetUserConfig().Git.DiffRenderers = []config.DiffRendererConfig{ + {Command: `printf '\033]1717;1\007'; sed 's#@@ -1,3 +1,4 @@#@@ -11,3 +11,4 @@#'`}, + } + }, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file1", "one\ntwo\nthree\n") + shell.Commit("one") + + shell.UpdateFileAndAdd("file1", "one\nADD1\ntwo\nthree\n") + shell.Commit("two") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("two").IsSelected(), + Contains("one"), + ). + Press(keys.Universal.FocusMainView) + + t.Views().Main(). + IsFocused(). + SelectedLines( + Contains("+ADD1"), + ). + PressPrimaryAction() + + t.ExpectPopup().Alert(). + Title(Equals("Error")). + Content(Equals("Can't find the selected lines in the diff of 'file1'")). + Confirm() + + // No patch is left behind. + t.Views().Information().Content(DoesNotContain("Building patch")) + }, +}) diff --git a/pkg/integration/tests/main_view/report_lines_missing_from_a_file_changed_since.go b/pkg/integration/tests/main_view/report_lines_missing_from_a_file_changed_since.go new file mode 100644 index 000000000..b10618189 --- /dev/null +++ b/pkg/integration/tests/main_view/report_lines_missing_from_a_file_changed_since.go @@ -0,0 +1,47 @@ +package main_view + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var ReportLinesMissingFromAFileChangedSince = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Staging lines that are gone from the file since its diff was shown shows an error", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(cfg *config.AppConfig) { + cfg.GetUserConfig().Gui.UseHunkModeInDiffView = false + }, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file1", "one\ntwo\nthree\n") + shell.Commit("one") + + shell.UpdateFile("file1", "one\nADD1\ntwo\nthree\n") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Files(). + IsFocused(). + Lines( + Contains(" M file1").IsSelected(), + ). + Press(keys.Universal.FocusMainView) + + t.Views().Main(). + IsFocused(). + SelectedLines( + Contains("+ADD1"), + ) + + // The change is undone outside of lazygit, and nothing has refreshed the diff + // since. + t.Shell().UpdateFile("file1", "one\ntwo\nthree\n") + + t.Views().Main(). + PressPrimaryAction() + + t.ExpectPopup().Alert(). + Title(Equals("Error")). + Content(Equals("Can't find the selected lines in the diff of 'file1'")). + Confirm() + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 095a44e97..566baf0dc 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -463,6 +463,8 @@ var tests = []*components.IntegrationTest{ main_view.RemovePartOfAddedFileFromCommit, main_view.RemovePatchFromCommit, main_view.RenderTheDiffBesideThePatchMarks, + main_view.ReportLinesMissingFromACommitsDiff, + main_view.ReportLinesMissingFromAFileChangedSince, main_view.ReportLinesOfAFileMissingFromTheCommitsDiff, main_view.ReportLinesOfAFileMissingFromTheFilesPanel, main_view.ResetAPatchBuiltFromACommitsDiff,