Refactor uncommitted changes handling

Unified configuration handling for uncommitted changes into `HandleUncommittedChanges` and added corresponding constants. Updated tests and logic to reflect this new approach, ensuring better maintainability and clarity.
remotemobprogramming · Sep 23, 2024 · 6d1e005 · 6d1e005
1 parent 883b38d
commit 6d1e005
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 62 deletions.
diff --git a/configuration/configuration.go b/configuration/configuration.go
@@ -15,6 +15,12 @@ const (
 	SquashWip = "squash-wip"
 )
 
+const (
+	IncludeChanges = "include-changes"
+	DiscardChanges = "discard-changes"
+	FailWithError  = "fail-with-error"
+)
+
 type Configuration struct {
 	CliName                        string // override with MOB_CLI_NAME
 	RemoteName                     string // override with MOB_REMOTE_NAME
@@ -28,8 +34,7 @@ type Configuration struct {
 	NotifyCommand                  string // override with MOB_NOTIFY_COMMAND
 	NotifyMessage                  string // override with MOB_NOTIFY_MESSAGE
 	NextStay                       bool   // override with MOB_NEXT_STAY
-	StartDiscardUncommittedChanges bool
-	StartIncludeUncommittedChanges bool
+	HandleUncommittedChanges       string
 	StartCreate                    bool // override with MOB_START_CREATE variable
 	StartJoin                      bool
 	StashName                      string // override with MOB_STASH_NAME
@@ -120,9 +125,9 @@ func ParseArgs(args []string, configuration Configuration) (command string, para
 		arg := args[i]
 		switch arg {
 		case "--discard-uncommitted-changes", "-d":
-			newConfiguration.StartDiscardUncommittedChanges = true
+			newConfiguration.HandleUncommittedChanges = DiscardChanges
 		case "--include-uncommitted-changes", "-i":
-			newConfiguration.StartIncludeUncommittedChanges = true
+			newConfiguration.HandleUncommittedChanges = IncludeChanges
 		case "--debug":
 			// ignore this, already parsed
 		case "--stay", "-s":
@@ -184,33 +189,32 @@ func GetDefaultConfiguration() Configuration {
 
 	}
 	return Configuration{
-		CliName:                        "mob",
-		RemoteName:                     "origin",
-		WipCommitMessage:               "mob next [ci-skip] [ci skip] [skip ci]",
-		StartCommitMessage:             "mob start [ci-skip] [ci skip] [skip ci]",
-		SkipCiPushOptionEnabled:        true,
-		GitHooksEnabled:                false,
-		VoiceCommand:                   voiceCommand,
-		VoiceMessage:                   "mob next",
-		NotifyCommand:                  notifyCommand,
-		NotifyMessage:                  "mob next",
-		NextStay:                       true,
-		RequireCommitMessage:           false,
-		StartDiscardUncommittedChanges: false,
-		StartIncludeUncommittedChanges: false,
-		StartCreate:                    false,
-		WipBranchQualifier:             "",
-		WipBranchQualifierSeparator:    "-",
-		DoneSquash:                     Squash,
-		OpenCommand:                    "",
-		Timer:                          "",
-		TimerLocal:                     true,
-		TimerRoom:                      "",
-		TimerUser:                      "",
-		TimerUrl:                       "https://timer.mob.sh/",
-		WipBranchPrefix:                "mob/",
-		StashName:                      "mob-stash-name",
-		ResetDeleteRemoteWipBranch:     false,
+		CliName:                     "mob",
+		RemoteName:                  "origin",
+		WipCommitMessage:            "mob next [ci-skip] [ci skip] [skip ci]",
+		StartCommitMessage:          "mob start [ci-skip] [ci skip] [skip ci]",
+		SkipCiPushOptionEnabled:     true,
+		GitHooksEnabled:             false,
+		VoiceCommand:                voiceCommand,
+		VoiceMessage:                "mob next",
+		NotifyCommand:               notifyCommand,
+		NotifyMessage:               "mob next",
+		NextStay:                    true,
+		RequireCommitMessage:        false,
+		HandleUncommittedChanges:    FailWithError,
+		StartCreate:                 false,
+		WipBranchQualifier:          "",
+		WipBranchQualifierSeparator: "-",
+		DoneSquash:                  Squash,
+		OpenCommand:                 "",
+		Timer:                       "",
+		TimerLocal:                  true,
+		TimerRoom:                   "",
+		TimerUser:                   "",
+		TimerUrl:                    "https://timer.mob.sh/",
+		WipBranchPrefix:             "mob/",
+		StashName:                   "mob-stash-name",
+		ResetDeleteRemoteWipBranch:  false,
 	}
 }
 

diff --git a/configuration/configuration_test.go b/configuration/configuration_test.go
@@ -103,14 +103,24 @@ func TestParseArgsStartRoom(t *testing.T) {
 	test.Equals(t, "testroom", configuration.TimerRoom)
 }
 
+func TestDefaultConfigurationHandleUncommitedChanges(t *testing.T) {
+	configuration := GetDefaultConfiguration()
+
+	command, parameters, configuration := ParseArgs([]string{"mob", "start"}, configuration)
+
+	test.Equals(t, "start", command)
+	test.Equals(t, 0, len(parameters))
+	test.Equals(t, FailWithError, configuration.HandleUncommittedChanges)
+}
+
 func TestParseArgsIncludeUncommitedChanges(t *testing.T) {
 	configuration := GetDefaultConfiguration()
 
 	command, parameters, configuration := ParseArgs([]string{"mob", "start", "--include-uncommitted-changes"}, configuration)
 
 	test.Equals(t, "start", command)
 	test.Equals(t, 0, len(parameters))
-	test.Equals(t, true, configuration.StartIncludeUncommittedChanges)
+	test.Equals(t, IncludeChanges, configuration.HandleUncommittedChanges)
 }
 
 func TestParseArgsIncludeUncommitedChangesShort(t *testing.T) {
@@ -120,7 +130,7 @@ func TestParseArgsIncludeUncommitedChangesShort(t *testing.T) {
 
 	test.Equals(t, "start", command)
 	test.Equals(t, 0, len(parameters))
-	test.Equals(t, true, configuration.StartIncludeUncommittedChanges)
+	test.Equals(t, IncludeChanges, configuration.HandleUncommittedChanges)
 }
 
 func TestParseArgsDiscardUncommitedChanges(t *testing.T) {
@@ -130,7 +140,7 @@ func TestParseArgsDiscardUncommitedChanges(t *testing.T) {
 
 	test.Equals(t, "start", command)
 	test.Equals(t, 0, len(parameters))
-	test.Equals(t, true, configuration.StartDiscardUncommittedChanges)
+	test.Equals(t, DiscardChanges, configuration.HandleUncommittedChanges)
 }
 
 func TestParseArgsDiscardUncommitedChangesShort(t *testing.T) {
@@ -140,7 +150,7 @@ func TestParseArgsDiscardUncommitedChangesShort(t *testing.T) {
 
 	test.Equals(t, "start", command)
 	test.Equals(t, 0, len(parameters))
-	test.Equals(t, true, configuration.StartDiscardUncommittedChanges)
+	test.Equals(t, DiscardChanges, configuration.HandleUncommittedChanges)
 }
 
 func TestParseArgsTimerRoom(t *testing.T) {
@@ -228,10 +238,6 @@ func (c Configuration) GetMobDoneSquash() string {
 	return c.DoneSquash
 }
 
-func (c Configuration) GetMobStartIncludeUncommittedChanges() bool {
-	return c.StartIncludeUncommittedChanges
-}
-
 func (c Configuration) GetMobStartCreateRemoteBranch() bool {
 	return c.StartCreate
 }

diff --git a/mob.go b/mob.go
@@ -527,7 +527,7 @@ func deleteRemoteWipBranch(configuration config.Configuration) {
 
 func start(configuration config.Configuration) error {
 	uncommittedChanges := hasUncommittedChanges()
-	if uncommittedChanges && !configuration.StartIncludeUncommittedChanges && !configuration.StartDiscardUncommittedChanges {
+	if uncommittedChanges && configuration.HandleUncommittedChanges == config.FailWithError {
 		say.Info("cannot start; clean working tree required")
 		sayUnstagedChangesInfo()
 		sayUntrackedFilesInfo()
@@ -557,22 +557,16 @@ func start(configuration config.Configuration) error {
 		return errors.New("cannot start; unpushed changes on base branch must be pushed upstream")
 	}
 
-	if uncommittedChanges && configuration.StartIncludeUncommittedChanges && configuration.StartDiscardUncommittedChanges {
-		errorMessage := "The options '--include-uncommitted-changes' (-i) and '--discard-uncommitted-changes' (-d) cannot be used together as they are mutually exclusive. Please choose either to include or discard uncommitted changes, but not both."
-		say.Error(errorMessage)
-		return errors.New(errorMessage)
-	}
-	if uncommittedChanges && configuration.StartDiscardUncommittedChanges {
+	if uncommittedChanges && configuration.HandleUncommittedChanges == config.DiscardChanges {
 		git("reset", "--hard")
 	}
 
-	if uncommittedChanges && configuration.StartIncludeUncommittedChanges && silentgit("ls-tree", "-r", "HEAD", "--full-name", "--name-only", ".") == "" {
-		say.Error("cannot start; current working dir is an uncommitted subdir")
-		say.Fix("to fix this, go to the parent directory and try again", "cd ..")
-		return errors.New("cannot start; current working dir is an uncommitted subdir")
-	}
-
-	if uncommittedChanges && configuration.StartIncludeUncommittedChanges {
+	if uncommittedChanges && configuration.HandleUncommittedChanges == config.IncludeChanges {
+		if silentgit("ls-tree", "-r", "HEAD", "--full-name", "--name-only", ".") == "" {
+			say.Error("cannot start; current working dir is an uncommitted subdir")
+			say.Fix("to fix this, go to the parent directory and try again", "cd ..")
+			return errors.New("cannot start; current working dir is an uncommitted subdir")
+		}
 		git("stash", "push", "--include-untracked", "--message", configuration.StashName)
 		say.Info("uncommitted changes were stashed. If an error occurs later on, you can recover them with 'git stash pop'.")
 	}
@@ -589,7 +583,7 @@ func start(configuration config.Configuration) error {
 		startNewMobSession(configuration)
 	}
 
-	if uncommittedChanges && configuration.StartIncludeUncommittedChanges {
+	if uncommittedChanges && configuration.HandleUncommittedChanges == config.IncludeChanges {
 		stashes := silentgit("stash", "list")
 		stash := findStashByName(stashes, configuration.StashName)
 		git("stash", "pop", stash)

diff --git a/mob_test.go b/mob_test.go
@@ -532,7 +532,7 @@ func TestCleanMissingBaseBranch(t *testing.T) {
 
 func TestStartUnstagedChanges(t *testing.T) {
 	output, configuration := setup(t)
-	configuration.StartIncludeUncommittedChanges = false
+	configuration.HandleUncommittedChanges = config.FailWithError
 	createFile(t, "test.txt", "contentIrrelevant")
 
 	start(configuration)
@@ -545,7 +545,7 @@ func TestStartUnstagedChanges(t *testing.T) {
 
 func TestStartIncludeUnstagedChanges(t *testing.T) {
 	_, configuration := setup(t)
-	configuration.StartIncludeUncommittedChanges = true
+	configuration.HandleUncommittedChanges = config.IncludeChanges
 	createFile(t, "test.txt", "contentIrrelevant")
 
 	start(configuration)
@@ -556,7 +556,7 @@ func TestStartIncludeUnstagedChanges(t *testing.T) {
 
 func TestStartDiscardUnstagedChanges(t *testing.T) {
 	_, configuration := setup(t)
-	configuration.StartDiscardUncommittedChanges = true
+	configuration.HandleUncommittedChanges = config.DiscardChanges
 	createFile(t, "test.txt", "contentIrrelevant")
 
 	start(configuration)
@@ -568,7 +568,7 @@ func TestStartDiscardUnstagedChanges(t *testing.T) {
 
 func TestStartIncludeUnstagedChangesInNewWorkingDirectory(t *testing.T) {
 	output, configuration := setup(t)
-	configuration.StartIncludeUncommittedChanges = true
+	configuration.HandleUncommittedChanges = config.IncludeChanges
 	createDirectory(t, "subdirnew")
 	setWorkingDir(tempDir + "/local/subdirnew")
 	createFile(t, "test.txt", "contentIrrelevant")
@@ -600,7 +600,7 @@ func TestBranch(t *testing.T) {
 
 func TestStartIncludeUntrackedFiles(t *testing.T) {
 	_, configuration := setup(t)
-	configuration.StartIncludeUncommittedChanges = true
+	configuration.HandleUncommittedChanges = config.IncludeChanges
 	createFile(t, "example.txt", "contentIrrelevant")
 
 	start(configuration)
@@ -610,7 +610,7 @@ func TestStartIncludeUntrackedFiles(t *testing.T) {
 
 func TestStartUntrackedFiles(t *testing.T) {
 	_, configuration := setup(t)
-	configuration.StartIncludeUncommittedChanges = false
+	configuration.HandleUncommittedChanges = config.FailWithError
 	createFile(t, "example.txt", "contentIrrelevant")
 
 	start(configuration)
@@ -683,7 +683,7 @@ func TestStartCreateIncludeUncommitedChangesOnUnpushedFeatureBranchWithUncommite
 	createFile(t, "file.txt", "contentIrrelevant")
 
 	configuration.StartCreate = true
-	configuration.StartIncludeUncommittedChanges = true
+	configuration.HandleUncommittedChanges = config.IncludeChanges
 	start(configuration)
 
 	assertOnBranch(t, "mob/feature1")
@@ -695,7 +695,7 @@ func TestStartCreateIncludeUncommitedChangesOnUnpushedFeatureBranchWithUncommite
 	createFile(t, "file.txt", "contentIrrelevant")
 
 	configuration.StartCreate = true
-	configuration.StartIncludeUncommittedChanges = true
+	configuration.HandleUncommittedChanges = config.IncludeChanges
 	configuration.WipBranchQualifier = "green"
 	start(configuration)