From eefc1a249a40f6f4c23bc40b4a11033b61200c98 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 13 Dec 2024 21:20:09 +0100 Subject: [PATCH] Adjust line number for working copy when editing a line There are two ways to jump to the editor on a specific line: pressing `e` in the staging or patch building panels, or clicking on a hyperlink in a delta diff. In both cases, this works perfectly in the unstaged changes view, but in other views (either staged changes, or an older commit) it can often jump to the wrong line; this happens when there are further changes to the file being viewed in later commits or in unstaged changes. This commit fixes this so that you end up on the right line in these cases. --- pkg/commands/patch/patch.go | 21 +++++++ pkg/commands/patch/patch_test.go | 56 +++++++++++++++++++ pkg/gui/context/branches_context.go | 8 +++ pkg/gui/context/commit_files_context.go | 7 +++ pkg/gui/context/local_commits_context.go | 8 +++ pkg/gui/context/reflog_commits_context.go | 4 ++ pkg/gui/context/remote_branches_context.go | 4 ++ pkg/gui/context/remotes_context.go | 4 ++ pkg/gui/context/stash_context.go | 4 ++ pkg/gui/context/sub_commits_context.go | 8 +++ pkg/gui/context/tags_context.go | 4 ++ pkg/gui/controllers/helpers/diff_helper.go | 43 ++++++++++++++ .../controllers/patch_building_controller.go | 1 + pkg/gui/controllers/staging_controller.go | 1 + pkg/gui/gui.go | 1 + pkg/gui/types/context.go | 8 +++ .../edit_line_in_patch_building_panel.go | 3 - 17 files changed, 182 insertions(+), 3 deletions(-) diff --git a/pkg/commands/patch/patch.go b/pkg/commands/patch/patch.go index 04933472747..d785fe49e27 100644 --- a/pkg/commands/patch/patch.go +++ b/pkg/commands/patch/patch.go @@ -154,3 +154,24 @@ func (self *Patch) LineCount() int { func (self *Patch) HunkCount() int { return len(self.hunks) } + +// Adjust the given line number (one-based) according to the current patch. The +// patch is supposed to be a diff of an old file state against the working +// directory; the line number is a line number in that old file, and the +// function returns the corresponding line number in the working directory file. +func (self *Patch) AdjustLineNumber(lineNumber int) int { + adjustedLineNumber := lineNumber + for _, hunk := range self.hunks { + if hunk.oldStart >= lineNumber { + break + } + + if hunk.oldStart+hunk.oldLength() > lineNumber { + return hunk.newStart + } + + adjustedLineNumber += hunk.newLength() - hunk.oldLength() + } + + return adjustedLineNumber +} diff --git a/pkg/commands/patch/patch_test.go b/pkg/commands/patch/patch_test.go index fc166cbae46..d9d330017f0 100644 --- a/pkg/commands/patch/patch_test.go +++ b/pkg/commands/patch/patch_test.go @@ -639,3 +639,59 @@ func TestGetNextStageableLineIndex(t *testing.T) { }) } } + +func TestAdjustLineNumber(t *testing.T) { + type scenario struct { + oldLineNumbers []int + expectedResults []int + } + scenarios := []scenario{ + { + oldLineNumbers: []int{1, 2, 3, 4, 5, 6, 7}, + expectedResults: []int{1, 2, 2, 3, 4, 7, 8}, + }, + } + + // The following diff was generated from old.txt: + // 1 + // 2a + // 2b + // 3 + // 4 + // 7 + // 8 + // against new.txt: + // 1 + // 2 + // 3 + // 4 + // 5 + // 6 + // 7 + // 8 + + // This test setup makes the test easy to understand, because the resulting + // adjusted line numbers are the same as the content of the lines in new.txt. + + diff := `--- old.txt 2024-12-16 18:04:29 ++++ new.txt 2024-12-16 18:04:27 +@@ -2,2 +2 @@ +-2a +-2b ++2 +@@ -5,0 +5,2 @@ ++5 ++6 +` + + patch := Parse(diff) + + for _, s := range scenarios { + t.Run("TestAdjustLineNumber", func(t *testing.T) { + for idx, oldLineNumber := range s.oldLineNumbers { + result := patch.AdjustLineNumber(oldLineNumber) + assert.Equal(t, s.expectedResults[idx], result) + } + }) + } +} diff --git a/pkg/gui/context/branches_context.go b/pkg/gui/context/branches_context.go index f03c05990a3..faff68ba9ce 100644 --- a/pkg/gui/context/branches_context.go +++ b/pkg/gui/context/branches_context.go @@ -80,6 +80,14 @@ func (self *BranchesContext) GetDiffTerminals() []string { return nil } +func (self *BranchesContext) RefForAdjustingLineNumberInDiff() string { + branch := self.GetSelected() + if branch != nil { + return branch.ID() + } + return "" +} + func (self *BranchesContext) ShowBranchHeadsInSubCommits() bool { return true } diff --git a/pkg/gui/context/commit_files_context.go b/pkg/gui/context/commit_files_context.go index 4e938248129..dc92139bddd 100644 --- a/pkg/gui/context/commit_files_context.go +++ b/pkg/gui/context/commit_files_context.go @@ -77,6 +77,13 @@ func (self *CommitFilesContext) GetDiffTerminals() []string { return []string{self.GetRef().RefName()} } +func (self *CommitFilesContext) RefForAdjustingLineNumberInDiff() string { + if refs := self.GetRefRange(); refs != nil { + return refs.To.RefName() + } + return self.GetRef().RefName() +} + func (self *CommitFilesContext) GetFromAndToForDiff() (string, string) { if refs := self.GetRefRange(); refs != nil { return refs.From.ParentRefName(), refs.To.RefName() diff --git a/pkg/gui/context/local_commits_context.go b/pkg/gui/context/local_commits_context.go index 6d1a72aae3f..4f2f797e5da 100644 --- a/pkg/gui/context/local_commits_context.go +++ b/pkg/gui/context/local_commits_context.go @@ -170,6 +170,14 @@ func (self *LocalCommitsContext) GetDiffTerminals() []string { return []string{itemId} } +func (self *LocalCommitsContext) RefForAdjustingLineNumberInDiff() string { + commits, _, _ := self.GetSelectedItems() + if commits == nil { + return "" + } + return commits[0].Hash +} + func (self *LocalCommitsContext) ModelSearchResults(searchStr string, caseSensitive bool) []gocui.SearchPosition { return searchModelCommits(caseSensitive, self.GetCommits(), self.ColumnPositions(), searchStr) } diff --git a/pkg/gui/context/reflog_commits_context.go b/pkg/gui/context/reflog_commits_context.go index db33481e59d..518edeefa88 100644 --- a/pkg/gui/context/reflog_commits_context.go +++ b/pkg/gui/context/reflog_commits_context.go @@ -86,6 +86,10 @@ func (self *ReflogCommitsContext) GetDiffTerminals() []string { return []string{itemId} } +func (self *ReflogCommitsContext) RefForAdjustingLineNumberInDiff() string { + return self.GetSelectedItemId() +} + func (self *ReflogCommitsContext) ShowBranchHeadsInSubCommits() bool { return false } diff --git a/pkg/gui/context/remote_branches_context.go b/pkg/gui/context/remote_branches_context.go index 892953f8256..9e9b00eb19b 100644 --- a/pkg/gui/context/remote_branches_context.go +++ b/pkg/gui/context/remote_branches_context.go @@ -78,6 +78,10 @@ func (self *RemoteBranchesContext) GetDiffTerminals() []string { return []string{itemId} } +func (self *RemoteBranchesContext) RefForAdjustingLineNumberInDiff() string { + return self.GetSelectedItemId() +} + func (self *RemoteBranchesContext) ShowBranchHeadsInSubCommits() bool { return true } diff --git a/pkg/gui/context/remotes_context.go b/pkg/gui/context/remotes_context.go index 237783dcad7..4a96bbc1881 100644 --- a/pkg/gui/context/remotes_context.go +++ b/pkg/gui/context/remotes_context.go @@ -53,3 +53,7 @@ func (self *RemotesContext) GetDiffTerminals() []string { return []string{itemId} } + +func (self *RemotesContext) RefForAdjustingLineNumberInDiff() string { + return "" +} diff --git a/pkg/gui/context/stash_context.go b/pkg/gui/context/stash_context.go index 64c7c9fc979..7dc0066d958 100644 --- a/pkg/gui/context/stash_context.go +++ b/pkg/gui/context/stash_context.go @@ -71,3 +71,7 @@ func (self *StashContext) GetDiffTerminals() []string { return []string{itemId} } + +func (self *StashContext) RefForAdjustingLineNumberInDiff() string { + return self.GetSelectedItemId() +} diff --git a/pkg/gui/context/sub_commits_context.go b/pkg/gui/context/sub_commits_context.go index cd19dcae228..b3b3fd1b476 100644 --- a/pkg/gui/context/sub_commits_context.go +++ b/pkg/gui/context/sub_commits_context.go @@ -217,6 +217,14 @@ func (self *SubCommitsContext) GetDiffTerminals() []string { return []string{itemId} } +func (self *SubCommitsContext) RefForAdjustingLineNumberInDiff() string { + commits, _, _ := self.GetSelectedItems() + if commits == nil { + return "" + } + return commits[0].Hash +} + func (self *SubCommitsContext) ModelSearchResults(searchStr string, caseSensitive bool) []gocui.SearchPosition { return searchModelCommits(caseSensitive, self.GetCommits(), self.ColumnPositions(), searchStr) } diff --git a/pkg/gui/context/tags_context.go b/pkg/gui/context/tags_context.go index 39ae607027e..c77a5292af7 100644 --- a/pkg/gui/context/tags_context.go +++ b/pkg/gui/context/tags_context.go @@ -66,6 +66,10 @@ func (self *TagsContext) GetDiffTerminals() []string { return []string{itemId} } +func (self *TagsContext) RefForAdjustingLineNumberInDiff() string { + return self.GetSelectedItemId() +} + func (self *TagsContext) ShowBranchHeadsInSubCommits() bool { return true } diff --git a/pkg/gui/controllers/helpers/diff_helper.go b/pkg/gui/controllers/helpers/diff_helper.go index 2eda84fc11b..7035849d4ff 100644 --- a/pkg/gui/controllers/helpers/diff_helper.go +++ b/pkg/gui/controllers/helpers/diff_helper.go @@ -5,6 +5,7 @@ import ( "github.com/jesseduffield/lazygit/pkg/commands/git_commands" "github.com/jesseduffield/lazygit/pkg/commands/models" + "github.com/jesseduffield/lazygit/pkg/commands/patch" "github.com/jesseduffield/lazygit/pkg/gui/context" "github.com/jesseduffield/lazygit/pkg/gui/modes/diffing" "github.com/jesseduffield/lazygit/pkg/gui/style" @@ -164,3 +165,45 @@ func (self *DiffHelper) OpenDiffToolForRef(selectedRef types.Ref) error { })) return err } + +// AdjustLineNumber is used to adjust a line number in the diff that's currently +// being viewed, so that it corresponds to the line number in the actual working +// copy state of the file. It is used when clicking on a delta hyperlink in a +// diff, or when pressing `e` in the staging or patch building panels. It works +// by getting a diff of what's being viewed in the main view against the working +// copy, and then using that diff to adjust the line number. +// path is the file path of the file being viewed +// linenumber is the line number to adjust (one-based) +// viewname is the name of the view that shows the diff. We need to pass it +// because the diff adjustment is slightly different depending on which view is +// showing the diff. +func (self *DiffHelper) AdjustLineNumber(path string, linenumber int, viewname string) int { + switch viewname { + + case "main", "patchBuilding": + if diffableContext, ok := self.c.Context().CurrentSide().(types.DiffableContext); ok { + ref := diffableContext.RefForAdjustingLineNumberInDiff() + if len(ref) != 0 { + return self.adjustLineNumber(linenumber, ref, "--", path) + } + } + // if the type cast to DiffableContext returns false, we are in the + // unstaged changes view of the Files panel; no need to adjust line + // numbers in this case + + case "secondary", "stagingSecondary": + return self.adjustLineNumber(linenumber, "--", path) + } + + return linenumber +} + +func (self *DiffHelper) adjustLineNumber(linenumber int, diffArgs ...string) int { + args := append([]string{"--unified=0"}, diffArgs...) + diff, err := self.c.Git().Diff.GetDiff(false, args...) + if err != nil { + return linenumber + } + patch := patch.Parse(diff) + return patch.AdjustLineNumber(linenumber) +} diff --git a/pkg/gui/controllers/patch_building_controller.go b/pkg/gui/controllers/patch_building_controller.go index ea181cd054d..f8a72303cb7 100644 --- a/pkg/gui/controllers/patch_building_controller.go +++ b/pkg/gui/controllers/patch_building_controller.go @@ -105,6 +105,7 @@ func (self *PatchBuildingController) EditFile() error { } lineNumber := self.context().GetState().CurrentLineNumber() + lineNumber = self.c.Helpers().Diff.AdjustLineNumber(path, lineNumber, self.context().GetViewName()) return self.c.Helpers().Files.EditFileAtLine(path, lineNumber) } diff --git a/pkg/gui/controllers/staging_controller.go b/pkg/gui/controllers/staging_controller.go index 4ae9a946b45..b9e092c2f7e 100644 --- a/pkg/gui/controllers/staging_controller.go +++ b/pkg/gui/controllers/staging_controller.go @@ -160,6 +160,7 @@ func (self *StagingController) EditFile() error { } lineNumber := self.context.GetState().CurrentLineNumber() + lineNumber = self.c.Helpers().Diff.AdjustLineNumber(path, lineNumber, self.context.GetViewName()) return self.c.Helpers().Files.EditFileAtLine(path, lineNumber) } diff --git a/pkg/gui/gui.go b/pkg/gui/gui.go index 0072d59f1be..fac8e06c050 100644 --- a/pkg/gui/gui.go +++ b/pkg/gui/gui.go @@ -368,6 +368,7 @@ func (gui *Gui) onNewRepo(startArgs appTypes.StartArgs, contextKey types.Context filepath := matches[1] if matches[2] != "" { lineNumber := utils.MustConvertToInt(matches[2]) + lineNumber = gui.helpers.Diff.AdjustLineNumber(filepath, lineNumber, viewname) return gui.helpers.Files.EditFileAtLine(filepath, lineNumber) } return gui.helpers.Files.EditFiles([]string{filepath}) diff --git a/pkg/gui/types/context.go b/pkg/gui/types/context.go index a2ee3425f3b..5ad8ff3bbf4 100644 --- a/pkg/gui/types/context.go +++ b/pkg/gui/types/context.go @@ -152,6 +152,14 @@ type DiffableContext interface { // which becomes an option when you bring up the diff menu, but when you're just // flicking through branches it will be using the local branch name. GetDiffTerminals() []string + + // Returns the ref that should be used for creating a diff of what's + // currently shown in the main view against the working directory, in order + // to adjust line numbers in the diff to match the current state of the + // shown file. For example, if the main view shows a range diff of commits, + // we need to pass the first commit of the range. This is used by + // DiffHelper.AdjustLineNumber. + RefForAdjustingLineNumberInDiff() string } type IListContext interface { diff --git a/pkg/integration/tests/patch_building/edit_line_in_patch_building_panel.go b/pkg/integration/tests/patch_building/edit_line_in_patch_building_panel.go index 8e9639aaad2..20a7d0a68f6 100644 --- a/pkg/integration/tests/patch_building/edit_line_in_patch_building_panel.go +++ b/pkg/integration/tests/patch_building/edit_line_in_patch_building_panel.go @@ -42,9 +42,6 @@ var EditLineInPatchBuildingPanel = NewIntegrationTest(NewIntegrationTestArgs{ NavigateToLine(Contains("+5")). Press(keys.Universal.Edit) - /* EXPECTED: t.FileSystem().FileContent("edit-command", Contains("file.txt:5\n")) - ACTUAL: */ - t.FileSystem().FileContent("edit-command", Contains("file.txt:2\n")) }, })