diff --git a/pkg/gui/controllers/commit_diff_actions.go b/pkg/gui/controllers/commit_diff_actions.go index dc6eb825a..54b88eb84 100644 --- a/pkg/gui/controllers/commit_diff_actions.go +++ b/pkg/gui/controllers/commit_diff_actions.go @@ -108,9 +108,6 @@ func (self *CommitDiffActions) PrimaryAction(pane types.DiffPaneContext, firstBu if err != nil { return err } - if len(selection.paths) == 0 { - return nil - } patchBuilder := self.c.Git().Patch.PatchBuilder from, reverse := self.patchEndpoints(target) @@ -182,7 +179,7 @@ func (self *CommitDiffActions) removePatchLines( for path, ordinals := range self.c.Helpers().DiffLine.ChangeLineOrdinals(self.customPatchDiff(), lines) { filename := self.patchBuilderPath(path) if filename == "" { - continue + return diffLinesFileNotFoundError(self.c, path) } included := patchBuilder.IncludedChangeLineIndices(filename) indices := []int{} @@ -243,9 +240,6 @@ func (self *CommitDiffActions) DiscardSelection(pane types.DiffPaneContext, firs if err != nil { return err } - if len(selection.paths) == 0 { - return nil - } patchBuilder := self.c.Git().Patch.PatchBuilder prompt := lo.Ternary(patchBuilder.IsEmpty(), @@ -353,7 +347,8 @@ type commitDiffSelection struct { } // 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. +// to. If a line belongs to no file of the diff, the diff on screen and the one the +// patch is built from disagree, so this fails and we build nothing. func (self *CommitDiffActions) selectionByFile(lines []types.DiffLineInfo) (commitDiffSelection, error) { files, err := self.filesInDiff() if err != nil { @@ -362,8 +357,8 @@ func (self *CommitDiffActions) selectionByFile(lines []types.DiffLineInfo) (comm selection := commitDiffSelection{files: files, linesByPath: map[string][]patch.LineIdentity{}} for _, line := range lines { path := self.patchBuilderPath(line.Path) - if path == "" { - continue + if _, ok := files[path]; !ok { + return commitDiffSelection{}, diffLinesFileNotFoundError(self.c, line.Path) } if _, seen := selection.linesByPath[path]; !seen { selection.paths = append(selection.paths, path) diff --git a/pkg/gui/controllers/diff_paths.go b/pkg/gui/controllers/diff_paths.go index 91215ba2c..017286c2d 100644 --- a/pkg/gui/controllers/diff_paths.go +++ b/pkg/gui/controllers/diff_paths.go @@ -1,11 +1,13 @@ package controllers import ( + "errors" "path" "path/filepath" "strings" "github.com/jesseduffield/lazygit/pkg/gui/filetree" + "github.com/jesseduffield/lazygit/pkg/utils" "github.com/samber/lo" ) @@ -132,6 +134,18 @@ func isInDir(path string, dir string) bool { return dir == "." || strings.HasPrefix(path, dir+"/") } +// diffLinesFileNotFoundError reports that the file selected diff lines belong to can't +// be found. It names the file by its path in the repo, or by the path the diff line +// carries for one outside of it. +func diffLinesFileNotFoundError(c *ControllerCommon, absolutePath string) error { + path := repoRelativePath(c.Git().RepoPaths.WorktreePath(), absolutePath) + if path == "" { + path = absolutePath + } + return errors.New(utils.ResolvePlaceholderString(c.Tr.DiffLinesFileNotFound, + 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/main_view_controller.go b/pkg/gui/controllers/main_view_controller.go index 71aee5888..7129e1c43 100644 --- a/pkg/gui/controllers/main_view_controller.go +++ b/pkg/gui/controllers/main_view_controller.go @@ -1105,7 +1105,7 @@ func (self *MainViewController) openPullRequestAtSelectedLine() error { } relativePath := repoRelativePath(self.c.Git().RepoPaths.WorktreePath(), info.Path) if relativePath == "" { - return nil + return diffLinesFileNotFoundError(self.c, info.Path) } self.c.LogAction(self.c.Tr.Actions.OpenPullRequest) diff --git a/pkg/gui/controllers/working_tree_diff_actions.go b/pkg/gui/controllers/working_tree_diff_actions.go index f42fce0c3..f4c0f6334 100644 --- a/pkg/gui/controllers/working_tree_diff_actions.go +++ b/pkg/gui/controllers/working_tree_diff_actions.go @@ -59,7 +59,10 @@ func (self *WorkingTreeDiffActions) PrimaryAction(pane types.DiffPaneContext, fi self.c.UserConfig().Keybinding.Universal.IncreaseContextInDiffView) } - linesByFile, onStagedSide := self.diffLineSelection(pane, firstBufferLine, lastBufferLine) + linesByFile, onStagedSide, err := self.diffLineSelection(pane, firstBufferLine, lastBufferLine) + if err != nil { + return err + } if len(linesByFile) == 0 { return nil } @@ -79,7 +82,10 @@ func (self *WorkingTreeDiffActions) DiscardSelection(pane types.DiffPaneContext, self.c.UserConfig().Keybinding.Universal.IncreaseContextInDiffView) } - linesByFile, onStagedSide := self.diffLineSelection(pane, firstBufferLine, lastBufferLine) + linesByFile, onStagedSide, err := self.diffLineSelection(pane, firstBufferLine, lastBufferLine) + if err != nil { + return err + } if len(linesByFile) == 0 { return nil } @@ -117,7 +123,10 @@ func (self *WorkingTreeDiffActions) DiscardSelectionDisabledReason(types.DiffPan func (self *WorkingTreeDiffActions) EditHunk( pane types.DiffPaneContext, firstBufferLine int, lastBufferLine int, ) error { - linesByFile, onStagedSide := self.diffLineSelection(pane, firstBufferLine, lastBufferLine) + linesByFile, onStagedSide, err := self.diffLineSelection(pane, firstBufferLine, lastBufferLine) + if err != nil { + return err + } if len(linesByFile) == 0 { return nil } @@ -203,21 +212,23 @@ type fileDiffLines struct { // 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. +// grouped by the file they belong to, in the order the diff shows the files. If a line +// belongs to no file of the files panel, the diff and the panel disagree. Acting on the +// other lines would do only part of what the user asked for, so this fails instead. func (self *WorkingTreeDiffActions) diffLineSelection( pane types.DiffPaneContext, firstBufferLine int, lastBufferLine int, -) (linesByFile []fileDiffLines, onStagedSide bool) { +) (linesByFile []fileDiffLines, onStagedSide bool, err error) { 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 + return nil, false, diffLinesFileNotFoundError(self.c, path) } linesByFile = append(linesByFile, fileDiffLines{file: file, lines: infosByPath[path]}) } - return linesByFile, self.showsStagedSide(pane) + return linesByFile, self.showsStagedSide(pane), nil } // applyDiffLineSelection applies the selected change lines, a patch per file, and diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index efd84d722..522f9cbbb 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -748,6 +748,7 @@ type TranslationSet struct { CopySelectedDiffLinesToClipboard string SelectedDiffLinesCopiedToast string SelectionNotFoundInDiffToast string + DiffLinesFileNotFound string NoFilesStagedTitle string NoFilesStagedPrompt string BranchNotFoundTitle string @@ -1938,6 +1939,7 @@ func EnglishTranslationSet() *TranslationSet { CopySelectedDiffLinesToClipboard: "Copy selected diff lines to clipboard", 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", 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_of_a_file_missing_from_the_commits_diff.go b/pkg/integration/tests/main_view/report_lines_of_a_file_missing_from_the_commits_diff.go new file mode 100644 index 000000000..68a5d3a1e --- /dev/null +++ b/pkg/integration/tests/main_view/report_lines_of_a_file_missing_from_the_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 ReportLinesOfAFileMissingFromTheCommitsDiff = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Adding lines to a custom patch that the diff puts in a file the commit doesn't change 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 +++ line names a file in another directory. + cfg.GetUserConfig().Git.DiffRenderers = []config.DiffRendererConfig{ + {Command: `printf '\033]1717;1\007'; sed 's#+++ b/#+++ b/elsewhere/#'`}, + } + }, + 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 file 'elsewhere/file1' that the selected lines belong to")). + Confirm() + + // No patch is left behind. + t.Views().Information().Content(DoesNotContain("Building patch")) + }, +}) diff --git a/pkg/integration/tests/main_view/report_lines_of_a_file_missing_from_the_files_panel.go b/pkg/integration/tests/main_view/report_lines_of_a_file_missing_from_the_files_panel.go new file mode 100644 index 000000000..1e03ba5eb --- /dev/null +++ b/pkg/integration/tests/main_view/report_lines_of_a_file_missing_from_the_files_panel.go @@ -0,0 +1,50 @@ +package main_view + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var ReportLinesOfAFileMissingFromTheFilesPanel = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Staging lines that the diff puts in a file the files panel doesn't have 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 +++ line names a file in another directory. + cfg.GetUserConfig().Git.DiffRenderers = []config.DiffRendererConfig{ + {Command: `printf '\033]1717;1\007'; sed 's#+++ b/#+++ b/elsewhere/#'`}, + } + }, + 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"), + ). + PressPrimaryAction() + + t.ExpectPopup().Alert(). + Title(Equals("Error")). + Content(Equals("Can't find the file 'elsewhere/file1' that the selected lines belong to")). + Confirm() + + t.Views().Files().Lines( + Contains(" M file1"), + ) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index c7a8219bc..095a44e97 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.ReportLinesOfAFileMissingFromTheCommitsDiff, + main_view.ReportLinesOfAFileMissingFromTheFilesPanel, main_view.ResetAPatchBuiltFromACommitsDiff, main_view.ResetPatchWithEscape, main_view.ResetThePatchFromThePaneShowingIt,