From 9cc33c479b6da6ef0db46ebf5a75e8b06c39310e Mon Sep 17 00:00:00 2001
From: Stefan Haller <stefan@haller-berlin.de>
Date: Thu, 23 Feb 2023 18:44:52 +0100
Subject: [PATCH] Use forward patches and --reverse flag for partial patches
 too

There's no reason to have two different ways of applying patches for whole-file
patches and partial patches; use --reverse for both. Not only does this simplify
the code a bit, but it fixes an actual problem: when reverseOnGenerate and
keepOriginalHeader are both true, the generated patch header is broken (the two
blobs in the line `index 6d1959b..6dc5f84 100644` are swapped). Git fails to do
a proper three-way merge in that case, as it expects the first of the two blobs
to be the common ancestor.

It would be possible to fix this by extending ModifiedPatchForLines to swap the
two blobs in this case; but this would prevent us from concatenating all patches
and apply them in one go, which we are going to do later in the branch.
---
 pkg/commands/patch/patch_manager.go  | 33 +++++++++++++---------------
 pkg/commands/patch/patch_modifier.go |  4 ++++
 pkg/gui/refresh.go                   |  2 +-
 3 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/pkg/commands/patch/patch_manager.go b/pkg/commands/patch/patch_manager.go
index 71d6116d8..a4b61dcd6 100644
--- a/pkg/commands/patch/patch_manager.go
+++ b/pkg/commands/patch/patch_manager.go
@@ -162,7 +162,7 @@ func (p *PatchManager) RemoveFileLineRange(filename string, firstLineIdx, lastLi
 	return nil
 }
 
-func (p *PatchManager) renderPlainPatchForFile(filename string, reverse bool, keepOriginalHeader bool) string {
+func (p *PatchManager) renderPlainPatchForFile(filename string, reverse bool, willBeAppliedReverse bool, keepOriginalHeader bool) string {
 	info, err := p.getFileInfo(filename)
 	if err != nil {
 		p.Log.Error(err)
@@ -177,14 +177,18 @@ func (p *PatchManager) renderPlainPatchForFile(filename string, reverse bool, ke
 	case PART:
 		// generate a new diff with just the selected lines
 		return ModifiedPatchForLines(p.Log, filename, info.diff, info.includedLineIndices,
-			PatchOptions{Reverse: reverse, KeepOriginalHeader: keepOriginalHeader})
+			PatchOptions{
+				Reverse:              reverse,
+				WillBeAppliedReverse: willBeAppliedReverse,
+				KeepOriginalHeader:   keepOriginalHeader,
+			})
 	default:
 		return ""
 	}
 }
 
-func (p *PatchManager) RenderPatchForFile(filename string, plain bool, reverse bool, keepOriginalHeader bool) string {
-	patch := p.renderPlainPatchForFile(filename, reverse, keepOriginalHeader)
+func (p *PatchManager) RenderPatchForFile(filename string, plain bool, reverse bool, willBeAppliedReverse bool, keepOriginalHeader bool) string {
+	patch := p.renderPlainPatchForFile(filename, reverse, willBeAppliedReverse, keepOriginalHeader)
 	if plain {
 		return patch
 	}
@@ -200,7 +204,7 @@ func (p *PatchManager) renderEachFilePatch(plain bool) []string {
 
 	sort.Strings(filenames)
 	patches := slices.Map(filenames, func(filename string) string {
-		return p.RenderPatchForFile(filename, plain, false, true)
+		return p.RenderPatchForFile(filename, plain, false, false, true)
 	})
 	output := slices.Filter(patches, func(patch string) bool {
 		return patch != ""
@@ -241,27 +245,20 @@ func (p *PatchManager) GetFileIncLineIndices(filename string) ([]int, error) {
 }
 
 func (p *PatchManager) ApplyPatches(reverse bool) error {
-	// for whole patches we'll apply the patch in reverse
-	// but for part patches we'll apply a reverse patch forwards
+	applyFlags := []string{"index", "3way"}
+	if reverse {
+		applyFlags = append(applyFlags, "reverse")
+	}
+
 	for filename, info := range p.fileInfoMap {
 		if info.mode == UNSELECTED {
 			continue
 		}
 
-		applyFlags := []string{"index", "3way"}
-		reverseOnGenerate := false
-		if reverse {
-			if info.mode == WHOLE {
-				applyFlags = append(applyFlags, "reverse")
-			} else {
-				reverseOnGenerate = true
-			}
-		}
-
 		var err error
 		// first run we try with the original header, then without
 		for _, keepOriginalHeader := range []bool{true, false} {
-			patch := p.RenderPatchForFile(filename, true, reverseOnGenerate, keepOriginalHeader)
+			patch := p.RenderPatchForFile(filename, true, false, reverse, keepOriginalHeader)
 			if patch == "" {
 				continue
 			}
diff --git a/pkg/commands/patch/patch_modifier.go b/pkg/commands/patch/patch_modifier.go
index fa20c7917..985743b76 100644
--- a/pkg/commands/patch/patch_modifier.go
+++ b/pkg/commands/patch/patch_modifier.go
@@ -96,6 +96,10 @@ func NewPatchModifier(log *logrus.Entry, filename string, diffText string) *Patc
 }
 
 func (d *PatchModifier) ModifiedPatchForLines(lineIndices []int, opts PatchOptions) string {
+	if opts.Reverse && opts.KeepOriginalHeader {
+		panic("reverse and keepOriginalHeader are not compatible")
+	}
+
 	// step one is getting only those hunks which we care about
 	hunksInRange := []*PatchHunk{}
 outer:
diff --git a/pkg/gui/refresh.go b/pkg/gui/refresh.go
index 9ee5ea0bd..60ca0accb 100644
--- a/pkg/gui/refresh.go
+++ b/pkg/gui/refresh.go
@@ -672,7 +672,7 @@ func (gui *Gui) refreshPatchBuildingPanel(opts types.OnFocusOpts) error {
 		return err
 	}
 
-	secondaryDiff := gui.git.Patch.PatchManager.RenderPatchForFile(path, false, false, true)
+	secondaryDiff := gui.git.Patch.PatchManager.RenderPatchForFile(path, false, false, false, true)
 	if err != nil {
 		return err
 	}