Disallow cherry-picking merge commits (#3316)

Cherry-picking merge commits is currently not supported because of the
way copy/pasting is currently implemented. Disable the command with a
proper error message if the user tries to copy a merge commit, instead
of running into the confusing error message that git would otherwise
give, see #1374.

While we're at it, disable it also when trying to copy an "update-ref"
todo, which doesn't make sense.

pkg/gui/controllers/basic_commits_controller.go

@@ -84,9 +84,10 @@ func (self *BasicCommitsController) GetKeybindings(opts types.KeybindingsOpts) [
 			DisplayOnScreen:   true,
 		},
 		{
-			Key:         opts.GetKey(opts.Config.Commits.CherryPickCopy),
-			Handler:     self.withItem(self.copyRange),
-			Description: self.c.Tr.CherryPickCopy,
+			Key:               opts.GetKey(opts.Config.Commits.CherryPickCopy),
+			Handler:           self.withItem(self.copyRange),
+			GetDisabledReason: self.require(self.itemRangeSelected(self.canCopyCommits)),
+			Description:       self.c.Tr.CherryPickCopy,
 			Tooltip: utils.ResolvePlaceholderString(self.c.Tr.CherryPickCopyTooltip,
 				map[string]string{
 					"paste":  keybindings.Label(opts.Config.Commits.PasteCommits),
@@ -292,6 +293,20 @@ func (self *BasicCommitsController) copyRange(*models.Commit) error {
 	return self.c.Helpers().CherryPick.CopyRange(self.context.GetCommits(), self.context)
 }
 
+func (self *BasicCommitsController) canCopyCommits(selectedCommits []*models.Commit, startIdx int, endIdx int) *types.DisabledReason {
+	for _, commit := range selectedCommits {
+		if commit.Sha == "" {
+			return &types.DisabledReason{Text: self.c.Tr.CannotCherryPickNonCommit, ShowErrorInPanel: true}
+		}
+
+		if commit.IsMerge() {
+			return &types.DisabledReason{Text: self.c.Tr.CannotCherryPickMergeCommit, ShowErrorInPanel: true}
+		}
+	}
+
+	return nil
+}
+
 func (self *BasicCommitsController) handleOldCherryPickKey() error {
 	msg := utils.ResolvePlaceholderString(self.c.Tr.OldCherryPickKeyWarning,
 		map[string]string{

pkg/gui/controllers/helpers/cherry_pick_helper.go

@@ -31,21 +31,6 @@ func (self *CherryPickHelper) getData() *cherrypicking.CherryPicking {
 	return self.c.Modes().CherryPicking
 }
 
-func (self *CherryPickHelper) Copy(commit *models.Commit, commitsList []*models.Commit, context types.Context) error {
-	if err := self.resetIfNecessary(context); err != nil {
-		return err
-	}
-
-	// we will un-copy it if it's already copied
-	if self.getData().SelectedShaSet().Includes(commit.Sha) {
-		self.getData().Remove(commit, commitsList)
-	} else {
-		self.getData().Add(commit, commitsList)
-	}
-
-	return self.rerender()
-}
-
 func (self *CherryPickHelper) CopyRange(commitsList []*models.Commit, context types.IListContext) error {
 	startIdx, endIdx := context.GetList().GetSelectionRange()
 

pkg/i18n/english.go

@@ -302,6 +302,8 @@ type TranslationSet struct {
 	PasteCommits                          string
 	SureCherryPick                        string
 	CherryPick                            string
+	CannotCherryPickNonCommit             string
+	CannotCherryPickMergeCommit           string
 	Donate                                string
 	AskQuestion                           string
 	PrevLine                              string
@@ -1242,6 +1244,8 @@ func EnglishTranslationSet() TranslationSet {
 		PasteCommits:                        "Paste (cherry-pick)",
 		SureCherryPick:                      "Are you sure you want to cherry-pick the copied commits onto this branch?",
 		CherryPick:                          "Cherry-pick",
+		CannotCherryPickNonCommit:           "Cannot cherry-pick this kind of todo item",
+		CannotCherryPickMergeCommit:         "Cherry-picking merge commits is not supported",
 		Donate:                              "Donate",
 		AskQuestion:                         "Ask Question",
 		PrevLine:                            "Select previous line",