From 15bdc92e8747cf40e4a1ebe3139c4a317ae8b784 Mon Sep 17 00:00:00 2001 From: Jens Feodor Nielsen Date: Tue, 7 May 2024 08:32:47 +0200 Subject: [PATCH] fix(cli): improved output for commands, and minor refactors Use common error printing functions, and improve the phrasing of some error and informative messages. --- cli/cmd/delete/delete.go | 34 ++++++++++----------------- cli/cmd/initialize/bootstrap_files.go | 21 ++++++++--------- cli/cmd/initialize/files.go | 7 +++--- cli/cmd/initialize/init.go | 17 +++++++++----- cli/cmd/initialize/wizard/folder.go | 4 ++-- cli/cmd/list/list.go | 11 ++++++--- cli/cmd/log/log.go | 8 +++---- cli/cmd/log/log_events.go | 14 ++++++++--- cli/cmd/output/error.go | 9 +++++-- cli/cmd/publish/publish.go | 12 ++-------- cli/cmd/push/push.go | 15 +++++++----- cli/cmd/unpublish/unpublish.go | 22 +++++------------ cli/tool/tool.go | 15 ++++++++++++ 13 files changed, 100 insertions(+), 89 deletions(-) diff --git a/cli/cmd/delete/delete.go b/cli/cmd/delete/delete.go index fec701b..f7e9367 100644 --- a/cli/cmd/delete/delete.go +++ b/cli/cmd/delete/delete.go @@ -5,6 +5,7 @@ import ( "fmt" "os" + "numerous/cli/cmd/output" "numerous/cli/internal/gql" "numerous/cli/internal/gql/app" "numerous/cli/tool" @@ -28,45 +29,36 @@ This action cannot be undone.`, func deleteApp(client *gqlclient.Client, args []string) error { var appID string - var err error - if len(args) == 1 { appID = args[0] + } else if readAppID, err := tool.ReadAppIDAndPrintErrors("."); err != nil { + return err } else { - appID, err = tool.ReadAppID(".") - if err == tool.ErrAppIDNotFound { - fmt.Println("Sorry, we could not recognize your app in the specified directory.", - "\nRun \"numerous init\" to initialize the app in Numerous.") - - return err - } else if err != nil { - fmt.Println("Whoops! An error occurred when reading the app ID. \n Please make sure you are in the correct directory and try again.") - fmt.Println("Error: ", err) - - return err - } + appID = readAppID } if _, err := app.Query(appID, client); err != nil { - fmt.Println( - "Sorry, we could not find the app in our database. \nPlease, make sure that the App ID in the .tool_id.txt file is correct and try again.") + output.PrintError( + "Sorry, we could not find the app in our database.", + "Please, make sure that the App ID in the %q file is correct and try again.", + tool.AppIDFileName, + ) return err } if result, err := app.Delete(appID, client); err != nil { - fmt.Println("An error occurred while removing the app from Numerous. Please try again.") - fmt.Println("Error: ", err) + output.PrintUnknownError(err) return err } else { if result.ToolDelete.Typename == "ToolDeleteSuccess" { fmt.Println("The app has been successfully removed from Numerous") } else if result.ToolDelete.Typename == "ToolDeleteFailure" { - fmt.Println("An error occurred while removing the app from Numerous. Please try again.") - fmt.Println("Error: ", result.ToolDelete.Result) + err := errors.New(result.ToolDelete.Result) + output.PrintUnknownError(err) - return errors.New(result.ToolDelete.Result) + return err } return nil diff --git a/cli/cmd/initialize/bootstrap_files.go b/cli/cmd/initialize/bootstrap_files.go index 3148edc..ed928ae 100644 --- a/cli/cmd/initialize/bootstrap_files.go +++ b/cli/cmd/initialize/bootstrap_files.go @@ -2,12 +2,12 @@ package initialize import ( "bytes" - "fmt" "io/fs" "os" "path/filepath" "numerous/cli/assets" + "numerous/cli/cmd/output" "numerous/cli/manifest" "numerous/cli/tool" ) @@ -15,28 +15,27 @@ import ( func bootstrapFiles(t tool.Tool, toolID string, basePath string) (err error) { manifestToml, err := manifest.FromTool(t).ToToml() if err != nil { - fmt.Println("Error encoding manifest file") + output.PrintErrorDetails("Error encoding manifest file", err) return err } - err = createAppIDFile(basePath, toolID) - if err != nil { + if err = createAppIDFile(basePath, toolID); err != nil { return err } - if err = addToGitIgnore(basePath, "# added by numerous init\n"+tool.ToolIDFileName); err != nil { + + if err = addToGitIgnore(basePath, "# added by numerous init\n"+tool.AppIDFileName); err != nil { return err } - if err = CreateAndWriteIfFileNotExist(filepath.Join(basePath, t.AppFile), t.Library.DefaultAppFile()); err != nil { + if err = createAndWriteIfFileNotExist(filepath.Join(basePath, t.AppFile), t.Library.DefaultAppFile()); err != nil { return err } - for _, path := range []string{t.RequirementsFile} { - if err = createFile(filepath.Join(basePath, path)); err != nil { - return err - } + if err = createFile(filepath.Join(basePath, t.RequirementsFile)); err != nil { + return err } + if err := bootstrapRequirements(t, basePath); err != nil { return err } @@ -44,7 +43,7 @@ func bootstrapFiles(t tool.Tool, toolID string, basePath string) (err error) { return err } - if err = CreateAndWriteIfFileNotExist(filepath.Join(basePath, manifest.ManifestPath), manifestToml); err != nil { + if err = createAndWriteIfFileNotExist(filepath.Join(basePath, manifest.ManifestPath), manifestToml); err != nil { return err } diff --git a/cli/cmd/initialize/files.go b/cli/cmd/initialize/files.go index 6d17602..08a085f 100644 --- a/cli/cmd/initialize/files.go +++ b/cli/cmd/initialize/files.go @@ -26,10 +26,10 @@ func createFile(path string) error { return nil } -func CreateAndWriteIfFileNotExist(path string, content string) error { +func createAndWriteIfFileNotExist(path string, content string) error { _, err := os.Stat(path) if err == nil { - fmt.Printf("Skipping creation of app file: %s already exists\n", path) + fmt.Printf("Skipping creation of %q; it already exists\n", path) return nil } @@ -44,8 +44,7 @@ func CreateAndWriteIfFileNotExist(path string, content string) error { defer file.Close() - _, err = file.WriteString(content) - if err != nil { + if _, err = file.WriteString(content); err != nil { output.PrintErrorDetails("Could not write to %q", err, path) } diff --git a/cli/cmd/initialize/init.go b/cli/cmd/initialize/init.go index 9851318..7d2a669 100644 --- a/cli/cmd/initialize/init.go +++ b/cli/cmd/initialize/init.go @@ -8,6 +8,7 @@ import ( "strings" "numerous/cli/cmd/initialize/wizard" + "numerous/cli/cmd/output" "numerous/cli/internal/gql" "numerous/cli/internal/gql/app" "numerous/cli/tool" @@ -50,9 +51,13 @@ func runInit(cmd *cobra.Command, args []string) { } if exist, _ := tool.AppIDExistsInCurrentDir(projectFolderPath); exist { - fmt.Printf("Error: An app is already initialized in '%s'\n", projectFolderPath) - fmt.Println("You can initialize an app in a folder by specifying a path in the command, like below:") - fmt.Println(" numerous init ./my-app-folder") + output.PrintError( + "An app is already initialized in %q", + "💡 You can initialize an app in another folder by specifying a\n"+ + " path in the command, like below:\n\n"+ + "numerous init ./my-app-folder\n\n", + projectFolderPath, + ) return } @@ -65,7 +70,7 @@ func runInit(cmd *cobra.Command, args []string) { setPython(&newApp) if continueBootstrap, err := wizard.RunInitAppWizard(projectFolderPath, &newApp); err != nil { - fmt.Println("Error running initialization wizard:", err) + output.PrintErrorDetails("Error running initialization wizard", err) return } else if !continueBootstrap { return @@ -74,12 +79,12 @@ func runInit(cmd *cobra.Command, args []string) { // Initialize and boostrap project files a, err := app.Create(newApp, gql.GetClient()) if err != nil { - fmt.Printf("Error creating app in the database.\n error: %s)", err) + output.PrintErrorDetails("Error registering app remotely.", err) return } if err := bootstrapFiles(newApp, a.ID, projectFolderPath); err != nil { - fmt.Printf("Error bootstrapping files.\n error: %s)", err) + output.PrintErrorDetails("Error bootstrapping files.", err) return } diff --git a/cli/cmd/initialize/wizard/folder.go b/cli/cmd/initialize/wizard/folder.go index b45c29a..0a97b7c 100644 --- a/cli/cmd/initialize/wizard/folder.go +++ b/cli/cmd/initialize/wizard/folder.go @@ -28,7 +28,7 @@ func createFolderSurvey(folderPath string, in terminal.FileReader) (bool, error) var confirm bool prompt := &survey.Confirm{ - Message: fmt.Sprintf("The selected folder '%s' does not exist. Create it? (default: yes)", folderPath), + Message: fmt.Sprintf("Create new folder %q? (default: yes)", folderPath), Default: true, } @@ -49,7 +49,7 @@ func createFolderSurvey(folderPath string, in terminal.FileReader) (bool, error) func confirmFolderSurvey(folderPath string, in terminal.FileReader) (bool, error) { var confirm bool - msg := fmt.Sprintf("Do you want to use the existing folder '%s' for your app? (default: yes)", folderPath) + msg := fmt.Sprintf("Use the existing folder %q for your app? (default: yes)", folderPath) prompt := &survey.Confirm{Message: msg, Default: true} err := survey.AskOne(prompt, &confirm, func(options *survey.AskOptions) error { options.Stdio.In = in; return nil }) if err != nil { diff --git a/cli/cmd/list/list.go b/cli/cmd/list/list.go index 276bf9e..3e1514f 100644 --- a/cli/cmd/list/list.go +++ b/cli/cmd/list/list.go @@ -5,6 +5,7 @@ import ( "os" "numerous/cli/auth" + "numerous/cli/cmd/output" "numerous/cli/internal/gql" "numerous/cli/internal/gql/app" @@ -17,7 +18,7 @@ var ListCmd = &cobra.Command{ Short: "List all your apps (login required)", Run: func(cmd *cobra.Command, args []string) { if err := list(auth.NumerousTenantAuthenticator, gql.GetClient()); err != nil { - fmt.Println("Error: ", err) + output.PrintUnknownError(err) os.Exit(1) } }, @@ -26,12 +27,16 @@ var ListCmd = &cobra.Command{ func list(a auth.Authenticator, c *gqlclient.Client) error { user := a.GetLoggedInUserFromKeyring() if user == nil { - fmt.Printf("Command requires login.\n Use \"numerous login\" to login or sign up.\n") + output.PrintError( + "Command requires login.", + "Use \"numerous login\" to login or sign up.\n", + ) + return nil } apps, err := app.QueryList(c) if err != nil { - fmt.Println(err) + output.PrintUnknownError(err) return err } diff --git a/cli/cmd/log/log.go b/cli/cmd/log/log.go index 041da32..3a0eecf 100644 --- a/cli/cmd/log/log.go +++ b/cli/cmd/log/log.go @@ -35,11 +35,9 @@ func log(cmd *cobra.Command, args []string) { return } - appID, err := tool.ReadAppID(appDir) - if err == tool.ErrAppIDNotFound { - output.PrintError("Could not find App ID in %q.", "", appDir) - } else if err != nil { - output.PrintUnknownError(err) + appID, err := tool.ReadAppIDAndPrintErrors(appDir) + if err != nil { + return } err = getLogs(appID, timestamps) diff --git a/cli/cmd/log/log_events.go b/cli/cmd/log/log_events.go index 630cc34..dc1b3a3 100644 --- a/cli/cmd/log/log_events.go +++ b/cli/cmd/log/log_events.go @@ -2,12 +2,12 @@ package log import ( "encoding/json" - "fmt" "io" "log/slog" "os" "time" + "numerous/cli/cmd/output" "numerous/cli/internal/gql" "github.com/hasura/go-graphql-client" @@ -31,10 +31,18 @@ func getClient() *graphql.SubscriptionClient { client := gql.GetSubscriptionClient() client = client.OnError(func(sc *graphql.SubscriptionClient, err error) error { if previousError != nil { - fmt.Printf("Error occurred listening for deploy logs. This does not mean that you app will be unavailable.\nFirst error: %s\nSecond error: %s\n", previousError, err) + output.PrintError( + "Error occurred subscribing to app logs", + "This does not mean that you app will be unavailable.\n"+ + "First error: %s\n"+ + "Second error: %s\n", + previousError, err, + ) + return err } - fmt.Printf("Error occurred listening for deploy logs.\nError: %s\nRetrying...\n", err) + + output.PrintErrorDetails("Error occurred subscribing to app logs. Retrying...", err) previousError = err return nil diff --git a/cli/cmd/output/error.go b/cli/cmd/output/error.go index 44e87b1..e663bc8 100644 --- a/cli/cmd/output/error.go +++ b/cli/cmd/output/error.go @@ -19,10 +19,15 @@ func PrintError(header, body string, args ...any) { // Prints an error message with the given header, and a body that contains // the error details. Variadic arguments will be used for string formatting. func PrintErrorDetails(header string, err error, args ...any) { - PrintError(header, "Error details: "+err.Error()) + PrintError(header, "Details: "+err.Error()) } // Prints the given error with a standardized error message. func PrintUnknownError(err error) { - PrintError("Sorry! An unexpected error occurred: %q", err.Error()) + PrintErrorDetails("Sorry! An unexpected error occurred.", err) +} + +func PrintErrorAppNotInitialized() { + PrintError("The current directory is not a numerous app", + "\nrun \"numerous init\" to initialize a numerous app in the current directory") } diff --git a/cli/cmd/publish/publish.go b/cli/cmd/publish/publish.go index 200d893..2d6b143 100644 --- a/cli/cmd/publish/publish.go +++ b/cli/cmd/publish/publish.go @@ -24,16 +24,8 @@ var PublishCmd = &cobra.Command{ } func publish(client *gqlclient.Client) error { - appID, err := tool.ReadAppID(".") - if err == tool.ErrAppIDNotFound { - fmt.Println("The current directory is not a numerous app", - "\nrun \"numerous init\" to initialize a numerous app in the current directory") - - return err - } else if err != nil { - fmt.Println("An error occurred reading the app ID") - fmt.Println("Error: ", err) - + appID, err := tool.ReadAppIDAndPrintErrors(".") + if err != nil { return err } diff --git a/cli/cmd/push/push.go b/cli/cmd/push/push.go index c3faf78..915b5de 100644 --- a/cli/cmd/push/push.go +++ b/cli/cmd/push/push.go @@ -7,6 +7,7 @@ import ( "path/filepath" "strings" + "numerous/cli/cmd/output" "numerous/cli/internal/gql" "numerous/cli/internal/gql/app" "numerous/cli/manifest" @@ -67,12 +68,14 @@ func push(cmd *cobra.Command, args []string) { appPath = rt } - toolID, readToolErr := tool.ReadAppID(appDir) - m, readManifestErr := manifest.LoadManifest(filepath.Join(appDir, manifest.ManifestPath)) - if readToolErr != nil || readManifestErr != nil { - fmt.Println("The current directory is not a numerous app", - "\nrun \"numerous init\" to initialize a numerous app in the current directory") + toolID, err := tool.ReadAppIDAndPrintErrors(appDir) + if err != nil { + return + } + m, err := manifest.LoadManifest(filepath.Join(appDir, manifest.ManifestPath)) + if err != nil { + output.PrintErrorAppNotInitialized() return } @@ -83,7 +86,7 @@ func push(cmd *cobra.Command, args []string) { os.Exit(1) } - _, err := app.Query(string(toolID), gql.GetClient()) + _, err = app.Query(string(toolID), gql.GetClient()) if err != nil { if strings.Contains(err.Error(), "record not found") { // TODO: replace strings-check with GraphQL error type, when GraphQL types exist. fmt.Println("Sorry, we can't find that app ID in our database. Please make sure you have the correct app ID entered.", diff --git a/cli/cmd/unpublish/unpublish.go b/cli/cmd/unpublish/unpublish.go index ac84a4e..d96a4d4 100644 --- a/cli/cmd/unpublish/unpublish.go +++ b/cli/cmd/unpublish/unpublish.go @@ -4,6 +4,7 @@ import ( "fmt" "os" + "numerous/cli/cmd/output" "numerous/cli/internal/gql" "numerous/cli/internal/gql/app" "numerous/cli/tool" @@ -24,35 +25,24 @@ var UnpublishCmd = &cobra.Command{ } func unpublish(client *gqlclient.Client) error { - appID, err := tool.ReadAppID(".") - if err == tool.ErrAppIDNotFound { - fmt.Println("The current directory is not a numerous app", - "\nrun \"numerous init\" to initialize a numerous app in the current directory") - - return err - } else if err != nil { - fmt.Println("An error occurred reading the app ID") - fmt.Println("Error: ", err) - + appID, err := tool.ReadAppIDAndPrintErrors(".") + if err != nil { return err } if a, err := app.Query(appID, client); err != nil { - fmt.Println("The app could not be found in the database.") + output.PrintError("The app could not be found in the database.", "") return err } else if a.PublicURL == "" { - fmt.Println("The app is currently not published to the public app gallery!") + output.PrintError("The app is currently not published to the public app gallery.", "") return nil } if _, err := app.Unpublish(appID, client); err != nil { - fmt.Println("An error occurred when unpublishing the app") - fmt.Println("Error: ", err) - + output.PrintErrorDetails("An error occurred when unpublishing the app", err) return err } else { fmt.Println("The app has been removed from the public app gallery!") - return nil } } diff --git a/cli/tool/tool.go b/cli/tool/tool.go index 09bc5d6..e82df2f 100644 --- a/cli/tool/tool.go +++ b/cli/tool/tool.go @@ -5,6 +5,8 @@ import ( "fmt" "os" "path/filepath" + + "numerous/cli/cmd/output" ) const ( @@ -65,6 +67,19 @@ func ReadAppID(basePath string) (string, error) { return string(appID), nil } +func ReadAppIDAndPrintErrors(appDir string) (string, error) { + appID, err := ReadAppID(appDir) + if err == ErrAppIDNotFound { + output.PrintErrorAppNotInitialized() + return "", err + } else if err != nil { + output.PrintErrorDetails("An error occurred reading the app ID", err) + return "", err + } + + return appID, nil +} + func (t Tool) String() string { return fmt.Sprintf(` Tool: