Resolve non-inline merge conflicts (#4431)

- **PR Description**

We offer an inline merge conflict editor for text conflicts (i.e. where
both sides modify the same section of code). However, there are other
types of conflicts that can't be resolved this way, for example when one
side modifies a file and the other side deletes it. For these cases we
would previously only show `* Unmerged path` in the main view, which
isn't helpful. (Also, for some of these we would split the main view and
show this text both in the unstaged changes and staged changes views,
which is a bit embarrassing.) But more importantly, it was very unclear
how to resolve such a conflict. The only option we had was to discard
the file, which would basically pick "ours" and discard "theirs"; but
there was no way to do the opposite.

This PR improves the situation in two ways:
- it shows elaborate help texts in the main view explaining the
situation (which, in some obscure cases, can be extremely non-obvious,
and a `git status` on the command line doesn't help at all either). For
`UD` and `DU` conflicts we also show the diff of the side that didn't
delete the file; this is usually essential for resolving the conflict
properly, because it's likely that this diff needs to be applied
manually somewhere else.
- when pressing enter, we show a dialog with options to keep the
modified file or to delete it.


![image](https://github.com/user-attachments/assets/568cdaa9-0945-4111-aa99-4bbbf465bf8b)

A note about terminology: a common way to describe the two sides of a
merge is "ours" and "theirs". I dislike these terms, because while they
work well for merges, they are backwards [for
rebases](https://git-scm.com/docs/git-checkout#Documentation/git-checkout.txt---ours).
I chose to avoid them in this PR, and to use the terms "current" and
"incoming" instead (like in the conflict code lenses in VS Code), which
I think work much better in general; however, they might not be easy to
understand when they occur in the middle of a sentence, so maybe we
should put them in quotes there.

- **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)
* [x] Text is internationalised (see
[here](https://github.com/jesseduffield/lazygit/blob/master/CONTRIBUTING.md#internationalisation))
* [ ] If a new UserConfig entry was added, make sure it can be
hot-reloaded (see
[here](https://github.com/jesseduffield/lazygit/blob/master/docs/dev/Codebase_Guide.md#using-userconfig))
* [ ] 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 2025-04-09 10:29:47 +02:00 committed by GitHub
commit 9469037cdd
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 239 additions and 4 deletions

View file

@ -341,6 +341,13 @@ func (self *WorkingTreeCommands) RemoveTrackedFiles(name string) error {
return self.cmd.New(cmdArgs).Run() return self.cmd.New(cmdArgs).Run()
} }
func (self *WorkingTreeCommands) RemoveConflictedFile(name string) error {
cmdArgs := NewGitCmd("rm").Arg("--", name).
ToArgv()
return self.cmd.New(cmdArgs).Run()
}
// RemoveUntrackedFiles runs `git clean -fd` // RemoveUntrackedFiles runs `git clean -fd`
func (self *WorkingTreeCommands) RemoveUntrackedFiles() error { func (self *WorkingTreeCommands) RemoveUntrackedFiles() error {
cmdArgs := NewGitCmd("clean").Arg("-fd").ToArgv() cmdArgs := NewGitCmd("clean").Arg("-fd").ToArgv()

View file

@ -1,6 +1,7 @@
package models package models
import ( import (
"github.com/jesseduffield/lazygit/pkg/i18n"
"github.com/jesseduffield/lazygit/pkg/utils" "github.com/jesseduffield/lazygit/pkg/utils"
"github.com/samber/lo" "github.com/samber/lo"
) )
@ -101,6 +102,22 @@ func (f *File) GetIsFile() bool {
return true return true
} }
func (f *File) GetMergeStateDescription(tr *i18n.TranslationSet) string {
m := map[string]string{
"DD": tr.MergeConflictDescription_DD,
"AU": tr.MergeConflictDescription_AU,
"UA": tr.MergeConflictDescription_UA,
"DU": tr.MergeConflictDescription_DU,
"UD": tr.MergeConflictDescription_UD,
}
if description, ok := m[f.ShortStatus]; ok {
return description
}
panic("should only be called if there's a merge conflict")
}
type StatusFields struct { type StatusFields struct {
HasStagedChanges bool HasStagedChanges bool
HasUnstagedChanges bool HasUnstagedChanges bool

View file

@ -268,6 +268,38 @@ func (self *FilesController) GetOnRenderToMain() func() {
self.c.Helpers().MergeConflicts.Render() self.c.Helpers().MergeConflicts.Render()
return return
} }
} else if node.File != nil && node.File.HasMergeConflicts {
opts := types.RefreshMainOpts{
Pair: self.c.MainViewPairs().Normal,
Main: &types.ViewUpdateOpts{
Title: self.c.Tr.DiffTitle,
SubTitle: self.c.Helpers().Diff.IgnoringWhitespaceSubTitle(),
},
}
message := node.File.GetMergeStateDescription(self.c.Tr)
message += "\n\n" + fmt.Sprintf(self.c.Tr.MergeConflictPressEnterToResolve,
self.c.UserConfig().Keybinding.Universal.GoInto)
if self.c.Views().Main.InnerWidth() > 70 {
// If the main view is very wide, wrap the message to increase readability
lines, _, _ := utils.WrapViewLinesToWidth(true, false, message, 70, 4)
message = strings.Join(lines, "\n")
}
if node.File.ShortStatus == "DU" || node.File.ShortStatus == "UD" {
cmdObj := self.c.Git().Diff.DiffCmdObj([]string{"--base", "--", node.GetPath()})
task := types.NewRunPtyTask(cmdObj.GetCmd())
task.Prefix = message + "\n\n"
if node.File.ShortStatus == "DU" {
task.Prefix += self.c.Tr.MergeConflictIncomingDiff
} else {
task.Prefix += self.c.Tr.MergeConflictCurrentDiff
}
task.Prefix += "\n\n"
opts.Main.Task = task
} else {
opts.Main.Task = types.NewRenderStringTask(message)
}
self.c.RenderToMainViews(opts)
return
} }
self.c.Helpers().MergeConflicts.ResetMergeState() self.c.Helpers().MergeConflicts.ResetMergeState()
@ -539,13 +571,59 @@ func (self *FilesController) EnterFile(opts types.OnFocusOpts) error {
return self.switchToMerge() return self.switchToMerge()
} }
if file.HasMergeConflicts { if file.HasMergeConflicts {
return errors.New(self.c.Tr.FileStagingRequirements) return self.handleNonInlineConflict(file)
} }
self.c.Context().Push(self.c.Contexts().Staging, opts) self.c.Context().Push(self.c.Contexts().Staging, opts)
return nil return nil
} }
func (self *FilesController) handleNonInlineConflict(file *models.File) error {
handle := func(command func(command string) error, logText string) error {
self.c.LogAction(logText)
if err := command(file.GetPath()); err != nil {
return err
}
return self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.FILES}})
}
keepItem := &types.MenuItem{
Label: self.c.Tr.MergeConflictKeepFile,
OnPress: func() error {
return handle(self.c.Git().WorkingTree.StageFile, self.c.Tr.Actions.ResolveConflictByKeepingFile)
},
Key: 'k',
}
deleteItem := &types.MenuItem{
Label: self.c.Tr.MergeConflictDeleteFile,
OnPress: func() error {
return handle(self.c.Git().WorkingTree.RemoveConflictedFile, self.c.Tr.Actions.ResolveConflictByDeletingFile)
},
Key: 'd',
}
items := []*types.MenuItem{}
switch file.ShortStatus {
case "DD":
// For "both deleted" conflicts, deleting the file is the only reasonable thing you can do.
// Restoring to the state before deletion is not the responsibility of a conflict resolution tool.
items = append(items, deleteItem)
case "DU", "UD":
// For these, we put the delete option first because it's the most common one,
// even if it's more destructive.
items = append(items, deleteItem, keepItem)
case "AU", "UA":
// For these, we put the keep option first because it's less destructive,
// and the chances between keep and delete are 50/50.
items = append(items, keepItem, deleteItem)
default:
panic("should only be called if there's a merge conflict")
}
return self.c.Menu(types.CreateMenuOptions{
Title: self.c.Tr.MergeConflictsTitle,
Prompt: file.GetMergeStateDescription(self.c.Tr),
Items: items,
})
}
func (self *FilesController) toggleStagedAll() error { func (self *FilesController) toggleStagedAll() error {
if err := self.toggleStagedAllWithLock(); err != nil { if err := self.toggleStagedAllWithLock(); err != nil {
return err return err

View file

@ -99,6 +99,16 @@ type TranslationSet struct {
FilterLabelUntrackedFiles string FilterLabelUntrackedFiles string
FilterLabelConflictingFiles string FilterLabelConflictingFiles string
MergeConflictsTitle string MergeConflictsTitle string
MergeConflictDescription_DD string
MergeConflictDescription_AU string
MergeConflictDescription_UA string
MergeConflictDescription_DU string
MergeConflictDescription_UD string
MergeConflictIncomingDiff string
MergeConflictCurrentDiff string
MergeConflictPressEnterToResolve string
MergeConflictKeepFile string
MergeConflictDeleteFile string
Checkout string Checkout string
CheckoutTooltip string CheckoutTooltip string
CantCheckoutBranchWhilePulling string CantCheckoutBranchWhilePulling string
@ -943,6 +953,8 @@ type Actions struct {
UnstageFile string UnstageFile string
UnstageAllFiles string UnstageAllFiles string
StageAllFiles string StageAllFiles string
ResolveConflictByKeepingFile string
ResolveConflictByDeletingFile string
NotEnoughContextToStage string NotEnoughContextToStage string
NotEnoughContextToDiscard string NotEnoughContextToDiscard string
IgnoreExcludeFile string IgnoreExcludeFile string
@ -1112,6 +1124,16 @@ func EnglishTranslationSet() *TranslationSet {
PullTooltip: "Pull changes from the remote for the current branch. If no upstream is configured, you will be prompted to configure an upstream branch.", PullTooltip: "Pull changes from the remote for the current branch. If no upstream is configured, you will be prompted to configure an upstream branch.",
Scroll: "Scroll", Scroll: "Scroll",
MergeConflictsTitle: "Merge conflicts", MergeConflictsTitle: "Merge conflicts",
MergeConflictDescription_DD: "Conflict: this file was moved or renamed both in the current and the incoming changes, but to different destinations. I don't know which ones, but they should both show up as conflicts too (marked 'AU' and 'UA', respectively). The most likely resolution is to delete this file, and pick one of the destinations and delete the other.",
MergeConflictDescription_AU: "Conflict: this file is the destination of a move or rename in the current changes, but was moved or renamed to a different destination in the incoming changes. That other destination should also show up as a conflict (marked 'UA'), as well as the file that both were renamed from (marked 'DD').",
MergeConflictDescription_UA: "Conflict: this file is the destination of a move or rename in the incoming changes, but was moved or renamed to a different destination in the current changes. That other destination should also show up as a conflict (marked 'AU'), as well as the file that both were renamed from (marked 'DD').",
MergeConflictDescription_DU: "Conflict: this file was deleted in the current changes and modified in the incoming changes.\n\nThe most likely resolution is to delete the file after applying the incoming modifications manually to some other place in the code.",
MergeConflictDescription_UD: "Conflict: this file was modified in the current changes and deleted in incoming changes.\n\nThe most likely resolution is to delete the file after applying the current modifications manually to some other place in the code.",
MergeConflictIncomingDiff: "Incoming changes:",
MergeConflictCurrentDiff: "Current changes:",
MergeConflictPressEnterToResolve: "Press %s to resolve.",
MergeConflictKeepFile: "Keep file",
MergeConflictDeleteFile: "Delete file",
Checkout: "Checkout", Checkout: "Checkout",
CheckoutTooltip: "Checkout selected item.", CheckoutTooltip: "Checkout selected item.",
CantCheckoutBranchWhilePulling: "You cannot checkout another branch while pulling the current branch", CantCheckoutBranchWhilePulling: "You cannot checkout another branch while pulling the current branch",
@ -1952,6 +1974,8 @@ func EnglishTranslationSet() *TranslationSet {
UnstageFile: "Unstage file", UnstageFile: "Unstage file",
UnstageAllFiles: "Unstage all files", UnstageAllFiles: "Unstage all files",
StageAllFiles: "Stage all files", StageAllFiles: "Stage all files",
ResolveConflictByKeepingFile: "Resolve by keeping file",
ResolveConflictByDeletingFile: "Resolve by deleting file",
NotEnoughContextToStage: "Staging or unstaging changes is not possible with a diff context size of 0. Increase the context using '%s'.", NotEnoughContextToStage: "Staging or unstaging changes is not possible with a diff context size of 0. Increase the context using '%s'.",
NotEnoughContextToDiscard: "Discarding changes is not possible with a diff context size of 0. Increase the context using '%s'.", NotEnoughContextToDiscard: "Discarding changes is not possible with a diff context size of 0. Increase the context using '%s'.",
IgnoreExcludeFile: "Ignore or exclude file", IgnoreExcludeFile: "Ignore or exclude file",

View file

@ -10,23 +10,25 @@ type FileSystem struct {
} }
// This does _not_ check the files panel, it actually checks the filesystem // This does _not_ check the files panel, it actually checks the filesystem
func (self *FileSystem) PathPresent(path string) { func (self *FileSystem) PathPresent(path string) *FileSystem {
self.assertWithRetries(func() (bool, string) { self.assertWithRetries(func() (bool, string) {
_, err := os.Stat(path) _, err := os.Stat(path)
return err == nil, fmt.Sprintf("Expected path '%s' to exist, but it does not", path) return err == nil, fmt.Sprintf("Expected path '%s' to exist, but it does not", path)
}) })
return self
} }
// This does _not_ check the files panel, it actually checks the filesystem // This does _not_ check the files panel, it actually checks the filesystem
func (self *FileSystem) PathNotPresent(path string) { func (self *FileSystem) PathNotPresent(path string) *FileSystem {
self.assertWithRetries(func() (bool, string) { self.assertWithRetries(func() (bool, string) {
_, err := os.Stat(path) _, err := os.Stat(path)
return os.IsNotExist(err), fmt.Sprintf("Expected path '%s' to not exist, but it does", path) return os.IsNotExist(err), fmt.Sprintf("Expected path '%s' to not exist, but it does", path)
}) })
return self
} }
// Asserts that the file at the given path has the given content // Asserts that the file at the given path has the given content
func (self *FileSystem) FileContent(path string, matcher *TextMatcher) { func (self *FileSystem) FileContent(path string, matcher *TextMatcher) *FileSystem {
self.assertWithRetries(func() (bool, string) { self.assertWithRetries(func() (bool, string) {
_, err := os.Stat(path) _, err := os.Stat(path)
if os.IsNotExist(err) { if os.IsNotExist(err) {
@ -46,4 +48,5 @@ func (self *FileSystem) FileContent(path string, matcher *TextMatcher) {
return true, "" return true, ""
}) })
return self
} }

View file

@ -0,0 +1,105 @@
package conflicts
import (
"github.com/jesseduffield/lazygit/pkg/config"
. "github.com/jesseduffield/lazygit/pkg/integration/components"
)
var ResolveNonTextualConflicts = NewIntegrationTest(NewIntegrationTestArgs{
Description: "Resolve non-textual merge conflicts (e.g. one side modified, the other side deleted)",
ExtraCmdArgs: []string{},
Skip: false,
SetupConfig: func(config *config.AppConfig) {},
SetupRepo: func(shell *Shell) {
shell.RunShellCommand(`echo test1 > both-deleted1.txt`)
shell.RunShellCommand(`echo test2 > both-deleted2.txt`)
shell.RunShellCommand(`git checkout -b conflict && git add both-deleted1.txt both-deleted2.txt`)
shell.RunShellCommand(`echo haha1 > deleted-them1.txt && git add deleted-them1.txt`)
shell.RunShellCommand(`echo haha2 > deleted-them2.txt && git add deleted-them2.txt`)
shell.RunShellCommand(`echo haha1 > deleted-us1.txt && git add deleted-us1.txt`)
shell.RunShellCommand(`echo haha2 > deleted-us2.txt && git add deleted-us2.txt`)
shell.RunShellCommand(`git commit -m one`)
// stuff on other branch
shell.RunShellCommand(`git branch conflict_second`)
shell.RunShellCommand(`git mv both-deleted1.txt added-them-changed-us1.txt`)
shell.RunShellCommand(`git mv both-deleted2.txt added-them-changed-us2.txt`)
shell.RunShellCommand(`git rm deleted-them1.txt deleted-them2.txt`)
shell.RunShellCommand(`echo modded1 > deleted-us1.txt && git add deleted-us1.txt`)
shell.RunShellCommand(`echo modded2 > deleted-us2.txt && git add deleted-us2.txt`)
shell.RunShellCommand(`git commit -m "two"`)
// stuff on our branch
shell.RunShellCommand(`git checkout conflict_second`)
shell.RunShellCommand(`git mv both-deleted1.txt changed-them-added-us1.txt`)
shell.RunShellCommand(`git mv both-deleted2.txt changed-them-added-us2.txt`)
shell.RunShellCommand(`echo modded1 > deleted-them1.txt && git add deleted-them1.txt`)
shell.RunShellCommand(`echo modded2 > deleted-them2.txt && git add deleted-them2.txt`)
shell.RunShellCommand(`git rm deleted-us1.txt deleted-us2.txt`)
shell.RunShellCommand(`git commit -m "three"`)
shell.RunShellCommand(`git reset --hard conflict_second`)
shell.RunCommandExpectError([]string{"git", "merge", "conflict"})
},
Run: func(t *TestDriver, keys config.KeybindingConfig) {
resolve := func(filename string, menuChoice string) {
t.Views().Files().
NavigateToLine(Contains(filename)).
Tap(func() {
t.Views().Main().Content(Contains("Conflict:"))
}).
Press(keys.Universal.GoInto).
Tap(func() {
t.ExpectPopup().Menu().Title(Equals("Merge conflicts")).
Select(Contains(menuChoice)).
Confirm()
})
}
t.Views().Files().
IsFocused().
Lines(
Equals("▼ /").IsSelected(),
Equals(" UA added-them-changed-us1.txt"),
Equals(" UA added-them-changed-us2.txt"),
Equals(" DD both-deleted1.txt"),
Equals(" DD both-deleted2.txt"),
Equals(" AU changed-them-added-us1.txt"),
Equals(" AU changed-them-added-us2.txt"),
Equals(" UD deleted-them1.txt"),
Equals(" UD deleted-them2.txt"),
Equals(" DU deleted-us1.txt"),
Equals(" DU deleted-us2.txt"),
).
Tap(func() {
resolve("added-them-changed-us1.txt", "Delete file")
resolve("added-them-changed-us2.txt", "Keep file")
resolve("both-deleted1.txt", "Delete file")
resolve("both-deleted2.txt", "Delete file")
resolve("changed-them-added-us1.txt", "Delete file")
resolve("changed-them-added-us2.txt", "Keep file")
resolve("deleted-them1.txt", "Delete file")
resolve("deleted-them2.txt", "Keep file")
resolve("deleted-us1.txt", "Delete file")
resolve("deleted-us2.txt", "Keep file")
}).
Lines(
Equals("▼ /"),
Equals(" A added-them-changed-us2.txt"),
Equals(" D changed-them-added-us1.txt"),
Equals(" D deleted-them1.txt"),
Equals(" A deleted-us2.txt"),
)
t.FileSystem().
PathNotPresent("added-them-changed-us1.txt").
FileContent("added-them-changed-us2.txt", Equals("test2\n")).
PathNotPresent("both-deleted1.txt").
PathNotPresent("both-deleted2.txt").
PathNotPresent("changed-them-added-us1.txt").
FileContent("changed-them-added-us2.txt", Equals("test2\n")).
PathNotPresent("deleted-them1.txt").
FileContent("deleted-them2.txt", Equals("modded2\n")).
PathNotPresent("deleted-us1.txt").
FileContent("deleted-us2.txt", Equals("modded2\n"))
},
})

View file

@ -141,6 +141,7 @@ var tests = []*components.IntegrationTest{
conflicts.ResolveExternally, conflicts.ResolveExternally,
conflicts.ResolveMultipleFiles, conflicts.ResolveMultipleFiles,
conflicts.ResolveNoAutoStage, conflicts.ResolveNoAutoStage,
conflicts.ResolveNonTextualConflicts,
conflicts.ResolveWithoutTrailingLf, conflicts.ResolveWithoutTrailingLf,
conflicts.UndoChooseHunk, conflicts.UndoChooseHunk,
custom_commands.AccessCommitProperties, custom_commands.AccessCommitProperties,