Merge pull request #2224 from Ryooooooga/ambiguous-branch

Author: jesseduffield

pkg/commands/git.go

@@ -119,9 +119,9 @@ func NewGitCommandAux(
 	patchCommands := git_commands.NewPatchCommands(gitCommon, rebaseCommands, commitCommands, statusCommands, stashCommands, patchManager)
 	bisectCommands := git_commands.NewBisectCommands(gitCommon)
 
-	branchLoader := git_commands.NewBranchLoader(cmn, branchCommands.GetRawBranches, branchCommands.CurrentBranchName, configCommands)
+	branchLoader := git_commands.NewBranchLoader(cmn, branchCommands.GetRawBranches, branchCommands.CurrentBranchInfo, configCommands)
 	commitFileLoader := git_commands.NewCommitFileLoader(cmn, cmd)
-	commitLoader := git_commands.NewCommitLoader(cmn, cmd, dotGitDir, branchCommands.CurrentBranchName, statusCommands.RebaseMode)
+	commitLoader := git_commands.NewCommitLoader(cmn, cmd, dotGitDir, branchCommands.CurrentBranchInfo, statusCommands.RebaseMode)
 	reflogCommitLoader := git_commands.NewReflogCommitLoader(cmn, cmd)
 	remoteLoader := git_commands.NewRemoteLoader(cmn, cmd, repo.Remotes)
 	stashLoader := git_commands.NewStashLoader(cmn, cmd)

pkg/commands/git_commands/branch.go

@@ -30,29 +30,39 @@ func (self *BranchCommands) New(name string, base string) error {
 	return self.cmd.New(fmt.Sprintf("git checkout -b %s %s", self.cmd.Quote(name), self.cmd.Quote(base))).Run()
 }
 
-// CurrentBranchName get the current branch name and displayname.
-// the first returned string is the name and the second is the displayname
-// e.g. name is 123asdf and displayname is '(HEAD detached at 123asdf)'
-func (self *BranchCommands) CurrentBranchName() (string, string, error) {
+// CurrentBranchInfo get the current branch information.
+func (self *BranchCommands) CurrentBranchInfo() (BranchInfo, error) {
 	branchName, err := self.cmd.New("git symbolic-ref --short HEAD").DontLog().RunWithOutput()
 	if err == nil && branchName != "HEAD\n" {
 		trimmedBranchName := strings.TrimSpace(branchName)
-		return trimmedBranchName, trimmedBranchName, nil
+		return BranchInfo{
+			RefName:      trimmedBranchName,
+			DisplayName:  trimmedBranchName,
+			DetachedHead: false,
+		}, nil
 	}
 	output, err := self.cmd.New("git branch --contains").DontLog().RunWithOutput()
 	if err != nil {
-		return "", "", err
+		return BranchInfo{}, err
 	}
 	for _, line := range utils.SplitLines(output) {
 		re := regexp.MustCompile(CurrentBranchNameRegex)
 		match := re.FindStringSubmatch(line)
 		if len(match) > 0 {
 			branchName = match[1]
 			displayBranchName := match[0][2:]
-			return branchName, displayBranchName, nil
+			return BranchInfo{
+				RefName:      branchName,
+				DisplayName:  displayBranchName,
+				DetachedHead: true,
+			}, nil
 		}
 	}
-	return "HEAD", "HEAD", nil
+	return BranchInfo{
+		RefName:      "HEAD",
+		DisplayName:  "HEAD",
+		DetachedHead: true,
+	}, nil
 }
 
 // Delete delete branch

pkg/commands/git_commands/branch_loader.go

@@ -27,24 +27,30 @@ type BranchLoaderConfigCommands interface {
 	Branches() (map[string]*config.Branch, error)
 }
 
+type BranchInfo struct {
+	RefName      string
+	DisplayName  string // e.g. '(HEAD detached at 123asdf)'
+	DetachedHead bool
+}
+
 // BranchLoader returns a list of Branch objects for the current repo
 type BranchLoader struct {
 	*common.Common
 	getRawBranches       func() (string, error)
-	getCurrentBranchName func() (string, string, error)
+	getCurrentBranchInfo func() (BranchInfo, error)
 	config               BranchLoaderConfigCommands
 }
 
 func NewBranchLoader(
 	cmn *common.Common,
 	getRawBranches func() (string, error),
-	getCurrentBranchName func() (string, string, error),
+	getCurrentBranchInfo func() (BranchInfo, error),
 	config BranchLoaderConfigCommands,
 ) *BranchLoader {
 	return &BranchLoader{
 		Common:               cmn,
 		getRawBranches:       getRawBranches,
-		getCurrentBranchName: getCurrentBranchName,
+		getCurrentBranchInfo: getCurrentBranchInfo,
 		config:               config,
 	}
 }
@@ -84,11 +90,11 @@ outer:
 		}
 	}
 	if !foundHead {
-		currentBranchName, currentBranchDisplayName, err := self.getCurrentBranchName()
+		info, err := self.getCurrentBranchInfo()
 		if err != nil {
 			return nil, err
 		}
-		branches = slices.Prepend(branches, &models.Branch{Name: currentBranchName, DisplayName: currentBranchDisplayName, Head: true, Recency: "  *"})
+		branches = slices.Prepend(branches, &models.Branch{Name: info.RefName, DisplayName: info.DisplayName, Head: true, DetachedHead: info.DetachedHead, Recency: "  *"})
 	}
 
 	configBranches, err := self.config.Branches()

