Skip to content

Resolve non-inline merge conflicts #4431

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions pkg/commands/git_commands/working_tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,13 @@ func (self *WorkingTreeCommands) RemoveTrackedFiles(name string) error {
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`
func (self *WorkingTreeCommands) RemoveUntrackedFiles() error {
cmdArgs := NewGitCmd("clean").Arg("-fd").ToArgv()
Expand Down
17 changes: 17 additions & 0 deletions pkg/commands/models/file.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package models

import (
"github.com/jesseduffield/lazygit/pkg/i18n"
"github.com/jesseduffield/lazygit/pkg/utils"
"github.com/samber/lo"
)
Expand Down Expand Up @@ -101,6 +102,22 @@ func (f *File) GetIsFile() bool {
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 {
HasStagedChanges bool
HasUnstagedChanges bool
Expand Down
80 changes: 79 additions & 1 deletion pkg/gui/controllers/files_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,38 @@ func (self *FilesController) GetOnRenderToMain() func() {
self.c.Helpers().MergeConflicts.Render()
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()
Expand Down Expand Up @@ -539,13 +571,59 @@ func (self *FilesController) EnterFile(opts types.OnFocusOpts) error {
return self.switchToMerge()
}
if file.HasMergeConflicts {
return errors.New(self.c.Tr.FileStagingRequirements)
return self.handleNonInlineConflict(file)
}

self.c.Context().Push(self.c.Contexts().Staging, opts)
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)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have put them both in the same order so that you can use muscle memory to make the choice. Given that this second one is 50/50, why not just retain the order of the first one? i.e. always have delete first.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found it more important not to have a destructive option as a default. And these AU/UA conflicts are so rare and probably so confusing that you won't be able to operate the dialog quickly and using muscle memory without looking. You'll have to read the choices carefully.

I'd prefer to keep the order this way, but I can also change it if you insist.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright fair, let's keep it as-is

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 {
if err := self.toggleStagedAllWithLock(); err != nil {
return err
Expand Down
24 changes: 24 additions & 0 deletions pkg/i18n/english.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,16 @@ type TranslationSet struct {
FilterLabelUntrackedFiles string
FilterLabelConflictingFiles 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
CheckoutTooltip string
CantCheckoutBranchWhilePulling string
Expand Down Expand Up @@ -943,6 +953,8 @@ type Actions struct {
UnstageFile string
UnstageAllFiles string
StageAllFiles string
ResolveConflictByKeepingFile string
ResolveConflictByDeletingFile string
NotEnoughContextToStage string
NotEnoughContextToDiscard string
IgnoreExcludeFile string
Expand Down Expand Up @@ -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.",
Scroll: "Scroll",
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",
CheckoutTooltip: "Checkout selected item.",
CantCheckoutBranchWhilePulling: "You cannot checkout another branch while pulling the current branch",
Expand Down Expand Up @@ -1952,6 +1974,8 @@ func EnglishTranslationSet() *TranslationSet {
UnstageFile: "Unstage file",
UnstageAllFiles: "Unstage 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'.",
NotEnoughContextToDiscard: "Discarding changes is not possible with a diff context size of 0. Increase the context using '%s'.",
IgnoreExcludeFile: "Ignore or exclude file",
Expand Down
9 changes: 6 additions & 3 deletions pkg/integration/components/file_system.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,23 +10,25 @@ type FileSystem struct {
}

// 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) {
_, err := os.Stat(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
func (self *FileSystem) PathNotPresent(path string) {
func (self *FileSystem) PathNotPresent(path string) *FileSystem {
self.assertWithRetries(func() (bool, string) {
_, err := os.Stat(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
func (self *FileSystem) FileContent(path string, matcher *TextMatcher) {
func (self *FileSystem) FileContent(path string, matcher *TextMatcher) *FileSystem {
self.assertWithRetries(func() (bool, string) {
_, err := os.Stat(path)
if os.IsNotExist(err) {
Expand All @@ -46,4 +48,5 @@ func (self *FileSystem) FileContent(path string, matcher *TextMatcher) {

return true, ""
})
return self
}
105 changes: 105 additions & 0 deletions pkg/integration/tests/conflicts/resolve_non_textual_conflicts.go
Original file line number Diff line number Diff line change
@@ -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"))
},
})
1 change: 1 addition & 0 deletions pkg/integration/tests/test_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ var tests = []*components.IntegrationTest{
conflicts.ResolveExternally,
conflicts.ResolveMultipleFiles,
conflicts.ResolveNoAutoStage,
conflicts.ResolveNonTextualConflicts,
conflicts.ResolveWithoutTrailingLf,
conflicts.UndoChooseHunk,
custom_commands.AccessCommitProperties,
Expand Down