Repository: remotemobprogramming/mob (go)

Issue:
"mob start" fails when base branch isn't a local branch
I'm not sure if this is a feature or a bug: Let's assume an ensemble works on a task with the base branch some-task. Now a colleague joins, clones the repo, switches immediately to mob/some-task (!) and enters mob s 10. Then he/she will see:

```
  git fetch origin --prune
ERROR git rev-list --count --left-only refs/heads/some-task...refs/remotes/origin/some-task
ERROR fatal: ambiguous argument 'refs/heads/some-task...refs/remotes/origin/some-task': unknown revision or path not in the working tree.
ERROR Use '--' to separate paths from revisions, like this:
ERROR 'git <command> [<revision>...] -- [<file>...]'
ERROR exit status 128
```

Of course he/she should have switched to some-task and do a mob s 10 (or probably mob s 10 --join ).
Is the current behaviour intended or could this be improved?

Pull request:
fix #451

Patch:
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..0e07ef2 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,20 @@ func (branch Branch) hasRemoteBranch(configuration config.Configuration) bool {
 	return false
 }
 
+func (branch Branch) hasLocalBranch() bool {
+	localBranches := gitBranches()
+	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
@@ -516,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) {
@@ -536,7 +550,8 @@ func start(configuration config.Configuration) error {
 	}
 
 	git("fetch", configuration.RemoteName, "--prune")
-	currentBaseBranch, currentWipBranch := determineBranches(gitCurrentBranch(), gitBranches(), configuration)
+	currentBranch := gitCurrentBranch()
+	currentBaseBranch, currentWipBranch := determineBranches(currentBranch, gitBranches(), configuration)
 
 	if !currentWipBranch.hasRemoteBranch(configuration) && configuration.StartJoin {
 		say.Error("Remote wip branch " + currentWipBranch.remote(configuration).String() + " is missing")
@@ -551,7 +566,7 @@ func start(configuration config.Configuration) error {
 
 	createRemoteBranch(configuration, currentBaseBranch)
 
-	if 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")
@@ -590,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)
 
@@ -751,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 + ") !")
 	}
 
@@ -1001,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)
@@ -1038,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")
 }
@@ -1074,7 +1079,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 {
@@ -1130,13 +1136,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 {
@@ -1205,7 +1211,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/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() + "'")


Test patch:
diff --git a/mob_test.go b/mob_test.go
index a4cba54..af95d31 100644
--- a/mob_test.go
+++ b/mob_test.go
@@ -1884,6 +1884,31 @@ func TestMobClean(t *testing.T) {
 	assertNoMobSessionBranches(t, configuration, "mob-session")
 }
 
+func TestMobStartOnWipBranchWithoutCheckedOutBaseBranchWithoutHyphens(t *testing.T) {
+	output, configuration := setup(t)
+
+	setWorkingDir(tempDir + "/alice")
+	git("checkout", "-b", "basebranchwithouthyphen")
+	configuration.StartCreate = true
+	start(configuration)
+	assertOnBranch(t, "mob/basebranchwithouthyphen")
+	createFile(t, "file1.txt", "abc")
+	next(configuration)
+	assertOnBranch(t, "basebranchwithouthyphen")
+
+	setWorkingDir(tempDir + "/bob")
+	git("checkout", "-b", "mob/basebranchwithouthyphen")
+	configuration.StartCreate = false
+
+	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) {
 	// Check real examples
 	equals(t, GitVersion{2, 34, 1}, parseGitVersion("git version 2.34.1"))
@@ -2093,7 +2118,7 @@ func setWorkingDir(dir string) {
 
 func assertNoError(t *testing.T, err error) {
 	if err != nil {
-		failWithFailure(t, err, nil)
+		failWithFailure(t, nil, err)
 	}
 
 }
@@ -2230,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)
 	}
 }