Refactoring: remove context switching, another way to catch panics, colored output. #96

cmd/context.go

@@ -22,9 +22,8 @@ import (
 
 // AptlyContext is a common context shared by all commands
 type AptlyContext struct {
-	flags        *flag.FlagSet
+	flags, globalFlags *flag.FlagSet
-	configLoaded bool
+	configLoaded       bool
-	panicked     bool
 
 	progress          aptly.Progress
 	downloader        aptly.Downloader
@@ -41,8 +40,6 @@ type AptlyContext struct {
 }
 
 var context *AptlyContext
-var savedContext *AptlyContext
-var tempContext *AptlyContext
 
 // Check interface
 var _ aptly.PublishedStorageProvider = &AptlyContext{}
@@ -60,26 +57,15 @@ func Fatal(err error) {
 	if err == commander.ErrFlagError || err == commander.ErrCommandError {
 		returnCode = 2
 	}
-	if context != nil {
-		context.panicked = true
-	}
 	panic(&FatalError{ReturnCode: returnCode, Message: err.Error()})
 }
 
-func switchContext() {
-
-	tempContext = context
-	context = savedContext
-	savedContext = tempContext
-
-}
-
 // Config loads and returns current configuration
 func (context *AptlyContext) Config() *utils.ConfigStructure {
 	if !context.configLoaded {
 		var err error
 
-		configLocation := context.flags.Lookup("config").Value.String()
+		configLocation := context.globalFlags.Lookup("config").Value.String()
 		if configLocation != "" {
 			err = utils.LoadConfig(configLocation, &utils.Config)
 
@@ -118,16 +104,16 @@ func (context *AptlyContext) Config() *utils.ConfigStructure {
 func (context *AptlyContext) DependencyOptions() int {
 	if context.dependencyOptions == -1 {
 		context.dependencyOptions = 0
-		if context.Config().DepFollowSuggests || context.flags.Lookup("dep-follow-suggests").Value.Get().(bool) {
+		if context.Config().DepFollowSuggests || context.globalFlags.Lookup("dep-follow-suggests").Value.Get().(bool) {
 			context.dependencyOptions |= deb.DepFollowSuggests
 		}
-		if context.Config().DepFollowRecommends || context.flags.Lookup("dep-follow-recommends").Value.Get().(bool) {
+		if context.Config().DepFollowRecommends || context.globalFlags.Lookup("dep-follow-recommends").Value.Get().(bool) {
 			context.dependencyOptions |= deb.DepFollowRecommends
 		}
-		if context.Config().DepFollowAllVariants || context.flags.Lookup("dep-follow-all-variants").Value.Get().(bool) {
+		if context.Config().DepFollowAllVariants || context.globalFlags.Lookup("dep-follow-all-variants").Value.Get().(bool) {
 			context.dependencyOptions |= deb.DepFollowAllVariants
 		}
-		if context.Config().DepFollowSource || context.flags.Lookup("dep-follow-source").Value.Get().(bool) {
+		if context.Config().DepFollowSource || context.globalFlags.Lookup("dep-follow-source").Value.Get().(bool) {
 			context.dependencyOptions |= deb.DepFollowSource
 		}
 	}
@@ -139,7 +125,7 @@ func (context *AptlyContext) DependencyOptions() int {
 func (context *AptlyContext) ArchitecturesList() []string {
 	if context.architecturesList == nil {
 		context.architecturesList = context.Config().Architectures
-		optionArchitectures := context.flags.Lookup("architectures").Value.String()
+		optionArchitectures := context.globalFlags.Lookup("architectures").Value.String()
 		if optionArchitectures != "" {
 			context.architecturesList = strings.Split(optionArchitectures, ",")
 		}
@@ -244,6 +230,10 @@ func (context *AptlyContext) GetPublishedStorage(name string) aptly.PublishedSto
 	return publishedStorage
 }
 
+func (context *AptlyContext) UpdateFlags(flags *flag.FlagSet) {
+	context.flags = flags
+}
+
 // ShutdownContext shuts context down
 func ShutdownContext() {
 	if aptly.EnableDebug {
@@ -264,12 +254,27 @@ func ShutdownContext() {
 	}
 	if context.database != nil {
 		context.database.Close()
+		context.database = nil
 	}
 	if context.downloader != nil {
 		context.downloader.Shutdown()
+		context.downloader = nil
 	}
 	if context.progress != nil {
 		context.progress.Shutdown()
+		context.progress = nil
+	}
+}
+
+// CleanupContext does partial shutdown of context
+func CleanupContext() {
+	if context.downloader != nil {
+		context.downloader.Shutdown()
+		context.downloader = nil
+	}
+	if context.progress != nil {
+		context.progress.Shutdown()
+		context.progress = nil
 	}
 }
 
@@ -277,9 +282,13 @@ func ShutdownContext() {
 func InitContext(flags *flag.FlagSet) error {
 	var err error
 
+	if context != nil {
+		panic("context already initialized")
+	}
+
 	context = &AptlyContext{
 		flags:             flags,
-		panicked:          false,
+		globalFlags:       flags,
 		dependencyOptions: -1,
 		publishedStorages: map[string]aptly.PublishedStorage{},
 	}

cmd/run.go

@@ -2,11 +2,10 @@ package cmd
 
 import (
 	"fmt"
-	"os"
+	"github.com/smira/commander"
 )
 
-func Run(cmd_args []string, exitOnPanic bool) {
+func Run(cmd *commander.Command, cmd_args []string, initContext bool) (returnCode int) {
-
 	defer func() {
 		if r := recover(); r != nil {
 			fatal, ok := r.(*FatalError)
@@ -14,29 +13,31 @@ func Run(cmd_args []string, exitOnPanic bool) {
 				panic(r)
 			}
 			fmt.Println("ERROR:", fatal.Message)
-			if exitOnPanic {
+			returnCode = fatal.ReturnCode
-				os.Exit(fatal.ReturnCode)
-			}
 		}
 	}()
 
-	command := RootCommand()
+	returnCode = 0
 
-	flags, args, err := command.ParseFlags(cmd_args)
+	flags, args, err := cmd.ParseFlags(cmd_args)
 	if err != nil {
 		Fatal(err)
 	}
 
-	err = InitContext(flags)
+	if initContext {
-	if err != nil {
+		err = InitContext(flags)
-		Fatal(err)
+		if err != nil {
-	}
+			Fatal(err)
-
+		}
-	defer ShutdownContext()
+		defer ShutdownContext()
-
+	}
-	err = command.Dispatch(args)
+
+	context.UpdateFlags(flags)
+
+	err = cmd.Dispatch(args)
 	if err != nil {
 		Fatal(err)
 	}
 
+	return
 }

cmd/task_run.go

@@ -8,7 +8,6 @@ import (
 
 	"github.com/mattn/go-shellwords"
 	"github.com/smira/commander"
-	"github.com/wsxiaoys/terminal/color"
 )
 
 func aptlyTaskRun(cmd *commander.Command, args []string) error {
@@ -80,27 +79,29 @@ func aptlyTaskRun(cmd *commander.Command, args []string) error {
 		cmd_list = formatCommands(args)
 	}
 
-	switchContext()
+	commandErrored := false
 
 	for i, command := range cmd_list {
+		if !commandErrored {
+			context.Progress().ColoredPrintf("@g%d) [Running]: %s@!", (i + 1), strings.Join(command, " "))
+			context.Progress().ColoredPrintf("\n@yBegin command output: ----------------------------@!")
+			context.Progress().Flush()
 
-		if context == nil || !context.panicked {
+			returnCode := Run(RootCommand(), command, false)
-			color.Printf("@g%d) [Running]: %s@!\n", (i + 1), strings.Join(command, " "))
+			if returnCode != 0 {
-			color.Println("\n@yBegin command output: ----------------------------\n@!")
+				commandErrored = true
-			Run(command, false)
+			}
-			color.Println("\n@yEnd command output: ------------------------------\n@!")
+			context.Progress().ColoredPrintf("\n@yEnd command output: ------------------------------@!")
-
+			CleanupContext()
 		} else {
-			color.Printf("@r%d) [Skipping]: %s@!\n", (i + 1), strings.Join(command, " "))
+			context.Progress().ColoredPrintf("@r%d) [Skipping]: %s@!", (i + 1), strings.Join(command, " "))
 		}
-
 	}
-	if context.panicked {
+
+	if commandErrored {
 		err = fmt.Errorf("At least one command has reported an error\n")
 	}
 
-	switchContext()
-
 	return err
 }
 

main.go

@@ -6,5 +6,5 @@ import (
 )
 
 func main() {
-	cmd.Run(os.Args[1:], true)
+	os.Exit(cmd.Run(cmd.RootCommand(), os.Args[1:], true))
 }

system/t10_task/RunTask1Test_gold

@@ -1,34 +1,26 @@
-1) [Running]: repo list
+1) [Running]: repo list
-
+
-Begin command output: ----------------------------
+Begin command output: ----------------------------
- 
 No local repositories found, create one with `aptly repo create ...`.
 
-End command output: ------------------------------
+End command output: ------------------------------
- 
+2) [Running]: repo create local
-2) [Running]: repo create local
+
-
+Begin command output: ----------------------------
-Begin command output: ----------------------------
- 
 
 Local repo [local] successfully added.
 You can run 'aptly repo add local ...' to add packages to repository.
 
-End command output: ------------------------------
+End command output: ------------------------------
- 
-3) [Running]: repo drop local
-
-Begin command output: ----------------------------
- 
 Local repo `local` has been removed.
+3) [Running]: repo drop local
 
-End command output: ------------------------------
+Begin command output: ----------------------------
- 
+
-4) [Running]: version
+End command output: ------------------------------
-
-Begin command output: ----------------------------
- 
 aptly version: 0.8~dev
+4) [Running]: version
 
-End command output: ------------------------------
+Begin command output: ----------------------------
- 
+
+End command output: ------------------------------