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,