From 91f02ae1bdb234b307c900eeaecd4c8a82992c73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joshua=20T=C3=B6pfer?= Date: Fri, 11 Oct 2024 08:15:27 +0200 Subject: [PATCH 1/4] fix #451 --- CHANGELOG.md | 3 +++ mob.go | 24 ++++++++++++++++++++++-- mob_test.go | 23 ++++++++++++++++++++++- 3 files changed, 47 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f00e097..22fe3b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,6 @@ +# 5.3.3 +- Fix: `mob start` now functions correctly on WIP branches when the base branch is not checked out, applicable to branch names that do not contain a `-` character. + # 5.3.2 - Fix: Removed wrong warning about diverging wip branch when joining a new session diff --git a/mob.go b/mob.go index 51941de..893ce37 100644 --- a/mob.go +++ b/mob.go @@ -21,7 +21,7 @@ import ( ) const ( - versionNumber = "5.3.2" + versionNumber = "5.3.3" minimumGitVersion = "2.13.0" ) @@ -122,6 +122,19 @@ func (branch Branch) hasRemoteBranch(configuration config.Configuration) bool { return false } +func (branch Branch) hasLocalBranch(localBranches []string) bool { + say.Debug("Local Branches: " + strings.Join(localBranches, "\n")) + say.Debug("Local Branch: " + branch.Name) + + for i := 0; i < len(localBranches); i++ { + if localBranches[i] == branch.Name { + return true + } + } + + return false +} + func (branch Branch) IsWipBranch(configuration config.Configuration) bool { if branch.Name == "mob-session" { return true @@ -536,7 +549,9 @@ func start(configuration config.Configuration) error { } git("fetch", configuration.RemoteName, "--prune") - currentBaseBranch, currentWipBranch := determineBranches(gitCurrentBranch(), gitBranches(), configuration) + currentBranch := gitCurrentBranch() + localBranches := gitBranches() + currentBaseBranch, currentWipBranch := determineBranches(currentBranch, localBranches, configuration) if !currentWipBranch.hasRemoteBranch(configuration) && configuration.StartJoin { say.Error("Remote wip branch " + currentWipBranch.remote(configuration).String() + " is missing") @@ -551,6 +566,11 @@ func start(configuration config.Configuration) error { createRemoteBranch(configuration, currentBaseBranch) + if !currentBaseBranch.hasLocalBranch(localBranches) { + silentgit("checkout", "-b", currentBaseBranch.Name) + silentgit("checkout", currentBranch.Name) + } + if currentBaseBranch.hasUnpushedCommits(configuration) { say.Error("cannot start; unpushed changes on base branch must be pushed upstream") say.Fix("to fix this, push those commits and try again", "git push "+configuration.RemoteName+" "+currentBaseBranch.String()) diff --git a/mob_test.go b/mob_test.go index a4cba54..ad9f832 100644 --- a/mob_test.go +++ b/mob_test.go @@ -1884,6 +1884,27 @@ func TestMobClean(t *testing.T) { assertNoMobSessionBranches(t, configuration, "mob-session") } +func TestMobStartOnWipBranchWithoutCheckedOutBaseBranchWithoutHyphens(t *testing.T) { + output, configuration := setup(t) + + setWorkingDir(tempDir + "/alice") + git("switch", "-C", "basebranchwithouthyphen") + configuration.StartCreate = true + start(configuration) + assertOnBranch(t, "mob/basebranchwithouthyphen") + createFile(t, "file1.txt", "abc") + next(configuration) + assertOnBranch(t, "basebranchwithouthyphen") + + setWorkingDir(tempDir + "/bob") + git("switch", "-C", "mob/basebranchwithouthyphen") + configuration.StartCreate = false + + assertNoError(t, start(configuration)) + assertOnBranch(t, "mob/basebranchwithouthyphen") + assertOutputContains(t, output, "joining existing session from origin/mob/basebranchwithouthyphen") +} + func TestGitVersionParse(t *testing.T) { // Check real examples equals(t, GitVersion{2, 34, 1}, parseGitVersion("git version 2.34.1")) @@ -2093,7 +2114,7 @@ func setWorkingDir(dir string) { func assertNoError(t *testing.T, err error) { if err != nil { - failWithFailure(t, err, nil) + failWithFailure(t, nil, err) } } From 61c53ca2bc7cebac2615bc47d25b6507c9d4393c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joshua=20T=C3=B6pfer?= Date: Fri, 11 Oct 2024 08:26:08 +0200 Subject: [PATCH 2/4] refactored test to be compatible with git 2.20.0 --- mob_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mob_test.go b/mob_test.go index ad9f832..ea7bc34 100644 --- a/mob_test.go +++ b/mob_test.go @@ -1888,7 +1888,7 @@ func TestMobStartOnWipBranchWithoutCheckedOutBaseBranchWithoutHyphens(t *testing output, configuration := setup(t) setWorkingDir(tempDir + "/alice") - git("switch", "-C", "basebranchwithouthyphen") + git("checkout", "-b", "basebranchwithouthyphen") configuration.StartCreate = true start(configuration) assertOnBranch(t, "mob/basebranchwithouthyphen") @@ -1897,7 +1897,7 @@ func TestMobStartOnWipBranchWithoutCheckedOutBaseBranchWithoutHyphens(t *testing assertOnBranch(t, "basebranchwithouthyphen") setWorkingDir(tempDir + "/bob") - git("switch", "-C", "mob/basebranchwithouthyphen") + git("checkout", "-b", "mob/basebranchwithouthyphen") configuration.StartCreate = false assertNoError(t, start(configuration)) From a36976f1a198f5a10301b4a5a7fe907601459d60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joshua=20T=C3=B6pfer?= Date: Sat, 12 Oct 2024 20:24:58 +0200 Subject: [PATCH 3/4] #451 adjusted fix to not checkout basebranch --- mob.go | 33 +++++++++++++++++---------------- mob_test.go | 4 ++++ status.go | 2 +- 3 files changed, 22 insertions(+), 17 deletions(-) diff --git a/mob.go b/mob.go index 893ce37..f02bce7 100644 --- a/mob.go +++ b/mob.go @@ -566,12 +566,7 @@ func start(configuration config.Configuration) error { createRemoteBranch(configuration, currentBaseBranch) - if !currentBaseBranch.hasLocalBranch(localBranches) { - silentgit("checkout", "-b", currentBaseBranch.Name) - silentgit("checkout", currentBranch.Name) - } - - if currentBaseBranch.hasUnpushedCommits(configuration) { + if currentBaseBranch.hasLocalBranch(localBranches) && currentBaseBranch.hasUnpushedCommits(configuration) { say.Error("cannot start; unpushed changes on base branch must be pushed upstream") say.Fix("to fix this, push those commits and try again", "git push "+configuration.RemoteName+" "+currentBaseBranch.String()) return errors.New("cannot start; unpushed changes on base branch must be pushed upstream") @@ -610,7 +605,7 @@ func start(configuration config.Configuration) error { } say.Info("you are on wip branch '" + currentWipBranch.String() + "' (base branch '" + currentBaseBranch.String() + "')") - sayLastCommitsList(currentBaseBranch.String(), currentWipBranch.String()) + sayLastCommitsList(currentBaseBranch, currentWipBranch, configuration) openLastModifiedFileIfPresent(configuration) @@ -1021,12 +1016,16 @@ func squashOrCommit(configuration config.Configuration) string { } } -func sayLastCommitsList(currentBaseBranch string, currentWipBranch string) { - commitsBaseWipBranch := currentBaseBranch + ".." + currentWipBranch - log := silentgit("--no-pager", "log", commitsBaseWipBranch, "--pretty=format:%h %cr <%an>", "--abbrev-commit") +func sayLastCommitsList(currentBaseBranch Branch, currentWipBranch Branch, configuration config.Configuration) { + commitsBaseWipBranch := currentBaseBranch.String() + ".." + currentWipBranch.String() + log, err := silentgitignorefailure("--no-pager", "log", commitsBaseWipBranch, "--pretty=format:%h %cr <%an>", "--abbrev-commit") + if err != nil { + commitsBaseWipBranch = currentBaseBranch.remote(configuration).String() + ".." + currentWipBranch.String() + log = silentgit("--no-pager", "log", commitsBaseWipBranch, "--pretty=format:%h %cr <%an>", "--abbrev-commit") + } lines := strings.Split(log, "\n") if len(lines) > 5 { - say.Info("wip branch '" + currentWipBranch + "' contains " + strconv.Itoa(len(lines)) + " commits. The last 5 were:") + say.Info("wip branch '" + currentWipBranch.String() + "' contains " + strconv.Itoa(len(lines)) + " commits. The last 5 were:") lines = lines[:5] } ReverseSlice(lines) @@ -1094,7 +1093,8 @@ func doBranchesDiverge(ancestor string, successor string) bool { } func gitUserName() string { - return silentgitignorefailure("config", "--get", "user.name") + output, _ := silentgitignorefailure("config", "--get", "user.name") + return output } func gitUserEmail() string { @@ -1150,13 +1150,13 @@ func silentgit(args ...string) string { return strings.TrimSpace(output) } -func silentgitignorefailure(args ...string) string { +func silentgitignorefailure(args ...string) (string, error) { _, output, err := runCommandSilent("git", args...) if err != nil { - return "" + return "", err } - return strings.TrimSpace(output) + return strings.TrimSpace(output), nil } func deleteEmptyStrings(s []string) []string { @@ -1225,7 +1225,8 @@ func gitIgnoreFailure(args ...string) error { } func gitCommitHash() string { - return silentgitignorefailure("rev-parse", "HEAD") + output, _ := silentgitignorefailure("rev-parse", "HEAD") + return output } func gitVersion() string { diff --git a/mob_test.go b/mob_test.go index ea7bc34..4ac1e86 100644 --- a/mob_test.go +++ b/mob_test.go @@ -1903,6 +1903,10 @@ func TestMobStartOnWipBranchWithoutCheckedOutBaseBranchWithoutHyphens(t *testing assertNoError(t, start(configuration)) assertOnBranch(t, "mob/basebranchwithouthyphen") assertOutputContains(t, output, "joining existing session from origin/mob/basebranchwithouthyphen") + + createFile(t, "file2.txt", "abc") + done(configuration) + assertOnBranch(t, "basebranchwithouthyphen") } func TestGitVersionParse(t *testing.T) { diff --git a/status.go b/status.go index 924cf82..3b8c1ae 100644 --- a/status.go +++ b/status.go @@ -10,7 +10,7 @@ func status(configuration config.Configuration) { currentBaseBranch, currentWipBranch := determineBranches(gitCurrentBranch(), gitBranches(), configuration) say.Info("you are on wip branch " + currentWipBranch.String() + " (base branch " + currentBaseBranch.String() + ")") - sayLastCommitsList(currentBaseBranch.String(), currentWipBranch.String()) + sayLastCommitsList(currentBaseBranch, currentWipBranch, configuration) } else { currentBaseBranch, _ := determineBranches(gitCurrentBranch(), gitBranches(), configuration) say.Info("you are on base branch '" + currentBaseBranch.String() + "'") From 5061e61509ce08f4f0f73849a939ba5496519c27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joshua=20T=C3=B6pfer?= Date: Thu, 17 Oct 2024 17:03:52 +0200 Subject: [PATCH 4/4] unified hasLocalBranch function --- mob.go | 26 ++++++-------------------- mob_test.go | 26 ++++++++++++++------------ 2 files changed, 20 insertions(+), 32 deletions(-) diff --git a/mob.go b/mob.go index f02bce7..0e07ef2 100644 --- a/mob.go +++ b/mob.go @@ -122,7 +122,8 @@ func (branch Branch) hasRemoteBranch(configuration config.Configuration) bool { return false } -func (branch Branch) hasLocalBranch(localBranches []string) bool { +func (branch Branch) hasLocalBranch() bool { + localBranches := gitBranches() say.Debug("Local Branches: " + strings.Join(localBranches, "\n")) say.Debug("Local Branch: " + branch.Name) @@ -529,7 +530,7 @@ func deleteRemoteWipBranch(configuration config.Configuration) { currentBaseBranch, currentWipBranch := determineBranches(gitCurrentBranch(), gitBranches(), configuration) git("checkout", currentBaseBranch.String()) - if hasLocalBranch(currentWipBranch.String()) { + if currentWipBranch.hasLocalBranch() { git("branch", "--delete", "--force", currentWipBranch.String()) } if currentWipBranch.hasRemoteBranch(configuration) { @@ -550,8 +551,7 @@ func start(configuration config.Configuration) error { git("fetch", configuration.RemoteName, "--prune") currentBranch := gitCurrentBranch() - localBranches := gitBranches() - currentBaseBranch, currentWipBranch := determineBranches(currentBranch, localBranches, configuration) + currentBaseBranch, currentWipBranch := determineBranches(currentBranch, gitBranches(), configuration) if !currentWipBranch.hasRemoteBranch(configuration) && configuration.StartJoin { say.Error("Remote wip branch " + currentWipBranch.remote(configuration).String() + " is missing") @@ -566,7 +566,7 @@ func start(configuration config.Configuration) error { createRemoteBranch(configuration, currentBaseBranch) - if currentBaseBranch.hasLocalBranch(localBranches) && currentBaseBranch.hasUnpushedCommits(configuration) { + if currentBaseBranch.hasLocalBranch() && currentBaseBranch.hasUnpushedCommits(configuration) { say.Error("cannot start; unpushed changes on base branch must be pushed upstream") say.Fix("to fix this, push those commits and try again", "git push "+configuration.RemoteName+" "+currentBaseBranch.String()) return errors.New("cannot start; unpushed changes on base branch must be pushed upstream") @@ -766,7 +766,7 @@ func startJoinMobSession(configuration config.Configuration) { baseBranch, currentWipBranch := determineBranches(gitCurrentBranch(), gitBranches(), configuration) say.Info("joining existing session from " + currentWipBranch.remote(configuration).String()) - if hasLocalBranch(currentWipBranch.Name) && doBranchesDiverge(baseBranch.remote(configuration).Name, currentWipBranch.Name) { + if currentWipBranch.hasLocalBranch() && doBranchesDiverge(baseBranch.remote(configuration).Name, currentWipBranch.Name) { say.Warning("Careful, your wip branch (" + currentWipBranch.Name + ") diverges from your main branch (" + baseBranch.remote(configuration).Name + ") !") } @@ -1057,20 +1057,6 @@ func isMobProgramming(configuration config.Configuration) bool { return currentWipBranch == currentBranch } -func hasLocalBranch(localBranch string) bool { - localBranches := gitBranches() - say.Debug("Local Branches: " + strings.Join(localBranches, "\n")) - say.Debug("Local Branch: " + localBranch) - - for i := 0; i < len(localBranches); i++ { - if localBranches[i] == localBranch { - return true - } - } - - return false -} - func gitBranches() []string { return strings.Split(silentgit("branch", "--format=%(refname:short)"), "\n") } diff --git a/mob_test.go b/mob_test.go index 4ac1e86..af95d31 100644 --- a/mob_test.go +++ b/mob_test.go @@ -2255,33 +2255,35 @@ func assertOutputNotContains(t *testing.T, output *string, notContains string) { } } -func assertMobSessionBranches(t *testing.T, configuration config.Configuration, branch string) { - if !newBranch(branch).hasRemoteBranch(configuration) { - failWithFailure(t, newBranch(branch).remote(configuration).Name, "none") +func assertMobSessionBranches(t *testing.T, configuration config.Configuration, branchName string) { + branch := newBranch(branchName) + if !branch.hasRemoteBranch(configuration) { + failWithFailure(t, branch.remote(configuration).Name, "none") } - if !hasLocalBranch(branch) { - failWithFailure(t, branch, "none") + if !branch.hasLocalBranch() { + failWithFailure(t, branchName, "none") } } func assertLocalBranch(t *testing.T, branch string) { - if !hasLocalBranch(branch) { + if !newBranch(branch).hasLocalBranch() { failWithFailure(t, branch, "none") } } func assertNoLocalBranch(t *testing.T, branch string) { - if hasLocalBranch(branch) { + if newBranch(branch).hasLocalBranch() { failWithFailure(t, branch, "none") } } -func assertNoMobSessionBranches(t *testing.T, configuration config.Configuration, branch string) { - if newBranch(branch).hasRemoteBranch(configuration) { - failWithFailure(t, "none", newBranch(branch).remote(configuration).Name) +func assertNoMobSessionBranches(t *testing.T, configuration config.Configuration, branchName string) { + branch := newBranch(branchName) + if branch.hasRemoteBranch(configuration) { + failWithFailure(t, "none", branch.remote(configuration).Name) } - if hasLocalBranch(branch) { - failWithFailure(t, "none", branch) + if branch.hasLocalBranch() { + failWithFailure(t, "none", branchName) } }