Repository: remotemobprogramming/mob
Pull request #96 (2020-12-26 18:50:24)

Adds the go.mod file, and further refactoring
The go.mod file helps with IntelliJ.
It enables to show code coverage and debug tests.
Maybe you want to change the module naming.

With the help of your tests I did more refactoring.
Was focusing on determineBranches.
I did many smaller commits, so it is more comprehensible.


Patch:
diff --git a/go.mod b/go.mod
new file mode 100644
index 0000000..57af791
--- /dev/null
+++ b/go.mod
@@ -0,0 +1,3 @@
+module mob.sh
+
+go 1.15
diff --git a/mob.go b/mob.go
index 56e6a17..4a1f59b 100644
--- a/mob.go
+++ b/mob.go
@@ -244,48 +244,78 @@ func execute(command string, parameter []string) {
 	}
 }
 
-func determineBranches(currentBranch string, userSpecifiedBranchQualifier string, allLocalBranches string) (baseBranch string, wipBranch string) {
-	localBranches := strings.Split(allLocalBranches, "\n")
-
-	currentBranchIsWipBranch := strings.Index(currentBranch, wipBranchPrefix) == 0
-
-	if currentBranch == "mob-session" || (currentBranch == "master" && userSpecifiedBranchQualifier == "") {
-		baseBranch = "master"
-		wipBranch = "mob-session"
-	} else if currentBranchIsWipBranch {
-		wipBranch = currentBranch
-		wipBranchWithPossibleSuffix := wipBranch[len(wipBranchPrefix):]
-		branchExists := stringContains(localBranches, wipBranchWithPossibleSuffix)
-		if branchExists {
-			baseBranch = wipBranchWithPossibleSuffix
-		} else {
-			if index := strings.LastIndex(wipBranchWithPossibleSuffix, configuration.WipBranchQualifierSeparator); index != -1 {
-				baseBranch = wipBranchWithPossibleSuffix[:index]
-			} else {
-				baseBranch = wipBranchWithPossibleSuffix
-			}
-		}
-	} else {
-		baseBranch = currentBranch
-		if userSpecifiedBranchQualifier == "" {
-			wipBranch = wipBranchPrefix + baseBranch
-		} else {
-			wipBranch = wipBranchPrefix + baseBranch + configuration.WipBranchQualifierSeparator + userSpecifiedBranchQualifier
-		}
-	}
+func determineBranches(currentBranch string, localBranches []string) (baseBranch string, wipBranch string) {
+	baseBranch = determineBaseBranch(currentBranch, localBranches)
+	wipBranch = determineWipBranch(currentBranch)
 
 	debugInfo("on currentBranch " + currentBranch + " => BASE " + baseBranch + " WIP " + wipBranch + " with allLocalBranches " + strings.Join(localBranches, ","))
 	if currentBranch == baseBranch {
 		debugInfo("on base branch")
 	} else if currentBranch == wipBranch {
 		debugInfo("on wip branch")
-	} else {
+	} else { // this is unreachable code, but afraid to delete it
 		debugInfo("neither on base nor on wip branch")
 		panic("bad")
 	}
 	return
 }
 
+func determineBaseBranch(currentBranch string, localBranches []string) string {
+	if currentBranch == "mob-session" || (currentBranch == "master" && !customWipBranchQualifierConfigured()) {
+		return "master"
+	} else if !isWipBranch(currentBranch) {
+		return currentBranch
+	} else if branchExists(removeWipPrefix(currentBranch), localBranches) || !hasSuffix(removeWipPrefix(currentBranch)) {
+		return removeWipPrefix(currentBranch)
+	} else {
+		return removeSuffix(removeWipPrefix(currentBranch))
+	}
+}
+
+func determineWipBranch(currentBranch string) string {
+	if currentBranch == "mob-session" || (currentBranch == "master" && !customWipBranchQualifierConfigured()) {
+		return "mob-session"
+	} else if isWipBranch(currentBranch) {
+		return currentBranch
+	} else if customWipBranchQualifierConfigured() {
+		return addSuffix(currentBranch)
+	} else {
+		return addWipPrefix(currentBranch)
+	}
+}
+
+func customWipBranchQualifierConfigured() bool {
+	return configuration.WipBranchQualifier != ""
+}
+
+func isWipBranch(branch string) bool {
+	return strings.Index(branch, wipBranchPrefix) == 0
+}
+
+func addWipPrefix(branch string) string {
+	return wipBranchPrefix + branch
+}
+
+func removeWipPrefix(branch string) string {
+	return branch[len(wipBranchPrefix):]
+}
+
+func addSuffix(branch string) string {
+	return addWipPrefix(branch) + configuration.WipBranchQualifierSeparator + configuration.WipBranchQualifier
+}
+
+func hasSuffix(branch string) bool {
+	return strings.Contains(branch, configuration.WipBranchQualifierSeparator)
+}
+
+func removeSuffix(branch string) string {
+	return branch[:strings.LastIndex(branch, configuration.WipBranchQualifierSeparator)]
+}
+
+func branchExists(branchInQuestion string, existingBranches []string) bool {
+	return stringContains(existingBranches, branchInQuestion)
+}
+
 func stringContains(list []string, element string) bool {
 	found := false
 	for i := 0; i < len(list); i++ {
@@ -376,7 +406,7 @@ func moo() {
 func reset() {
 	git("fetch", configuration.RemoteName)
 
-	currentBaseBranch, currentWipBranch := determineBranches(gitCurrentBranch(), configuration.WipBranchQualifier, gitBranches())
+	currentBaseBranch, currentWipBranch := determineBranches(gitCurrentBranch(), gitBranches())
 
 	git("checkout", currentBaseBranch)
 	if hasLocalBranch(currentWipBranch) {
@@ -396,18 +426,8 @@ func start() {
 			stashed = true
 		} else {
 			sayInfo("cannot start; clean working tree required")
-			unstagedChanges := getUnstagedChanges()
-			untrackedFiles := getUntrackedFiles()
-			hasUnstagedChanges := len(unstagedChanges) > 0
-			hasUntrackedFiles := len(untrackedFiles) > 0
-			if hasUnstagedChanges {
-				sayInfo("unstaged changes present:")
-				sayInfo(unstagedChanges)
-			}
-			if hasUntrackedFiles {
-				sayInfo("untracked files present:")
-				sayInfo(untrackedFiles)
-			}
+			sayUnstagedChangesInfo()
+			sayUntrackedFilesInfo()
 			sayTodo("To start mob programming including uncommitted changes, use", "mob start --include-uncommitted-changes")
 			return
 		}
@@ -415,7 +435,7 @@ func start() {
 
 	git("fetch", configuration.RemoteName, "--prune")
 
-	currentBaseBranch, currentWipBranch := determineBranches(gitCurrentBranch(), configuration.WipBranchQualifier, gitBranches())
+	currentBaseBranch, currentWipBranch := determineBranches(gitCurrentBranch(), gitBranches())
 
 	hasWipBranchesWithQualifier := hasQualifiedBranches(currentBaseBranch, gitRemoteBranches())
 
@@ -449,14 +469,32 @@ func start() {
 	}
 }
 
-func hasQualifiedBranches(currentBaseBranch string, remoteBranches string) bool {
-	debugInfo("check on current base branch " + currentBaseBranch + " with remote branches " + strings.Join(strings.Split(remoteBranches, "\n"), ","))
-	hasWipBranchesWithQualifier := strings.Contains(remoteBranches, configuration.RemoteName+"/"+wipBranchPrefix+currentBaseBranch+configuration.WipBranchQualifierSeparator)
+func sayUntrackedFilesInfo() {
+	untrackedFiles := getUntrackedFiles()
+	hasUntrackedFiles := len(untrackedFiles) > 0
+	if hasUntrackedFiles {
+		sayInfo("untracked files present:")
+		sayInfo(untrackedFiles)
+	}
+}
+
+func sayUnstagedChangesInfo() {
+	unstagedChanges := getUnstagedChanges()
+	hasUnstagedChanges := len(unstagedChanges) > 0
+	if hasUnstagedChanges {
+		sayInfo("unstaged changes present:")
+		sayInfo(unstagedChanges)
+	}
+}
+
+func hasQualifiedBranches(currentBaseBranch string, remoteBranches []string) bool {
+	debugInfo("check on current base branch " + currentBaseBranch + " with remote branches " + strings.Join(remoteBranches, ","))
+	hasWipBranchesWithQualifier := strings.Contains(strings.Join(remoteBranches, "\n"), configuration.RemoteName+"/"+wipBranchPrefix+currentBaseBranch+configuration.WipBranchQualifierSeparator)
 	return hasWipBranchesWithQualifier
 }
 
 func startJoinMobSession() {
-	_, currentWipBranch := determineBranches(gitCurrentBranch(), configuration.WipBranchQualifier, gitBranches())
+	_, currentWipBranch := determineBranches(gitCurrentBranch(), gitBranches())
 
 	sayInfo("joining existing mob session from " + configuration.RemoteName + "/" + currentWipBranch)
 	git("checkout", "-B", currentWipBranch, configuration.RemoteName+"/"+currentWipBranch)
@@ -464,7 +502,7 @@ func startJoinMobSession() {
 }
 
 func startNewMobSession() {
-	currentBaseBranch, currentWipBranch := determineBranches(gitCurrentBranch(), configuration.WipBranchQualifier, gitBranches())
+	currentBaseBranch, currentWipBranch := determineBranches(gitCurrentBranch(), gitBranches())
 
 	sayInfo("starting new mob session from " + configuration.RemoteName + "/" + currentBaseBranch)
 	git("checkout", "-B", currentWipBranch, configuration.RemoteName+"/"+currentBaseBranch)
@@ -497,7 +535,7 @@ func next() {
 		return
 	}
 
-	currentBaseBranch, currentWipBranch := determineBranches(gitCurrentBranch(), configuration.WipBranchQualifier, gitBranches())
+	currentBaseBranch, currentWipBranch := determineBranches(gitCurrentBranch(), gitBranches())
 
 	if isNothingToCommit() {
 		if hasLocalCommits(currentWipBranch) {
@@ -536,7 +574,7 @@ func done() {
 
 	git("fetch", configuration.RemoteName, "--prune")
 
-	currentBaseBranch, currentWipBranch := determineBranches(gitCurrentBranch(), configuration.WipBranchQualifier, gitBranches())
+	currentBaseBranch, currentWipBranch := determineBranches(gitCurrentBranch(), gitBranches())
 
 	if hasRemoteBranch(currentWipBranch) {
 		if !isNothingToCommit() {
@@ -568,13 +606,13 @@ func status() {
 	if isMobProgramming() {
 		sayInfo("you are mob programming")
 
-		currentBaseBranch, currentWipBranch := determineBranches(gitCurrentBranch(), configuration.WipBranchQualifier, gitBranches())
+		currentBaseBranch, currentWipBranch := determineBranches(gitCurrentBranch(), gitBranches())
 		sayInfo("on wip branch " + currentWipBranch + " (base branch " + currentBaseBranch + ")")
 
 		say(silentgit("--no-pager", "log", currentBaseBranch+".."+currentWipBranch, "--pretty=format:%h %cr <%an>", "--abbrev-commit"))
 	} else {
 		sayInfo("you aren't mob programming")
-		currentBaseBranch, currentWipBranch := determineBranches(gitCurrentBranch(), configuration.WipBranchQualifier, gitBranches())
+		currentBaseBranch, currentWipBranch := determineBranches(gitCurrentBranch(), gitBranches())
 		sayInfo("on base branch " + currentBaseBranch + " (wip branch " + currentWipBranch + ")")
 
 		sayTodo("to start mob programming, use", "mob start")
@@ -600,17 +638,16 @@ func hasUncommittedChanges() bool {
 
 func isMobProgramming() bool {
 	currentBranch := gitCurrentBranch()
-	_, currentWipBranch := determineBranches(currentBranch, configuration.WipBranchQualifier, gitBranches())
+	_, currentWipBranch := determineBranches(currentBranch, gitBranches())
 	debugInfo("current branch " + currentBranch + " and currentWipBranch " + currentWipBranch)
 	return currentWipBranch == currentBranch
 }
 
 func hasLocalBranch(localBranch string) bool {
-	localBranchesRaw := gitBranches()
-	debugInfo("Local Branches: " + localBranchesRaw)
+	localBranches := gitBranches()
+	debugInfo("Local Branches: " + strings.Join(localBranches, "\n"))
 	debugInfo("Local Branch: " + localBranch)
 
-	localBranches := strings.Split(localBranchesRaw, "\n")
 	for i := 0; i < len(localBranches); i++ {
 		if localBranches[i] == localBranch {
 			return true
@@ -621,12 +658,11 @@ func hasLocalBranch(localBranch string) bool {
 }
 
 func hasRemoteBranch(branch string) bool {
-	remoteBranchesRaw := gitRemoteBranches()
+	remoteBranches := gitRemoteBranches()
 	remoteBranch := configuration.RemoteName + "/" + branch
-	debugInfo("Remote Branches: " + remoteBranchesRaw)
+	debugInfo("Remote Branches: " + strings.Join(remoteBranches, "\n"))
 	debugInfo("Remote Branch: " + remoteBranch)
 
-	remoteBranches := strings.Split(remoteBranchesRaw, "\n")
 	for i := 0; i < len(remoteBranches); i++ {
 		if remoteBranches[i] == remoteBranch {
 			return true
@@ -636,12 +672,12 @@ func hasRemoteBranch(branch string) bool {
 	return false
 }
 
-func gitBranches() string {
-	return strings.TrimSpace(silentgit("branch", "--format=%(refname:short)"))
+func gitBranches() []string {
+	return strings.Split(strings.TrimSpace(silentgit("branch", "--format=%(refname:short)")), "\n")
 }
 
-func gitRemoteBranches() string {
-	return strings.TrimSpace(silentgit("branch", "--remotes", "--format=%(refname:short)"))
+func gitRemoteBranches() []string {
+	return strings.Split(strings.TrimSpace(silentgit("branch", "--remotes", "--format=%(refname:short)")), "\n")
 }
 
 func gitCurrentBranch() string {
@@ -656,7 +692,7 @@ func gitUserName() string {
 func showNext() {
 	debugInfo("determining next person based on previous changes")
 
-	currentBaseBranch, currentWipBranch := determineBranches(gitCurrentBranch(), configuration.WipBranchQualifier, gitBranches())
+	currentBaseBranch, currentWipBranch := determineBranches(gitCurrentBranch(), gitBranches())
 
 	changes := strings.TrimSpace(silentgit("--no-pager", "log", currentBaseBranch+".."+currentWipBranch, "--pretty=format:%an", "--abbrev-commit"))
 	lines := strings.Split(strings.Replace(changes, "\r\n", "\n", -1), "\n")


Test patch:
diff --git a/mob_test.go b/mob_test.go
index 6f8ee17..cb634ec 100644
--- a/mob_test.go
+++ b/mob_test.go
@@ -40,26 +40,27 @@ func TestDetermineBranches(t *testing.T) {
 	configuration.WipBranchQualifierSeparator = "-"
 	configuration.Debug = true
 
-	assertDetermineBranches(t, "master", "", "", "master", "mob-session")
-	assertDetermineBranches(t, "mob-session", "", "", "master", "mob-session")
-	assertDetermineBranches(t, "mob-session", "green", "", "master", "mob-session")
+	assertDetermineBranches(t, "master", "", []string{}, "master", "mob-session")
+	assertDetermineBranches(t, "mob-session", "", []string{}, "master", "mob-session")
+	assertDetermineBranches(t, "mob-session", "green", []string{}, "master", "mob-session")
 
-	assertDetermineBranches(t, "master", "green", "", "master", "mob/master-green")
-	assertDetermineBranches(t, "mob/master-green", "", "", "master", "mob/master-green")
+	assertDetermineBranches(t, "master", "green", []string{}, "master", "mob/master-green")
+	assertDetermineBranches(t, "mob/master-green", "", []string{}, "master", "mob/master-green")
 
-	assertDetermineBranches(t, "feature1", "", "", "feature1", "mob/feature1")
-	assertDetermineBranches(t, "mob/feature1", "", "", "feature1", "mob/feature1")
-	assertDetermineBranches(t, "mob/feature1-green", "", "", "feature1", "mob/feature1-green")
-	assertDetermineBranches(t, "feature1", "green", "", "feature1", "mob/feature1-green")
+	assertDetermineBranches(t, "feature1", "", []string{}, "feature1", "mob/feature1")
+	assertDetermineBranches(t, "mob/feature1", "", []string{}, "feature1", "mob/feature1")
+	assertDetermineBranches(t, "mob/feature1-green", "", []string{}, "feature1", "mob/feature1-green")
+	assertDetermineBranches(t, "feature1", "green", []string{}, "feature1", "mob/feature1-green")
 
-	assertDetermineBranches(t, "feature/test", "", "feature/test", "feature/test", "mob/feature/test")
-	assertDetermineBranches(t, "mob/feature/test", "", "feature/test\nmob/feature/test", "feature/test", "mob/feature/test")
+	assertDetermineBranches(t, "feature/test", "", []string{"feature/test"}, "feature/test", "mob/feature/test")
+	assertDetermineBranches(t, "mob/feature/test", "", []string{"feature/test", "mob/feature/test"}, "feature/test", "mob/feature/test")
 
-	assertDetermineBranches(t, "feature/test-ch", "", "DPL-2638-update-apis\nDPL-2814-create-project\nfeature/test-ch\nfix/smallChanges\nmaster\npipeship/pipelineupdate-pipeship-pipeline.yaml\n", "feature/test-ch", "mob/feature/test-ch")
+	assertDetermineBranches(t, "feature/test-ch", "", []string{"DPL-2638-update-apis", "DPL-2814-create-project", "feature/test-ch", "fix/smallChanges", "master", "pipeship/pipelineupdate-pipeship-pipeline.yaml"}, "feature/test-ch", "mob/feature/test-ch")
 }
 
-func assertDetermineBranches(t *testing.T, branch string, qualifier string, branches string, expectedBase string, expectedWip string) {
-	baseBranch, wipBranch := determineBranches(branch, qualifier, branches)
+func assertDetermineBranches(t *testing.T, branch string, qualifier string, branches []string, expectedBase string, expectedWip string) {
+	configuration.WipBranchQualifier = qualifier
+	baseBranch, wipBranch := determineBranches(branch, branches)
 	equals(t, expectedBase, baseBranch)
 	equals(t, expectedWip, wipBranch)
 }
@@ -575,8 +576,8 @@ func setup(t *testing.T) *string {
 	output := captureOutput()
 	createTestbed(t)
 	assertOnBranch(t, "master")
-	equals(t, "master", gitBranches())
-	equals(t, "origin/master", gitRemoteBranches())
+	equals(t, []string{"master"}, gitBranches())
+	equals(t, []string{"origin/master"}, gitRemoteBranches())
 	assertNoMobSessionBranches(t, "mob-session")
 	return output
 }