diff options
author | Ryooooooga <eial5q265e5@gmail.com> | 2022-10-16 21:31:42 +0900 |
---|---|---|
committer | Jesse Duffield <jessedduffield@gmail.com> | 2022-11-14 19:05:07 +1100 |
commit | 52a2e4c1dc270c95366d05687de514caed95f695 (patch) | |
tree | 3c0d5d086da0a365c8429166a903874f6c5aaf2f /pkg | |
parent | b33ec5a05025a2755f4511f042e9e0fcb224bf47 (diff) |
fix: fix ambiguous branch name
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
Diffstat (limited to 'pkg')
-rw-r--r-- | pkg/commands/git.go | 4 | ||||
-rw-r--r-- | pkg/commands/git_commands/branch.go | 26 | ||||
-rw-r--r-- | pkg/commands/git_commands/branch_loader.go | 16 | ||||
-rw-r--r-- | pkg/commands/git_commands/branch_test.go | 40 | ||||
-rw-r--r-- | pkg/commands/git_commands/commit_loader.go | 10 | ||||
-rw-r--r-- | pkg/commands/git_commands/commit_loader_test.go | 4 | ||||
-rw-r--r-- | pkg/commands/models/branch.go | 4 | ||||
-rw-r--r-- | pkg/gui/controllers/branches_controller.go | 4 | ||||
-rw-r--r-- | pkg/gui/presentation/icons/git_icons.go | 2 | ||||
-rw-r--r-- | pkg/integration/tests/branch/checkout_by_name.go | 39 | ||||
-rw-r--r-- | pkg/integration/tests/tests.go | 1 |
11 files changed, 107 insertions, 43 deletions
diff --git a/pkg/commands/git.go b/pkg/commands/git.go index 2faa75c3d..ee223a6d0 100644 --- a/pkg/commands/git.go +++ b/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) diff --git a/pkg/commands/git_commands/branch.go b/pkg/commands/git_commands/branch.go index d8be71deb..117427778 100644 --- a/pkg/commands/git_commands/branch.go +++ b/pkg/commands/git_commands/branch.go @@ -30,18 +30,20 @@ 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) @@ -49,10 +51,18 @@ func (self *BranchCommands) CurrentBranchName() (string, string, error) { 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 diff --git a/pkg/commands/git_commands/branch_loader.go b/pkg/commands/git_commands/branch_loader.go index 08811374d..58e7bb940 100644 --- a/pkg/commands/git_commands/branch_loader.go +++ b/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() diff --git a/pkg/commands/git_commands/branch_test.go b/pkg/commands/git_commands/branch_test.go index 757a8a83d..94456c0f8 100644 --- a/pkg/commands/git_commands/branch_test.go +++ b/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,32 +162,34 @@ 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) }, }, { @@ -195,10 +197,11 @@ func TestBranchCurrentBranchName(t *testing.T) { 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) }, }, { @@ -206,10 +209,11 @@ func TestBranchCurrentBranchName(t *testing.T) { 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() }) } diff --git a/pkg/commands/git_commands/commit_loader.go b/pkg/commands/git_commands/commit_loader.go index 21395fad4..f89f62c48 100644 --- a/pkg/commands/git_commands/commit_loader.go +++ b/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" } diff --git a/pkg/commands/git_commands/commit_loader_test.go b/pkg/commands/git_commands/commit_loader_test.go index 7d45101ff..ab6d5a105 100644 --- a/pkg/commands/git_commands/commit_loader_test.go +++ b/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", diff --git a/pkg/commands/models/branch.go b/pkg/commands/models/branch.go index 49bb801fa..895b0442a 100644 --- a/pkg/commands/models/branch.go +++ b/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 } diff --git a/pkg/gui/controllers/branches_controller.go b/pkg/gui/controllers/branches_controller.go index b049a0a0b..9222492d7 100644 --- a/pkg/gui/controllers/branches_controller.go +++ b/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 { diff --git a/pkg/gui/presentation/icons/git_icons.go b/pkg/gui/presentation/icons/git_icons.go index 5118652ea..a66149b55 100644 --- a/pkg/gui/presentation/icons/git_icons.go +++ b/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 diff --git a/pkg/integration/tests/branch/checkout_by_name.go b/pkg/integration/tests/branch/checkout_by_name.go new file mode 100644 index 000000000..4888b0112 --- /dev/null +++ b/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")) + }, +}) diff --git a/pkg/integration/tests/tests.go b/pkg/integration/tests/tests.go index 8d5f6adde..1ed99a47c 100644 --- a/pkg/integration/tests/tests.go +++ b/pkg/integration/tests/tests.go @@ -26,6 +26,7 @@ import ( var tests = []*components.IntegrationTest{ bisect.Basic, bisect.FromOtherBranch, + branch.CheckoutByName, branch.Delete, branch.Rebase, branch.RebaseAndDrop, |