Skip to content

Commit 4cd15a3

Browse files
committed
Fix custom patch operations on added files
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.
1 parent 13a3540 commit 4cd15a3

File tree

10 files changed

+297
-28
lines changed

10 files changed

+297
-28
lines changed

pkg/commands/git_commands/patch.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,8 @@ type ApplyPatchOpts struct {
4747
Reverse bool
4848
}
4949

50-
func (self *PatchCommands) ApplyCustomPatch(reverse bool) error {
51-
patch := self.PatchBuilder.PatchToApply(reverse)
50+
func (self *PatchCommands) ApplyCustomPatch(reverse bool, turnAddedFilesIntoDiffAgainstEmptyFile bool) error {
51+
patch := self.PatchBuilder.PatchToApply(reverse, turnAddedFilesIntoDiffAgainstEmptyFile)
5252

5353
return self.ApplyPatch(patch, ApplyPatchOpts{
5454
Index: true,
@@ -94,7 +94,7 @@ func (self *PatchCommands) DeletePatchesFromCommit(commits []*models.Commit, com
9494
}
9595

9696
// apply each patch in reverse
97-
if err := self.ApplyCustomPatch(true); err != nil {
97+
if err := self.ApplyCustomPatch(true, true); err != nil {
9898
_ = self.rebase.AbortRebase()
9999
return err
100100
}
@@ -123,7 +123,7 @@ func (self *PatchCommands) MovePatchToSelectedCommit(commits []*models.Commit, s
123123
}
124124

125125
// apply each patch forward
126-
if err := self.ApplyCustomPatch(false); err != nil {
126+
if err := self.ApplyCustomPatch(false, false); err != nil {
127127
// Don't abort the rebase here; this might cause conflicts, so give
128128
// the user a chance to resolve them
129129
return err
@@ -172,7 +172,7 @@ func (self *PatchCommands) MovePatchToSelectedCommit(commits []*models.Commit, s
172172
}
173173

174174
// apply each patch in reverse
175-
if err := self.ApplyCustomPatch(true); err != nil {
175+
if err := self.ApplyCustomPatch(true, true); err != nil {
176176
_ = self.rebase.AbortRebase()
177177
return err
178178
}
@@ -228,7 +228,7 @@ func (self *PatchCommands) MovePatchIntoIndex(commits []*models.Commit, commitId
228228
return err
229229
}
230230

231-
if err := self.ApplyCustomPatch(true); err != nil {
231+
if err := self.ApplyCustomPatch(true, true); err != nil {
232232
if self.status.WorkingTreeState() == enums.REBASE_MODE_REBASING {
233233
_ = self.rebase.AbortRebase()
234234
}
@@ -282,7 +282,7 @@ func (self *PatchCommands) PullPatchIntoNewCommit(
282282
return err
283283
}
284284

285-
if err := self.ApplyCustomPatch(true); err != nil {
285+
if err := self.ApplyCustomPatch(true, true); err != nil {
286286
_ = self.rebase.AbortRebase()
287287
return err
288288
}

pkg/commands/patch/patch_builder.go

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ func (p *PatchBuilder) Start(from, to string, reverse bool, canRebase bool) {
6565
p.fileInfoMap = map[string]*fileInfo{}
6666
}
6767

68-
func (p *PatchBuilder) PatchToApply(reverse bool) string {
68+
func (p *PatchBuilder) PatchToApply(reverse bool, turnAddedFilesIntoDiffAgainstEmptyFile bool) string {
6969
patch := ""
7070

7171
for filename, info := range p.fileInfoMap {
@@ -74,9 +74,10 @@ func (p *PatchBuilder) PatchToApply(reverse bool) string {
7474
}
7575

7676
patch += p.RenderPatchForFile(RenderPatchForFileOpts{
77-
Filename: filename,
78-
Plain: true,
79-
Reverse: reverse,
77+
Filename: filename,
78+
Plain: true,
79+
Reverse: reverse,
80+
TurnAddedFilesIntoDiffAgainstEmptyFile: turnAddedFilesIntoDiffAgainstEmptyFile,
8081
})
8182
}
8283

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

179180
type RenderPatchForFileOpts struct {
180-
Filename string
181-
Plain bool
182-
Reverse bool
181+
Filename string
182+
Plain bool
183+
Reverse bool
184+
TurnAddedFilesIntoDiffAgainstEmptyFile bool
183185
}
184186

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

203205
patch := Parse(info.diff).
204206
Transform(TransformOpts{
205-
Reverse: opts.Reverse,
206-
IncludedLineIndices: info.includedLineIndices,
207+
Reverse: opts.Reverse,
208+
TurnAddedFilesIntoDiffAgainstEmptyFile: opts.TurnAddedFilesIntoDiffAgainstEmptyFile,
209+
IncludedLineIndices: info.includedLineIndices,
207210
})
208211

209212
if opts.Plain {
@@ -220,9 +223,10 @@ func (p *PatchBuilder) renderEachFilePatch(plain bool) []string {
220223
sort.Strings(filenames)
221224
patches := lo.Map(filenames, func(filename string, _ int) string {
222225
return p.RenderPatchForFile(RenderPatchForFileOpts{
223-
Filename: filename,
224-
Plain: plain,
225-
Reverse: false,
226+
Filename: filename,
227+
Plain: plain,
228+
Reverse: false,
229+
TurnAddedFilesIntoDiffAgainstEmptyFile: true,
226230
})
227231
})
228232
output := lo.Filter(patches, func(patch string, _ int) bool {

pkg/commands/patch/transform.go

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
package patch
22

3-
import "github.com/samber/lo"
3+
import (
4+
"strings"
5+
6+
"github.com/samber/lo"
7+
)
48

59
type patchTransformer struct {
610
patch *Patch
@@ -22,6 +26,13 @@ type TransformOpts struct {
2226
// information it needs to cleanly apply patches
2327
FileNameOverride string
2428

29+
// Custom patches tend to work better when treating new files as diffs
30+
// against an empty file. The only case where we need this to be false is
31+
// when moving a custom patch to an earlier commit; in that case the patch
32+
// command would fail with the error "file does not exist in index" if we
33+
// treat it as a diff against an empty file.
34+
TurnAddedFilesIntoDiffAgainstEmptyFile bool
35+
2536
// The indices of lines that should be included in the patch.
2637
IncludedLineIndices []int
2738
}
@@ -61,6 +72,18 @@ func (self *patchTransformer) transformHeader() []string {
6172
"--- a/" + self.opts.FileNameOverride,
6273
"+++ b/" + self.opts.FileNameOverride,
6374
}
75+
} else if self.opts.TurnAddedFilesIntoDiffAgainstEmptyFile {
76+
result := make([]string, 0, len(self.patch.header))
77+
for idx, line := range self.patch.header {
78+
if strings.HasPrefix(line, "new file mode") {
79+
continue
80+
}
81+
if line == "--- /dev/null" && strings.HasPrefix(self.patch.header[idx+1], "+++ b/") {
82+
line = "--- a/" + self.patch.header[idx+1][6:]
83+
}
84+
result = append(result, line)
85+
}
86+
return result
6487
} else {
6588
return self.patch.header
6689
}

pkg/gui/controllers/custom_patch_options_menu_action.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@ func (self *CustomPatchOptionsMenuAction) handleApplyPatch(reverse bool) error {
237237
action = "Apply patch in reverse"
238238
}
239239
self.c.LogAction(action)
240-
if err := self.c.Git().Patch.ApplyCustomPatch(reverse); err != nil {
240+
if err := self.c.Git().Patch.ApplyCustomPatch(reverse, true); err != nil {
241241
return err
242242
}
243243
return self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC})

pkg/gui/controllers/helpers/patch_building_helper.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,10 @@ func (self *PatchBuildingHelper) RefreshPatchBuildingPanel(opts types.OnFocusOpt
8282
}
8383

8484
secondaryDiff := self.c.Git().Patch.PatchBuilder.RenderPatchForFile(patch.RenderPatchForFileOpts{
85-
Filename: path,
86-
Plain: false,
87-
Reverse: false,
85+
Filename: path,
86+
Plain: false,
87+
Reverse: false,
88+
TurnAddedFilesIntoDiffAgainstEmptyFile: true,
8889
})
8990

9091
context := self.c.Contexts().CustomPatchBuilder
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
package patch_building
2+
3+
import (
4+
"github.com/jesseduffield/lazygit/pkg/config"
5+
. "github.com/jesseduffield/lazygit/pkg/integration/components"
6+
)
7+
8+
var MoveToIndexFromAddedFileWithConflict = NewIntegrationTest(NewIntegrationTestArgs{
9+
Description: "Move a patch from a file that was added in a commit to the index, causing a conflict",
10+
ExtraCmdArgs: []string{},
11+
Skip: false,
12+
SetupConfig: func(config *config.AppConfig) {},
13+
SetupRepo: func(shell *Shell) {
14+
shell.EmptyCommit("first commit")
15+
16+
shell.CreateFileAndAdd("file1", "1st line\n2nd line\n3rd line\n")
17+
shell.Commit("commit to move from")
18+
shell.UpdateFileAndAdd("file1", "1st line\n2nd line changed\n3rd line\n")
19+
shell.Commit("conflicting change")
20+
},
21+
Run: func(t *TestDriver, keys config.KeybindingConfig) {
22+
t.Views().Commits().
23+
Focus().
24+
Lines(
25+
Contains("conflicting change").IsSelected(),
26+
Contains("commit to move from"),
27+
Contains("first commit"),
28+
).
29+
SelectNextItem().
30+
PressEnter()
31+
32+
t.Views().CommitFiles().
33+
IsFocused().
34+
Lines(
35+
Contains("file1").IsSelected(),
36+
).
37+
PressEnter()
38+
39+
t.Views().PatchBuilding().
40+
IsFocused().
41+
SelectNextItem().
42+
PressPrimaryAction()
43+
44+
t.Views().Information().Content(Contains("Building patch"))
45+
46+
t.Common().SelectPatchOption(Contains("Move patch out into index"))
47+
48+
t.Common().AcknowledgeConflicts()
49+
50+
t.Views().Files().
51+
IsFocused().
52+
Lines(
53+
Contains("UU").Contains("file1"),
54+
).
55+
PressEnter()
56+
57+
t.Views().MergeConflicts().
58+
IsFocused().
59+
ContainsLines(
60+
Contains("1st line"),
61+
Contains("<<<<<<< HEAD"),
62+
Contains("======="),
63+
Contains("2nd line changed"),
64+
Contains(">>>>>>>"),
65+
Contains("3rd line"),
66+
).
67+
SelectNextItem().
68+
PressPrimaryAction()
69+
70+
t.Common().ContinueOnConflictsResolved()
71+
72+
t.ExpectPopup().Alert().
73+
Title(Equals("Error")).
74+
Content(Contains("Applied patch to 'file1' with conflicts")).
75+
Confirm()
76+
77+
t.Views().Files().
78+
IsFocused().
79+
Lines(
80+
Contains("UU").Contains("file1"),
81+
).
82+
PressEnter()
83+
84+
t.Views().MergeConflicts().
85+
TopLines(
86+
Contains("1st line"),
87+
Contains("<<<<<<< ours"),
88+
Contains("2nd line changed"),
89+
Contains("======="),
90+
Contains("2nd line"),
91+
Contains(">>>>>>> theirs"),
92+
Contains("3rd line"),
93+
).
94+
IsFocused()
95+
},
96+
})
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
package patch_building
2+
3+
import (
4+
"github.com/jesseduffield/lazygit/pkg/config"
5+
. "github.com/jesseduffield/lazygit/pkg/integration/components"
6+
)
7+
8+
var MoveToNewCommitFromAddedFile = NewIntegrationTest(NewIntegrationTestArgs{
9+
Description: "Move a patch from a file that was added in a commit to a new commit",
10+
ExtraCmdArgs: []string{},
11+
Skip: false,
12+
SetupConfig: func(config *config.AppConfig) {},
13+
SetupRepo: func(shell *Shell) {
14+
shell.EmptyCommit("first commit")
15+
16+
shell.CreateFileAndAdd("file1", "1st line\n2nd line\n3rd line\n")
17+
shell.Commit("commit to move from")
18+
},
19+
Run: func(t *TestDriver, keys config.KeybindingConfig) {
20+
t.Views().Commits().
21+
Focus().
22+
Lines(
23+
Contains("commit to move from").IsSelected(),
24+
Contains("first commit"),
25+
).
26+
PressEnter()
27+
28+
t.Views().CommitFiles().
29+
IsFocused().
30+
Lines(
31+
Contains("file1").IsSelected(),
32+
).
33+
PressEnter()
34+
35+
t.Views().PatchBuilding().
36+
IsFocused().
37+
SelectNextItem().
38+
PressPrimaryAction()
39+
40+
t.Views().Information().Content(Contains("Building patch"))
41+
42+
t.Common().SelectPatchOption(Contains("Move patch into new commit"))
43+
44+
t.ExpectPopup().CommitMessagePanel().
45+
InitialText(Equals("")).
46+
Type("new commit").Confirm()
47+
48+
t.Views().Commits().
49+
IsFocused().
50+
Lines(
51+
Contains("new commit").IsSelected(),
52+
Contains("commit to move from"),
53+
Contains("first commit"),
54+
).
55+
PressEnter()
56+
57+
t.Views().CommitFiles().
58+
IsFocused().
59+
Lines(
60+
Contains("M file1").IsSelected(),
61+
).
62+
Tap(func() {
63+
t.Views().Main().ContainsLines(
64+
Equals(" 1st line"),
65+
Equals("+2nd line"),
66+
Equals(" 3rd line"),
67+
)
68+
}).
69+
PressEscape()
70+
71+
t.Views().Commits().
72+
IsFocused().
73+
NavigateToLine(Contains("commit to move from")).
74+
PressEnter()
75+
76+
t.Views().CommitFiles().
77+
IsFocused().
78+
Lines(
79+
Contains("A file1").IsSelected(),
80+
).
81+
Tap(func() {
82+
t.Views().Main().ContainsLines(
83+
Equals("+1st line"),
84+
Equals("+3rd line"),
85+
)
86+
})
87+
},
88+
})

0 commit comments

Comments
 (0)