From 2e1be459574cf17e250f26e35939b3a122e3b2aa Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 26 Mar 2025 18:26:18 +0100 Subject: [PATCH 1/3] Better main view display for conflicing files For the less common conflict types DD, AU, UA, DU, and UD, 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. Improve this by offering more explanation about what's going on, and what the most likely way to resolve the situation is for each case. --- pkg/commands/models/file.go | 17 +++++++++++++ pkg/gui/controllers/files_controller.go | 32 +++++++++++++++++++++++++ pkg/i18n/english.go | 16 +++++++++++++ 3 files changed, 65 insertions(+) diff --git a/pkg/commands/models/file.go b/pkg/commands/models/file.go index 561399820..e48696a4f 100644 --- a/pkg/commands/models/file.go +++ b/pkg/commands/models/file.go @@ -1,6 +1,7 @@ package models import ( + "github.com/jesseduffield/lazygit/pkg/i18n" "github.com/jesseduffield/lazygit/pkg/utils" "github.com/samber/lo" ) @@ -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 diff --git a/pkg/gui/controllers/files_controller.go b/pkg/gui/controllers/files_controller.go index f8e6061b3..cf3d0475e 100644 --- a/pkg/gui/controllers/files_controller.go +++ b/pkg/gui/controllers/files_controller.go @@ -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() diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index 0478b114a..eeaf4f2e9 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -99,6 +99,14 @@ 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 Checkout string CheckoutTooltip string CantCheckoutBranchWhilePulling string @@ -1112,6 +1120,14 @@ 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.", Checkout: "Checkout", CheckoutTooltip: "Checkout selected item.", CantCheckoutBranchWhilePulling: "You cannot checkout another branch while pulling the current branch", From efcd71b29644b77ef642e6123679e3f10dc88052 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 27 Mar 2025 17:01:35 +0100 Subject: [PATCH 2/3] Allow chaining of FileSystem methods --- pkg/integration/components/file_system.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/integration/components/file_system.go b/pkg/integration/components/file_system.go index feea9a5b1..194125ec6 100644 --- a/pkg/integration/components/file_system.go +++ b/pkg/integration/components/file_system.go @@ -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) { @@ -46,4 +48,5 @@ func (self *FileSystem) FileContent(path string, matcher *TextMatcher) { return true, "" }) + return self } From ebb576feac4081a8050c80e98a13e8f63abb201b Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 27 Mar 2025 15:31:45 +0100 Subject: [PATCH 3/3] Provide conflict resolution dialogs for non-textual conflicts --- pkg/commands/git_commands/working_tree.go | 7 ++ pkg/gui/controllers/files_controller.go | 48 +++++++- pkg/i18n/english.go | 8 ++ .../resolve_non_textual_conflicts.go | 105 ++++++++++++++++++ pkg/integration/tests/test_list.go | 1 + 5 files changed, 168 insertions(+), 1 deletion(-) create mode 100644 pkg/integration/tests/conflicts/resolve_non_textual_conflicts.go diff --git a/pkg/commands/git_commands/working_tree.go b/pkg/commands/git_commands/working_tree.go index e1339ee56..c967ca576 100644 --- a/pkg/commands/git_commands/working_tree.go +++ b/pkg/commands/git_commands/working_tree.go @@ -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() diff --git a/pkg/gui/controllers/files_controller.go b/pkg/gui/controllers/files_controller.go index cf3d0475e..167e29f1b 100644 --- a/pkg/gui/controllers/files_controller.go +++ b/pkg/gui/controllers/files_controller.go @@ -571,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) + 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 diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index eeaf4f2e9..473ce50e8 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -107,6 +107,8 @@ type TranslationSet struct { MergeConflictIncomingDiff string MergeConflictCurrentDiff string MergeConflictPressEnterToResolve string + MergeConflictKeepFile string + MergeConflictDeleteFile string Checkout string CheckoutTooltip string CantCheckoutBranchWhilePulling string @@ -951,6 +953,8 @@ type Actions struct { UnstageFile string UnstageAllFiles string StageAllFiles string + ResolveConflictByKeepingFile string + ResolveConflictByDeletingFile string NotEnoughContextToStage string NotEnoughContextToDiscard string IgnoreExcludeFile string @@ -1128,6 +1132,8 @@ func EnglishTranslationSet() *TranslationSet { 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", @@ -1968,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", diff --git a/pkg/integration/tests/conflicts/resolve_non_textual_conflicts.go b/pkg/integration/tests/conflicts/resolve_non_textual_conflicts.go new file mode 100644 index 000000000..8601d0680 --- /dev/null +++ b/pkg/integration/tests/conflicts/resolve_non_textual_conflicts.go @@ -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")) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 0d6884071..2590e03ca 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -141,6 +141,7 @@ var tests = []*components.IntegrationTest{ conflicts.ResolveExternally, conflicts.ResolveMultipleFiles, conflicts.ResolveNoAutoStage, + conflicts.ResolveNonTextualConflicts, conflicts.ResolveWithoutTrailingLf, conflicts.UndoChooseHunk, custom_commands.AccessCommitProperties,