Skip to content

Commit

Permalink
Fix custom patch operations on added files
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
stefanhaller committed Jun 22, 2024
1 parent 3f336d2 commit 65d4614
Show file tree
Hide file tree
Showing 10 changed files with 297 additions and 28 deletions.
14 changes: 7 additions & 7 deletions pkg/commands/git_commands/patch.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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()
}
Expand Down Expand Up @@ -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
}
Expand Down
28 changes: 16 additions & 12 deletions pkg/commands/patch/patch_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -74,9 +74,10 @@ func (p *PatchBuilder) PatchToApply(reverse bool) string {
}

patch += p.RenderPatchForFile(RenderPatchForFileOpts{
Filename: filename,
Plain: true,
Reverse: reverse,
Filename: filename,
Plain: true,
Reverse: reverse,
TurnAddedFilesIntoDiffAgainstEmptyFile: turnAddedFilesIntoDiffAgainstEmptyFile,
})
}

Expand Down Expand Up @@ -177,9 +178,10 @@ func (p *PatchBuilder) RemoveFileLineRange(filename string, firstLineIdx, lastLi
}

type RenderPatchForFileOpts struct {
Filename string
Plain bool
Reverse bool
Filename string
Plain bool
Reverse bool
TurnAddedFilesIntoDiffAgainstEmptyFile bool
}

func (p *PatchBuilder) RenderPatchForFile(opts RenderPatchForFileOpts) string {
Expand All @@ -202,8 +204,9 @@ func (p *PatchBuilder) RenderPatchForFile(opts RenderPatchForFileOpts) string {

patch := Parse(info.diff).
Transform(TransformOpts{
Reverse: opts.Reverse,
IncludedLineIndices: info.includedLineIndices,
Reverse: opts.Reverse,
TurnAddedFilesIntoDiffAgainstEmptyFile: opts.TurnAddedFilesIntoDiffAgainstEmptyFile,
IncludedLineIndices: info.includedLineIndices,
})

if opts.Plain {
Expand All @@ -220,9 +223,10 @@ func (p *PatchBuilder) renderEachFilePatch(plain bool) []string {
sort.Strings(filenames)
patches := lo.Map(filenames, func(filename string, _ int) string {
return p.RenderPatchForFile(RenderPatchForFileOpts{
Filename: filename,
Plain: plain,
Reverse: false,
Filename: filename,
Plain: plain,
Reverse: false,
TurnAddedFilesIntoDiffAgainstEmptyFile: true,
})
})
output := lo.Filter(patches, func(patch string, _ int) bool {
Expand Down
25 changes: 24 additions & 1 deletion pkg/commands/patch/transform.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package patch

import "github.com/samber/lo"
import (
"strings"

"github.com/samber/lo"
)

type patchTransformer struct {
patch *Patch
Expand All @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/gui/controllers/custom_patch_options_menu_action.go
Original file line number Diff line number Diff line change
Expand Up @@ -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})
Expand Down
7 changes: 4 additions & 3 deletions pkg/gui/controllers/helpers/patch_building_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,10 @@ func (self *PatchBuildingHelper) RefreshPatchBuildingPanel(opts types.OnFocusOpt
}

secondaryDiff := self.c.Git().Patch.PatchBuilder.RenderPatchForFile(patch.RenderPatchForFileOpts{
Filename: path,
Plain: false,
Reverse: false,
Filename: path,
Plain: false,
Reverse: false,
TurnAddedFilesIntoDiffAgainstEmptyFile: true,
})

context := self.c.Contexts().CustomPatchBuilder
Expand Down
Original file line number Diff line number Diff line change
@@ -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()
},
})
Original file line number Diff line number Diff line change
@@ -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"),
)
})
},
})
Loading

0 comments on commit 65d4614

Please sign in to comment.