-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Add merge menu with conflict resolver #4889
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -5,6 +5,7 @@ import ( | |||||||
"os" | ||||||||
"path/filepath" | ||||||||
"regexp" | ||||||||
"strings" | ||||||||
|
||||||||
"github.com/go-errors/errors" | ||||||||
"github.com/jesseduffield/lazygit/pkg/commands/models" | ||||||||
|
@@ -407,3 +408,26 @@ func (self *WorkingTreeCommands) ResetMixed(ref string) error { | |||||||
|
||||||||
return self.cmd.New(cmdArgs).Run() | ||||||||
} | ||||||||
|
||||||||
func (self *WorkingTreeCommands) ObjectIDAtStage(path string, stage int) (string, error) { | ||||||||
cmdArgs := NewGitCmd("rev-parse"). | ||||||||
Arg(fmt.Sprintf(":%d:%s", stage, path)). | ||||||||
ToArgv() | ||||||||
|
||||||||
output, err := self.cmd.New(cmdArgs).RunWithOutput() | ||||||||
if err != nil { | ||||||||
return "", err | ||||||||
} | ||||||||
|
||||||||
return strings.TrimSpace(output), nil | ||||||||
} | ||||||||
|
||||||||
func (self *WorkingTreeCommands) MergeFile(strategy string, oursID string, baseID string, theirsID string) (string, error) { | ||||||||
cmdArgs := NewGitCmd("merge-file"). | ||||||||
Arg("--object-id"). | ||||||||
Arg(strategy). | ||||||||
Arg("-p", oursID, baseID, theirsID). | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpicks: would use
Suggested change
|
||||||||
ToArgv() | ||||||||
|
||||||||
return self.cmd.New(cmdArgs).RunWithOutput() | ||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ package controllers | |
import ( | ||
"errors" | ||
"fmt" | ||
"os" | ||
"path/filepath" | ||
"strings" | ||
|
||
|
@@ -12,6 +13,7 @@ import ( | |
"github.com/jesseduffield/lazygit/pkg/commands/models" | ||
"github.com/jesseduffield/lazygit/pkg/gui/context" | ||
"github.com/jesseduffield/lazygit/pkg/gui/filetree" | ||
"github.com/jesseduffield/lazygit/pkg/gui/style" | ||
"github.com/jesseduffield/lazygit/pkg/gui/types" | ||
"github.com/jesseduffield/lazygit/pkg/utils" | ||
"github.com/samber/lo" | ||
|
@@ -178,10 +180,12 @@ func (self *FilesController) GetKeybindings(opts types.KeybindingsOpts) []*types | |
Description: self.c.Tr.OpenDiffTool, | ||
}, | ||
{ | ||
Key: opts.GetKey(opts.Config.Files.OpenMergeTool), | ||
Handler: self.c.Helpers().WorkingTree.OpenMergeTool, | ||
Description: self.c.Tr.OpenMergeTool, | ||
Tooltip: self.c.Tr.OpenMergeToolTooltip, | ||
Key: opts.GetKey(opts.Config.Files.OpenMergeOptions), | ||
Handler: self.withItems(self.createMergeConflictMenu), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of implementing the menu here, we should implement it in Edit: hm, when you are in the merge conflict editor you probably expect to work only on the current file you are in. So we'd have to pass a list of file paths to that function, and pass the selected ones from here, and the current one from merge_conflicts_controller. |
||
Description: self.c.Tr.ViewMergeConflictOptions, | ||
Tooltip: self.c.Tr.ViewMergeConflictOptionsTooltip, | ||
OpensMenu: true, | ||
DisplayOnScreen: true, | ||
}, | ||
{ | ||
Key: opts.GetKey(opts.Config.Files.Fetch), | ||
|
@@ -1024,6 +1028,92 @@ func (self *FilesController) createStashMenu() error { | |
}) | ||
} | ||
|
||
func (self *FilesController) createMergeConflictMenu(nodes []*filetree.FileNode) error { | ||
onMergeStrategySelected := func(strategy string) error { | ||
normalizedNodes := normalisedSelectedNodes(nodes) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This relies on the assumption that all currently shown files have conflicts. This doesn't have to be true; while we automatically filter for only conflicted files when there are any, the user can open the filter menu (ctrl-b) and choose "no filter" to show all files. Which means we should probably filter on the status again here. Also, for files that have non-textual conflicts (see #4431), I guess the |
||
fileNodes := lo.Filter(normalizedNodes, func(node *filetree.FileNode, _ int) bool { | ||
return node.File != nil | ||
}) | ||
filenames := lo.Map(fileNodes, func(node *filetree.FileNode, _ int) string { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick: would call the variable |
||
return node.GetPath() | ||
}) | ||
|
||
for _, filename := range filenames { | ||
baseID, err := self.c.Git().WorkingTree.ObjectIDAtStage(filename, 1) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
oursID, err := self.c.Git().WorkingTree.ObjectIDAtStage(filename, 2) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
theirsID, err := self.c.Git().WorkingTree.ObjectIDAtStage(filename, 3) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
output, err := self.c.Git().WorkingTree.MergeFile(strategy, oursID, baseID, theirsID) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
if err = os.WriteFile(filename, []byte(output), 0o644); err != nil { | ||
return err | ||
} | ||
} | ||
|
||
err := self.c.Git().WorkingTree.StageFiles(filenames, nil) | ||
return self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(err) | ||
} | ||
|
||
cmdColor := style.FgBlue | ||
return self.c.Menu(types.CreateMenuOptions{ | ||
Title: self.c.Tr.MergeConflictOptionsTitle, | ||
Items: []*types.MenuItem{ | ||
{ | ||
LabelColumns: []string{ | ||
self.c.Tr.UseHead, | ||
cmdColor.Sprint("git merge-file --ours"), | ||
}, | ||
OnPress: func() error { | ||
return onMergeStrategySelected("--ours") | ||
}, | ||
Key: 'h', | ||
}, | ||
{ | ||
LabelColumns: []string{ | ||
self.c.Tr.UseIncoming, | ||
cmdColor.Sprint("git merge-file --theirs"), | ||
}, | ||
OnPress: func() error { | ||
return onMergeStrategySelected("--theirs") | ||
}, | ||
Key: 'i', | ||
}, | ||
{ | ||
LabelColumns: []string{ | ||
self.c.Tr.UseBoth, | ||
cmdColor.Sprint("git merge-file --union"), | ||
}, | ||
OnPress: func() error { | ||
return onMergeStrategySelected("--union") | ||
}, | ||
Key: 'b', | ||
}, | ||
{ | ||
LabelColumns: []string{ | ||
self.c.Tr.OpenMergeTool, | ||
cmdColor.Sprint("git mergetool"), | ||
}, | ||
OnPress: self.c.Helpers().WorkingTree.OpenMergeTool, | ||
Key: 'm', | ||
}, | ||
}, | ||
}) | ||
} | ||
|
||
func (self *FilesController) openCopyMenu() error { | ||
node := self.context().GetSelected() | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -898,8 +898,13 @@ type TranslationSet struct { | |
BreakingChangesTitle string | ||
BreakingChangesMessage string | ||
BreakingChangesByVersion map[string]string | ||
ViewMergeConflictOptions string | ||
ViewMergeConflictOptionsTooltip string | ||
MergeConflictOptionsTitle string | ||
UseHead string | ||
UseIncoming string | ||
UseBoth string | ||
} | ||
|
||
type Bisect struct { | ||
MarkStart string | ||
ResetTitle string | ||
|
@@ -1970,6 +1975,12 @@ func EnglishTranslationSet() *TranslationSet { | |
CustomCommands: "Custom commands", | ||
NoApplicableCommandsInThisContext: "(No applicable commands in this context)", | ||
SelectCommitsOfCurrentBranch: "Select commits of current branch", | ||
ViewMergeConflictOptions: "View merge conflict options", | ||
ViewMergeConflictOptionsTooltip: "View options for resolving merge conflicts.", | ||
MergeConflictOptionsTitle: "Resolve merge conflicts", | ||
UseHead: "Use HEAD", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For non-textual conflicts we settled on the terms "current changes" (for head) and "incoming changes"; see #4431, so I'd suggest to use those here, too. Also, we don't use Title Case in menu items, so it should be "Use current changes" and "Use both". |
||
UseIncoming: "Use Incoming", | ||
UseBoth: "Use Both", | ||
|
||
Actions: Actions{ | ||
// TODO: combine this with the original keybinding descriptions (those are all in lowercase atm) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a breaking change (users might have changed the keybinding in their config file). We have the technology to migrate users's config files to account for this, and we should do that by adding an entry to this array.