pkg/commands/git_commands/branch_test.go

@@ -53,10 +53,10 @@ func TestBranchGetCommitDifferences(t *testing.T) {
 
 func TestBranchNewBranch(t *testing.T) {
 	runner := oscommands.NewFakeRunner(t).
-		Expect(`git checkout -b "test" "master"`, "", nil)
+		Expect(`git checkout -b "test" "refs/heads/master"`, "", nil)
 	instance := buildBranchCommands(commonDeps{runner: runner})
 
-	assert.NoError(t, instance.New("test", "master"))
+	assert.NoError(t, instance.New("test", "refs/heads/master"))
 	runner.CheckForMissingCalls()
 }
 
@@ -162,54 +162,58 @@ func TestBranchGetAllBranchGraph(t *testing.T) {
 	assert.NoError(t, err)
 }
 
-func TestBranchCurrentBranchName(t *testing.T) {
+func TestBranchCurrentBranchInfo(t *testing.T) {
 	type scenario struct {
 		testName string
 		runner   *oscommands.FakeCmdObjRunner
-		test     func(string, string, error)
+		test     func(BranchInfo, error)
 	}
 
 	scenarios := []scenario{
 		{
 			"says we are on the master branch if we are",
 			oscommands.NewFakeRunner(t).Expect(`git symbolic-ref --short HEAD`, "master", nil),
-			func(name string, displayname string, err error) {
+			func(info BranchInfo, err error) {
 				assert.NoError(t, err)
-				assert.EqualValues(t, "master", name)
-				assert.EqualValues(t, "master", displayname)
+				assert.EqualValues(t, "master", info.RefName)
+				assert.EqualValues(t, "master", info.DisplayName)
+				assert.False(t, info.DetachedHead)
 			},
 		},
 		{
 			"falls back to git `git branch --contains` if symbolic-ref fails",
 			oscommands.NewFakeRunner(t).
 				Expect(`git symbolic-ref --short HEAD`, "", errors.New("error")).
-				Expect(`git branch --contains`, "* master", nil),
-			func(name string, displayname string, err error) {
+				Expect(`git branch --contains`, "* (HEAD detached at 8982166a)", nil),
+			func(info BranchInfo, err error) {
 				assert.NoError(t, err)
-				assert.EqualValues(t, "master", name)
-				assert.EqualValues(t, "master", displayname)
+				assert.EqualValues(t, "8982166a", info.RefName)
+				assert.EqualValues(t, "(HEAD detached at 8982166a)", info.DisplayName)
+				assert.True(t, info.DetachedHead)
 			},
 		},
 		{
 			"handles a detached head",
 			oscommands.NewFakeRunner(t).
 				Expect(`git symbolic-ref --short HEAD`, "", errors.New("error")).
 				Expect(`git branch --contains`, "* (HEAD detached at 123abcd)", nil),
-			func(name string, displayname string, err error) {
+			func(info BranchInfo, err error) {
 				assert.NoError(t, err)
-				assert.EqualValues(t, "123abcd", name)
-				assert.EqualValues(t, "(HEAD detached at 123abcd)", displayname)
+				assert.EqualValues(t, "123abcd", info.RefName)
+				assert.EqualValues(t, "(HEAD detached at 123abcd)", info.DisplayName)
+				assert.True(t, info.DetachedHead)
 			},
 		},
 		{
 			"bubbles up error if there is one",
 			oscommands.NewFakeRunner(t).
 				Expect(`git symbolic-ref --short HEAD`, "", errors.New("error")).
 				Expect(`git branch --contains`, "", errors.New("error")),
-			func(name string, displayname string, err error) {
+			func(info BranchInfo, err error) {
 				assert.Error(t, err)
-				assert.EqualValues(t, "", name)
-				assert.EqualValues(t, "", displayname)
+				assert.EqualValues(t, "", info.RefName)
+				assert.EqualValues(t, "", info.DisplayName)
+				assert.False(t, info.DetachedHead)
 			},
 		},
 	}
@@ -218,7 +222,7 @@ func TestBranchCurrentBranchName(t *testing.T) {
 		s := s
 		t.Run(s.testName, func(t *testing.T) {
 			instance := buildBranchCommands(commonDeps{runner: s.runner})
-			s.test(instance.CurrentBranchName())
+			s.test(instance.CurrentBranchInfo())
 			s.runner.CheckForMissingCalls()
 		})
 	}

pkg/commands/git_commands/commit_loader.go

@@ -29,7 +29,7 @@ type CommitLoader struct {
 	*common.Common
 	cmd oscommands.ICmdObjBuilder
 
-	getCurrentBranchName func() (string, string, error)
+	getCurrentBranchInfo func() (BranchInfo, error)
 	getRebaseMode        func() (enums.RebaseMode, error)
 	readFile             func(filename string) ([]byte, error)
 	walkFiles            func(root string, fn filepath.WalkFunc) error
@@ -41,13 +41,13 @@ func NewCommitLoader(
 	cmn *common.Common,
 	cmd oscommands.ICmdObjBuilder,
 	dotGitDir string,
-	getCurrentBranchName func() (string, string, error),
+	getCurrentBranchInfo func() (BranchInfo, error),
 	getRebaseMode func() (enums.RebaseMode, error),
 ) *CommitLoader {
 	return &CommitLoader{
 		Common:               cmn,
 		cmd:                  cmd,
-		getCurrentBranchName: getCurrentBranchName,
+		getCurrentBranchInfo: getCurrentBranchInfo,
 		getRebaseMode:        getRebaseMode,
 		readFile:             os.ReadFile,
 		walkFiles:            filepath.Walk,
@@ -371,13 +371,13 @@ func (self *CommitLoader) setCommitMergedStatuses(refName string, commits []*mod
 }
 
 func (self *CommitLoader) getMergeBase(refName string) (string, error) {
-	currentBranch, _, err := self.getCurrentBranchName()
+	info, err := self.getCurrentBranchInfo()
 	if err != nil {
 		return "", err
 	}
 
 	baseBranch := "master"
-	if strings.HasPrefix(currentBranch, "feature/") {
+	if strings.HasPrefix(info.RefName, "feature/") {
 		baseBranch = "develop"
 	}
 

pkg/commands/git_commands/commit_loader_test.go

@@ -182,8 +182,8 @@ func TestGetCommits(t *testing.T) {
 			builder := &CommitLoader{
 				Common: utils.NewDummyCommon(),
 				cmd:    oscommands.NewDummyCmdObjBuilder(scenario.runner),
-				getCurrentBranchName: func() (string, string, error) {
-					return scenario.currentBranchName, scenario.currentBranchName, nil
+				getCurrentBranchInfo: func() (BranchInfo, error) {
+					return BranchInfo{RefName: scenario.currentBranchName, DisplayName: scenario.currentBranchName, DetachedHead: false}, nil
 				},
 				getRebaseMode: func() (enums.RebaseMode, error) { return scenario.rebaseMode, nil },
 				dotGitDir:     ".git",

pkg/commands/models/branch.go

@@ -11,6 +11,7 @@ type Branch struct {
 	Pullables    string
 	UpstreamGone bool
 	Head         bool
+	DetachedHead bool
 	// if we have a named remote locally this will be the name of that remote e.g.
 	// 'origin' or 'tiwood'. If we don't have the remote locally it'll look like
 	// 'git@github.com:tiwood/lazygit.git'
@@ -19,6 +20,9 @@ type Branch struct {
 }
 
 func (b *Branch) FullRefName() string {
+	if b.DetachedHead {
+		return b.Name
+	}
 	return "refs/heads/" + b.Name
 }
 

pkg/gui/controllers/branches_controller.go

@@ -255,7 +255,7 @@ func (self *BranchesController) createNewBranchWithName(newBranchName string) er
 		return nil
 	}
 
-	if err := self.git.Branch.New(newBranchName, branch.Name); err != nil {
+	if err := self.git.Branch.New(newBranchName, branch.FullRefName()); err != nil {
 		return self.c.Error(err)
 	}
 
@@ -411,7 +411,7 @@ func (self *BranchesController) rename(branch *models.Branch) error {
 }
 
 func (self *BranchesController) newBranch(selectedBranch *models.Branch) error {
-	return self.helpers.Refs.NewBranch(selectedBranch.RefName(), selectedBranch.RefName(), "")
+	return self.helpers.Refs.NewBranch(selectedBranch.FullRefName(), selectedBranch.RefName(), "")
 }
 
 func (self *BranchesController) createPullRequestMenu(selectedBranch *models.Branch, checkedOutBranch *models.Branch) error {

pkg/gui/presentation/icons/git_icons.go

@@ -29,7 +29,7 @@ var remoteIcons = []remoteIcon{
 }
 
 func IconForBranch(branch *models.Branch) string {
-	if branch.DisplayName != "" {
+	if branch.DetachedHead {
 		return DETACHED_HEAD_ICON
 	}
 	return BRANCH_ICON

pkg/integration/tests/branch/checkout_by_name.go

@@ -0,0 +1,39 @@
+package branch
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var CheckoutByName = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "Try to checkout branch by name. Verify that it also works on the branch with the special name @.",
+	ExtraCmdArgs: "",
+	Skip:         false,
+	SetupConfig:  func(config *config.AppConfig) {},
+	SetupRepo: func(shell *Shell) {
+		shell.
+			CreateNCommits(3).
+			NewBranch("@").
+			Checkout("master").
+			EmptyCommit("blah")
+	},
+	Run: func(shell *Shell, input *Input, assert *Assert, keys config.KeybindingConfig) {
+		input.SwitchToBranchesWindow()
+		assert.CurrentViewName("localBranches")
+
+		assert.MatchSelectedLine(Contains("master"))
+		input.NextItem()
+		assert.MatchSelectedLine(Contains("@"))
+		input.PressKeys(keys.Branches.CheckoutBranchByName)
+		assert.InPrompt()
+		assert.MatchCurrentViewTitle(Equals("Branch name:"))
+		input.Type("new-branch")
+		input.Confirm()
+		assert.InAlert()
+		assert.MatchCurrentViewContent(Equals("Branch not found. Create a new branch named new-branch?"))
+		input.Confirm()
+
+		assert.CurrentViewName("localBranches")
+		assert.MatchSelectedLine(Contains("new-branch"))
+	},
+})