1
0
mirror of https://github.com/jesseduffield/lazygit.git synced 2025-04-23 12:18:51 +02:00

Fix custom patch operations for added files (#3684)

- **PR Description**

Several custom patch commands on parts of an added file would fail with the
confusing error message "error: new file XXX depends on old contents".
These were dropping the custom patch from the original commit, moving the
patch to a new commit, moving it to a later commit, or moving it to the index.

We fix this by converting the patch header from an added file to a diff against
an empty file. We do this not just for the purpose of applying the patch, but
also for rendering it and copying it to the clip board. I'm not sure it matters
much in these cases, but it does feel more correct for a filtered patch to be
presented this way.

Fixes #3679.

- **Please check if the PR fulfills these requirements**

* [x] Cheatsheets are up-to-date (run `go generate ./...`)
* [x] Code has been formatted (see
[here](https://github.com/jesseduffield/lazygit/blob/master/CONTRIBUTING.md#code-formatting))
* [x] Tests have been added/updated (see
[here](https://github.com/jesseduffield/lazygit/blob/master/pkg/integration/README.md)
for the integration test guide)
* [ ] Text is internationalised (see
[here](https://github.com/jesseduffield/lazygit/blob/master/CONTRIBUTING.md#internationalisation))
* [ ] Docs have been updated if necessary
* [x] You've read through your own file changes for silly mistakes etc
This commit is contained in:
Stefan Haller 2024-06-23 12:43:09 +02:00 committed by GitHub
commit a08c86c182
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
15 changed files with 516 additions and 26 deletions

View File

@ -47,8 +47,8 @@ type ApplyPatchOpts struct {
Reverse bool
}
func (self *PatchCommands) ApplyCustomPatch(reverse bool) error {
patch := self.PatchBuilder.PatchToApply(reverse)
func (self *PatchCommands) ApplyCustomPatch(reverse bool, turnAddedFilesIntoDiffAgainstEmptyFile bool) error {
patch := self.PatchBuilder.PatchToApply(reverse, turnAddedFilesIntoDiffAgainstEmptyFile)
return self.ApplyPatch(patch, ApplyPatchOpts{
Index: true,
@ -94,7 +94,7 @@ func (self *PatchCommands) DeletePatchesFromCommit(commits []*models.Commit, com
}
// apply each patch in reverse
if err := self.ApplyCustomPatch(true); err != nil {
if err := self.ApplyCustomPatch(true, true); err != nil {
_ = self.rebase.AbortRebase()
return err
}
@ -123,7 +123,7 @@ func (self *PatchCommands) MovePatchToSelectedCommit(commits []*models.Commit, s
}
// apply each patch forward
if err := self.ApplyCustomPatch(false); err != nil {
if err := self.ApplyCustomPatch(false, false); err != nil {
// Don't abort the rebase here; this might cause conflicts, so give
// the user a chance to resolve them
return err
@ -172,7 +172,7 @@ func (self *PatchCommands) MovePatchToSelectedCommit(commits []*models.Commit, s
}
// apply each patch in reverse
if err := self.ApplyCustomPatch(true); err != nil {
if err := self.ApplyCustomPatch(true, true); err != nil {
_ = self.rebase.AbortRebase()
return err
}
@ -228,7 +228,7 @@ func (self *PatchCommands) MovePatchIntoIndex(commits []*models.Commit, commitId
return err
}
if err := self.ApplyCustomPatch(true); err != nil {
if err := self.ApplyCustomPatch(true, true); err != nil {
if self.status.WorkingTreeState() == enums.REBASE_MODE_REBASING {
_ = self.rebase.AbortRebase()
}
@ -282,7 +282,7 @@ func (self *PatchCommands) PullPatchIntoNewCommit(
return err
}
if err := self.ApplyCustomPatch(true); err != nil {
if err := self.ApplyCustomPatch(true, true); err != nil {
_ = self.rebase.AbortRebase()
return err
}

View File

@ -65,7 +65,7 @@ func (p *PatchBuilder) Start(from, to string, reverse bool, canRebase bool) {
p.fileInfoMap = map[string]*fileInfo{}
}
func (p *PatchBuilder) PatchToApply(reverse bool) string {
func (p *PatchBuilder) PatchToApply(reverse bool, turnAddedFilesIntoDiffAgainstEmptyFile bool) string {
patch := ""
for filename, info := range p.fileInfoMap {
@ -73,7 +73,12 @@ func (p *PatchBuilder) PatchToApply(reverse bool) string {
continue
}
patch += p.RenderPatchForFile(filename, true, reverse)
patch += p.RenderPatchForFile(RenderPatchForFileOpts{
Filename: filename,
Plain: true,
Reverse: reverse,
TurnAddedFilesIntoDiffAgainstEmptyFile: turnAddedFilesIntoDiffAgainstEmptyFile,
})
}
return patch
@ -172,8 +177,15 @@ func (p *PatchBuilder) RemoveFileLineRange(filename string, firstLineIdx, lastLi
return nil
}
func (p *PatchBuilder) RenderPatchForFile(filename string, plain bool, reverse bool) string {
info, err := p.getFileInfo(filename)
type RenderPatchForFileOpts struct {
Filename string
Plain bool
Reverse bool
TurnAddedFilesIntoDiffAgainstEmptyFile bool
}
func (p *PatchBuilder) RenderPatchForFile(opts RenderPatchForFileOpts) string {
info, err := p.getFileInfo(opts.Filename)
if err != nil {
p.Log.Error(err)
return ""
@ -183,7 +195,7 @@ func (p *PatchBuilder) RenderPatchForFile(filename string, plain bool, reverse b
return ""
}
if info.mode == WHOLE && plain {
if info.mode == WHOLE && opts.Plain {
// Use the whole diff (spares us parsing it and then formatting it).
// TODO: see if this is actually noticeably faster.
// The reverse flag is only for part patches so we're ignoring it here.
@ -192,11 +204,12 @@ func (p *PatchBuilder) RenderPatchForFile(filename string, plain bool, reverse b
patch := Parse(info.diff).
Transform(TransformOpts{
Reverse: reverse,
IncludedLineIndices: info.includedLineIndices,
Reverse: opts.Reverse,
TurnAddedFilesIntoDiffAgainstEmptyFile: opts.TurnAddedFilesIntoDiffAgainstEmptyFile,
IncludedLineIndices: info.includedLineIndices,
})
if plain {
if opts.Plain {
return patch.FormatPlain()
} else {
return patch.FormatView(FormatViewOpts{})
@ -209,7 +222,12 @@ func (p *PatchBuilder) renderEachFilePatch(plain bool) []string {
sort.Strings(filenames)
patches := lo.Map(filenames, func(filename string, _ int) string {
return p.RenderPatchForFile(filename, plain, false)
return p.RenderPatchForFile(RenderPatchForFileOpts{
Filename: filename,
Plain: plain,
Reverse: false,
TurnAddedFilesIntoDiffAgainstEmptyFile: true,
})
})
output := lo.Filter(patches, func(patch string, _ int) bool {
return patch != ""

View File

@ -1,6 +1,10 @@
package patch
import "github.com/samber/lo"
import (
"strings"
"github.com/samber/lo"
)
type patchTransformer struct {
patch *Patch
@ -22,6 +26,13 @@ type TransformOpts struct {
// information it needs to cleanly apply patches
FileNameOverride string
// Custom patches tend to work better when treating new files as diffs
// against an empty file. The only case where we need this to be false is
// when moving a custom patch to an earlier commit; in that case the patch
// command would fail with the error "file does not exist in index" if we
// treat it as a diff against an empty file.
TurnAddedFilesIntoDiffAgainstEmptyFile bool
// The indices of lines that should be included in the patch.
IncludedLineIndices []int
}
@ -61,6 +72,18 @@ func (self *patchTransformer) transformHeader() []string {
"--- a/" + self.opts.FileNameOverride,
"+++ b/" + self.opts.FileNameOverride,
}
} else if self.opts.TurnAddedFilesIntoDiffAgainstEmptyFile {
result := make([]string, 0, len(self.patch.header))
for idx, line := range self.patch.header {
if strings.HasPrefix(line, "new file mode") {
continue
}
if line == "--- /dev/null" && strings.HasPrefix(self.patch.header[idx+1], "+++ b/") {
line = "--- a/" + self.patch.header[idx+1][6:]
}
result = append(result, line)
}
return result
} else {
return self.patch.header
}

View File

@ -237,7 +237,7 @@ func (self *CustomPatchOptionsMenuAction) handleApplyPatch(reverse bool) error {
action = "Apply patch in reverse"
}
self.c.LogAction(action)
if err := self.c.Git().Patch.ApplyCustomPatch(reverse); err != nil {
if err := self.c.Git().Patch.ApplyCustomPatch(reverse, true); err != nil {
return err
}
return self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC})

View File

@ -3,6 +3,7 @@ package helpers
import (
"errors"
"github.com/jesseduffield/lazygit/pkg/commands/patch"
"github.com/jesseduffield/lazygit/pkg/commands/types/enums"
"github.com/jesseduffield/lazygit/pkg/gui/patch_exploring"
"github.com/jesseduffield/lazygit/pkg/gui/types"
@ -80,7 +81,12 @@ func (self *PatchBuildingHelper) RefreshPatchBuildingPanel(opts types.OnFocusOpt
return err
}
secondaryDiff := self.c.Git().Patch.PatchBuilder.RenderPatchForFile(path, false, false)
secondaryDiff := self.c.Git().Patch.PatchBuilder.RenderPatchForFile(patch.RenderPatchForFileOpts{
Filename: path,
Plain: false,
Reverse: false,
TurnAddedFilesIntoDiffAgainstEmptyFile: true,
})
context := self.c.Contexts().CustomPatchBuilder

View File

@ -0,0 +1,115 @@
package patch_building
import (
"github.com/jesseduffield/lazygit/pkg/config"
. "github.com/jesseduffield/lazygit/pkg/integration/components"
)
var MoveToEarlierCommitFromAddedFile = NewIntegrationTest(NewIntegrationTestArgs{
Description: "Move a patch from a file that was added in a commit to an earlier commit",
ExtraCmdArgs: []string{},
Skip: false,
SetupConfig: func(config *config.AppConfig) {},
SetupRepo: func(shell *Shell) {
shell.EmptyCommit("first commit")
shell.EmptyCommit("destination commit")
shell.CreateFileAndAdd("file1", "1st line\n2nd line\n3rd line\n")
shell.Commit("commit to move from")
},
Run: func(t *TestDriver, keys config.KeybindingConfig) {
t.Views().Commits().
Focus().
Lines(
Contains("commit to move from").IsSelected(),
Contains("destination commit"),
Contains("first commit"),
).
PressEnter()
t.Views().CommitFiles().
IsFocused().
Lines(
Contains("A file").IsSelected(),
).
PressEnter()
t.Views().PatchBuilding().
IsFocused().
SelectNextItem().
PressPrimaryAction()
t.Views().Information().Content(Contains("Building patch"))
t.Views().Commits().
Focus().
SelectNextItem()
t.Common().SelectPatchOption(Contains("Move patch to selected commit"))
// This results in a conflict at the commit we're moving from, because
// it tries to add a file that already exists
t.Common().AcknowledgeConflicts()
t.Views().Files().
IsFocused().
Lines(
Contains("AA").Contains("file"),
).
PressEnter()
t.Views().MergeConflicts().
IsFocused().
TopLines(
Contains("<<<<<<< HEAD"),
Contains("2nd line"),
Contains("======="),
Contains("1st line"),
Contains("2nd line"),
Contains("3rd line"),
Contains(">>>>>>>"),
).
SelectNextItem().
PressPrimaryAction() // choose the version with all three lines
t.Common().ContinueOnConflictsResolved()
t.Views().Commits().
Focus().
Lines(
Contains("commit to move from"),
Contains("destination commit").IsSelected(),
Contains("first commit"),
).
PressEnter()
t.Views().CommitFiles().
IsFocused().
Lines(
Contains("A file").IsSelected(),
).
Tap(func() {
t.Views().Main().ContainsLines(
Equals("+2nd line"),
)
}).
PressEscape()
t.Views().Commits().
IsFocused().
NavigateToLine(Contains("commit to move from")).
PressEnter()
t.Views().CommitFiles().
IsFocused().
Lines(
Contains("M file").IsSelected(),
).
Tap(func() {
t.Views().Main().ContainsLines(
Equals("+1st line"),
Equals(" 2nd line"),
Equals("+3rd line"),
)
})
},
})

View File

@ -0,0 +1,96 @@
package patch_building
import (
"github.com/jesseduffield/lazygit/pkg/config"
. "github.com/jesseduffield/lazygit/pkg/integration/components"
)
var MoveToIndexFromAddedFileWithConflict = NewIntegrationTest(NewIntegrationTestArgs{
Description: "Move a patch from a file that was added in a commit to the index, causing a conflict",
ExtraCmdArgs: []string{},
Skip: false,
SetupConfig: func(config *config.AppConfig) {},
SetupRepo: func(shell *Shell) {
shell.EmptyCommit("first commit")
shell.CreateFileAndAdd("file1", "1st line\n2nd line\n3rd line\n")
shell.Commit("commit to move from")
shell.UpdateFileAndAdd("file1", "1st line\n2nd line changed\n3rd line\n")
shell.Commit("conflicting change")
},
Run: func(t *TestDriver, keys config.KeybindingConfig) {
t.Views().Commits().
Focus().
Lines(
Contains("conflicting change").IsSelected(),
Contains("commit to move from"),
Contains("first commit"),
).
SelectNextItem().
PressEnter()
t.Views().CommitFiles().
IsFocused().
Lines(
Contains("file1").IsSelected(),
).
PressEnter()
t.Views().PatchBuilding().
IsFocused().
SelectNextItem().
PressPrimaryAction()
t.Views().Information().Content(Contains("Building patch"))
t.Common().SelectPatchOption(Contains("Move patch out into index"))
t.Common().AcknowledgeConflicts()
t.Views().Files().
IsFocused().
Lines(
Contains("UU").Contains("file1"),
).
PressEnter()
t.Views().MergeConflicts().
IsFocused().
ContainsLines(
Contains("1st line"),
Contains("<<<<<<< HEAD"),
Contains("======="),
Contains("2nd line changed"),
Contains(">>>>>>>"),
Contains("3rd line"),
).
SelectNextItem().
PressPrimaryAction()
t.Common().ContinueOnConflictsResolved()
t.ExpectPopup().Alert().
Title(Equals("Error")).
Content(Contains("Applied patch to 'file1' with conflicts")).
Confirm()
t.Views().Files().
IsFocused().
Lines(
Contains("UU").Contains("file1"),
).
PressEnter()
t.Views().MergeConflicts().
TopLines(
Contains("1st line"),
Contains("<<<<<<< ours"),
Contains("2nd line changed"),
Contains("======="),
Contains("2nd line"),
Contains(">>>>>>> theirs"),
Contains("3rd line"),
).
IsFocused()
},
})

View File

@ -40,7 +40,6 @@ var MoveToIndexPartOfAdjacentAddedLines = NewIntegrationTest(NewIntegrationTestA
t.Views().PatchBuilding().
IsFocused().
PressEnter().
PressPrimaryAction()
t.Views().Information().Content(Contains("Building patch"))

View File

@ -40,7 +40,6 @@ var MoveToLaterCommitPartialHunk = NewIntegrationTest(NewIntegrationTestArgs{
t.Views().PatchBuilding().
IsFocused().
PressEnter().
PressPrimaryAction().
PressEscape()

View File

@ -0,0 +1,88 @@
package patch_building
import (
"github.com/jesseduffield/lazygit/pkg/config"
. "github.com/jesseduffield/lazygit/pkg/integration/components"
)
var MoveToNewCommitFromAddedFile = NewIntegrationTest(NewIntegrationTestArgs{
Description: "Move a patch from a file that was added in a commit to a new commit",
ExtraCmdArgs: []string{},
Skip: false,
SetupConfig: func(config *config.AppConfig) {},
SetupRepo: func(shell *Shell) {
shell.EmptyCommit("first commit")
shell.CreateFileAndAdd("file1", "1st line\n2nd line\n3rd line\n")
shell.Commit("commit to move from")
},
Run: func(t *TestDriver, keys config.KeybindingConfig) {
t.Views().Commits().
Focus().
Lines(
Contains("commit to move from").IsSelected(),
Contains("first commit"),
).
PressEnter()
t.Views().CommitFiles().
IsFocused().
Lines(
Contains("file1").IsSelected(),
).
PressEnter()
t.Views().PatchBuilding().
IsFocused().
SelectNextItem().
PressPrimaryAction()
t.Views().Information().Content(Contains("Building patch"))
t.Common().SelectPatchOption(Contains("Move patch into new commit"))
t.ExpectPopup().CommitMessagePanel().
InitialText(Equals("")).
Type("new commit").Confirm()
t.Views().Commits().
IsFocused().
Lines(
Contains("new commit").IsSelected(),
Contains("commit to move from"),
Contains("first commit"),
).
PressEnter()
t.Views().CommitFiles().
IsFocused().
Lines(
Contains("M file1").IsSelected(),
).
Tap(func() {
t.Views().Main().ContainsLines(
Equals(" 1st line"),
Equals("+2nd line"),
Equals(" 3rd line"),
)
}).
PressEscape()
t.Views().Commits().
IsFocused().
NavigateToLine(Contains("commit to move from")).
PressEnter()
t.Views().CommitFiles().
IsFocused().
Lines(
Contains("A file1").IsSelected(),
).
Tap(func() {
t.Views().Main().ContainsLines(
Equals("+1st line"),
Equals("+3rd line"),
)
})
},
})

View File

@ -0,0 +1,88 @@
package patch_building
import (
"github.com/jesseduffield/lazygit/pkg/config"
. "github.com/jesseduffield/lazygit/pkg/integration/components"
)
var MoveToNewCommitFromDeletedFile = NewIntegrationTest(NewIntegrationTestArgs{
Description: "Move a patch from a file that was deleted in a commit to a new commit",
ExtraCmdArgs: []string{},
Skip: false,
SetupConfig: func(config *config.AppConfig) {},
SetupRepo: func(shell *Shell) {
shell.CreateFileAndAdd("file1", "1st line\n2nd line\n3rd line\n")
shell.Commit("first commit")
shell.DeleteFileAndAdd("file1")
shell.Commit("commit to move from")
},
Run: func(t *TestDriver, keys config.KeybindingConfig) {
t.Views().Commits().
Focus().
Lines(
Contains("commit to move from").IsSelected(),
Contains("first commit"),
).
PressEnter()
t.Views().CommitFiles().
IsFocused().
Lines(
Contains("D file1").IsSelected(),
).
PressEnter()
t.Views().PatchBuilding().
IsFocused().
SelectNextItem().
PressPrimaryAction()
t.Views().Information().Content(Contains("Building patch"))
t.Common().SelectPatchOption(Contains("Move patch into new commit"))
t.ExpectPopup().CommitMessagePanel().
InitialText(Equals("")).
Type("new commit").Confirm()
t.Views().Commits().
IsFocused().
Lines(
Contains("new commit").IsSelected(),
Contains("commit to move from"),
Contains("first commit"),
).
PressEnter()
t.Views().CommitFiles().
IsFocused().
Lines(
Contains("D file1").IsSelected(),
).
Tap(func() {
t.Views().Main().ContainsLines(
Equals("-2nd line"),
)
}).
PressEscape()
t.Views().Commits().
IsFocused().
NavigateToLine(Contains("commit to move from")).
PressEnter()
t.Views().CommitFiles().
IsFocused().
Lines(
// In the original commit the file is no longer deleted, but modified
Contains("M file1").IsSelected(),
).
Tap(func() {
t.Views().Main().ContainsLines(
Equals("-1st line"),
Equals(" 2nd line"),
Equals("-3rd line"),
)
})
},
})

View File

@ -40,7 +40,6 @@ var MoveToNewCommitPartialHunk = NewIntegrationTest(NewIntegrationTestArgs{
t.Views().PatchBuilding().
IsFocused().
PressEnter().
PressPrimaryAction()
t.Views().Information().Content(Contains("Building patch"))

View File

@ -0,0 +1,56 @@
package patch_building
import (
"github.com/jesseduffield/lazygit/pkg/config"
. "github.com/jesseduffield/lazygit/pkg/integration/components"
)
var RemovePartsOfAddedFile = NewIntegrationTest(NewIntegrationTestArgs{
Description: "Remove a custom patch from a file that was added in a commit",
ExtraCmdArgs: []string{},
Skip: false,
SetupConfig: func(config *config.AppConfig) {},
SetupRepo: func(shell *Shell) {
shell.EmptyCommit("first commit")
shell.CreateFileAndAdd("file1", "1st line\n2nd line\n3rd line\n")
shell.Commit("commit to remove from")
},
Run: func(t *TestDriver, keys config.KeybindingConfig) {
t.Views().Commits().
Focus().
Lines(
Contains("commit to remove from").IsSelected(),
Contains("first commit"),
).
PressEnter()
t.Views().CommitFiles().
IsFocused().
Lines(
Contains("A file1").IsSelected(),
).
PressEnter()
t.Views().PatchBuilding().
IsFocused().
SelectNextItem().
PressPrimaryAction()
t.Views().Information().Content(Contains("Building patch"))
t.Common().SelectPatchOption(Contains("Remove patch from original commit"))
t.Views().CommitFiles().
IsFocused().
Lines(
Contains("A file1").IsSelected(),
).
PressEscape()
t.Views().Main().ContainsLines(
Equals("+1st line"),
Equals("+3rd line"),
)
},
})

View File

@ -126,9 +126,8 @@ var SpecificSelection = NewIntegrationTest(NewIntegrationTestArgs{
t.Views().Secondary().ContainsLines(
// direct-file patch
Contains(`diff --git a/direct-file b/direct-file`),
Contains(`new file mode 100644`),
Contains(`index`),
Contains(`--- /dev/null`),
Contains(`--- a/direct-file`),
Contains(`+++ b/direct-file`),
Contains(`@@ -0,0 +1 @@`),
Contains(`+direct file content`),
@ -149,9 +148,8 @@ var SpecificSelection = NewIntegrationTest(NewIntegrationTestArgs{
Contains(` 1f`),
// line-file patch
Contains(`diff --git a/line-file b/line-file`),
Contains(`new file mode 100644`),
Contains(`index`),
Contains(`--- /dev/null`),
Contains(`--- a/line-file`),
Contains(`+++ b/line-file`),
Contains(`@@ -0,0 +1,5 @@`),
Contains(`+2a`),

View File

@ -229,8 +229,10 @@ var tests = []*components.IntegrationTest{
patch_building.ApplyInReverseWithConflict,
patch_building.MoveRangeToIndex,
patch_building.MoveToEarlierCommit,
patch_building.MoveToEarlierCommitFromAddedFile,
patch_building.MoveToEarlierCommitNoKeepEmpty,
patch_building.MoveToIndex,
patch_building.MoveToIndexFromAddedFileWithConflict,
patch_building.MoveToIndexPartOfAdjacentAddedLines,
patch_building.MoveToIndexPartial,
patch_building.MoveToIndexWithConflict,
@ -238,8 +240,11 @@ var tests = []*components.IntegrationTest{
patch_building.MoveToLaterCommit,
patch_building.MoveToLaterCommitPartialHunk,
patch_building.MoveToNewCommit,
patch_building.MoveToNewCommitFromAddedFile,
patch_building.MoveToNewCommitFromDeletedFile,
patch_building.MoveToNewCommitPartialHunk,
patch_building.RemoveFromCommit,
patch_building.RemovePartsOfAddedFile,
patch_building.ResetWithEscape,
patch_building.SelectAllFiles,
patch_building.SpecificSelection,