Skip to content

Commit

Permalink
fix: fix ambiguous branch name
Browse files Browse the repository at this point in the history
test: add an integration test for checkout branch by name

fix: fix full ref name of detached head

refactor: refactor current branch loader

chore: use field name explicitly
  • Loading branch information
Ryooooooga authored and jesseduffield committed Nov 14, 2022
1 parent b33ec5a commit 52a2e4c
Show file tree
Hide file tree
Showing 39 changed files with 150 additions and 43 deletions.
4 changes: 2 additions & 2 deletions pkg/commands/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
26 changes: 18 additions & 8 deletions pkg/commands/git_commands/branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 11 additions & 5 deletions pkg/commands/git_commands/branch_loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}
Expand Down Expand Up @@ -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()
Expand Down
40 changes: 22 additions & 18 deletions pkg/commands/git_commands/branch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

Expand Down Expand Up @@ -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)
},
},
}
Expand All @@ -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()
})
}
Expand Down
10 changes: 5 additions & 5 deletions pkg/commands/git_commands/commit_loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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"
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/commands/git_commands/commit_loader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
4 changes: 4 additions & 0 deletions pkg/commands/models/branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
// '[email protected]:tiwood/lazygit.git'
Expand All @@ -19,6 +20,9 @@ type Branch struct {
}

func (b *Branch) FullRefName() string {
if b.DetachedHead {
return b.Name
}
return "refs/heads/" + b.Name
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/gui/controllers/branches_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion pkg/gui/presentation/icons/git_icons.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
39 changes: 39 additions & 0 deletions pkg/integration/tests/branch/checkout_by_name.go
Original file line number Diff line number Diff line change
@@ -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"))
},
})
1 change: 1 addition & 0 deletions pkg/integration/tests/tests.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
var tests = []*components.IntegrationTest{
bisect.Basic,
bisect.FromOtherBranch,
branch.CheckoutByName,
branch.Delete,
branch.Rebase,
branch.RebaseAndDrop,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
blah
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ref: refs/heads/new-branch
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
[core]
repositoryformatversion = 0
filemode = true
bare = false
logallrefupdates = true
ignorecase = true
precomposeunicode = true
[user]
email = [email protected]
name = CI
[commit]
gpgSign = false
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Unnamed repository; edit this file 'description' to name the repository.
Binary file not shown.
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
0000000000000000000000000000000000000000 95efd4ea6ed74b904cd8eeeaa6245ec462372f67 CI <[email protected]> 1665924398 +0900 commit (initial): commit 01
95efd4ea6ed74b904cd8eeeaa6245ec462372f67 cfc66e0e89b1dc11a927cc453dbee025ff03cf83 CI <[email protected]> 1665924398 +0900 commit: commit 02
cfc66e0e89b1dc11a927cc453dbee025ff03cf83 6d69ac71e12a83769fca195d0a714435e1f4661a CI <[email protected]> 1665924398 +0900 commit: commit 03
6d69ac71e12a83769fca195d0a714435e1f4661a 6d69ac71e12a83769fca195d0a714435e1f4661a CI <[email protected]> 1665924398 +0900 checkout: moving from master to @
6d69ac71e12a83769fca195d0a714435e1f4661a 6d69ac71e12a83769fca195d0a714435e1f4661a CI <[email protected]> 1665924398 +0900 checkout: moving from @ to master
6d69ac71e12a83769fca195d0a714435e1f4661a 18565748bda3ca01a67a92f340705af5a11384ae CI <[email protected]> 1665924398 +0900 commit: blah
18565748bda3ca01a67a92f340705af5a11384ae 6d69ac71e12a83769fca195d0a714435e1f4661a CI <[email protected]> 1665924403 +0900 checkout: moving from master to new-branch
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
0000000000000000000000000000000000000000 6d69ac71e12a83769fca195d0a714435e1f4661a CI <[email protected]> 1665924398 +0900 branch: Created from HEAD
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
0000000000000000000000000000000000000000 95efd4ea6ed74b904cd8eeeaa6245ec462372f67 CI <[email protected]> 1665924398 +0900 commit (initial): commit 01
95efd4ea6ed74b904cd8eeeaa6245ec462372f67 cfc66e0e89b1dc11a927cc453dbee025ff03cf83 CI <[email protected]> 1665924398 +0900 commit: commit 02
cfc66e0e89b1dc11a927cc453dbee025ff03cf83 6d69ac71e12a83769fca195d0a714435e1f4661a CI <[email protected]> 1665924398 +0900 commit: commit 03
6d69ac71e12a83769fca195d0a714435e1f4661a 18565748bda3ca01a67a92f340705af5a11384ae CI <[email protected]> 1665924398 +0900 commit: blah
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
0000000000000000000000000000000000000000 6d69ac71e12a83769fca195d0a714435e1f4661a CI <[email protected]> 1665924403 +0900 branch: Created from refs/heads/@
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
x��M
�0@a�9E��d�3̀��U�1I&ThL)<�=�ۏ�x���.�т%A1۔�p�`<q���C#�Y�v>�=4$�X�nB���B1<��.T���3�~�y��yyʗ۾�-��Ѐ�zGQ_�N=�������~\
9f
Expand Down
Binary file not shown.
Binary file not shown.
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
x��=
1@a�bzAf�gD����d��q�%��w `���'K���06U�X ��Ė4WO�9'm� ]Dݛ%�f͛�H�5q�*D��Y�W�*��:iə��e�i��4�����ԓ,�
c`�'8"#���SC��?��;�
Binary file not shown.
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
x��A
1 @Q�=E����4mAD���H�� C��,<���[�����Ʀ
b�P�8s�-��G�X��Er�H�)�U6}H^[%�('K�FUa$���������/�78Ϸ�~��O=��_�1��4�G��5{ݧ���,�/�[;�
Expand Down
Binary file not shown.
Loading

0 comments on commit 52a2e4c

Please sign in to comment.