diff --git a/cmd/matecommit/main.go b/cmd/matecommit/main.go index aac052f..2366bd6 100644 --- a/cmd/matecommit/main.go +++ b/cmd/matecommit/main.go @@ -47,8 +47,15 @@ func main() { // through the slog handler at Info level. With the handler's default // Warn threshold, that silently drops every fatal error unless --debug // is passed — and even then it prints mislabeled as "[INFO]". + // + // Commands are expected to display their own errors via ui.PrintError / + // ui.HandleAppError before returning them (wrapped with ui.Shown) — this + // is the last-resort net for the few paths that don't, so it only prints + // when nothing has shown the error yet. if err := app.Run(context.Background(), os.Args); err != nil { - fmt.Fprintln(os.Stderr, err) + if !ui.IsShown(err) { + fmt.Fprintln(os.Stderr, err) + } os.Exit(1) } } diff --git a/internal/ai/gemini/helper.go b/internal/ai/gemini/helper.go index de9e927..a335584 100644 --- a/internal/ai/gemini/helper.go +++ b/internal/ai/gemini/helper.go @@ -114,6 +114,16 @@ func extractTextFromMap(respMap map[string]interface{}) string { return result.String() } +// unescapeLiteralNewlines converts a literal two-character "\n" escape +// sequence into a real newline character. Despite explicit prompt +// instructions asking the model not to, it sometimes still emits the +// escape sequence as text instead of an actual line break inside +// multi-line Markdown fields (PR bodies, issue descriptions) — this is a +// deterministic safety net for when that instruction isn't followed. +func unescapeLiteralNewlines(s string) string { + return strings.ReplaceAll(s, `\n`, "\n") +} + // CleanLabels cleans and validates labels, keeping only the allowed ones. // It accepts a list of labels to clean and a list of available labels from the repository. // If availableLabels is empty, it falls back to a default list of common labels. diff --git a/internal/ai/gemini/helper_test.go b/internal/ai/gemini/helper_test.go index 4a0a00f..f157a61 100644 --- a/internal/ai/gemini/helper_test.go +++ b/internal/ai/gemini/helper_test.go @@ -33,6 +33,24 @@ func TestExtractUsage(t *testing.T) { }) } +func TestUnescapeLiteralNewlines(t *testing.T) { + t.Run("replaces literal backslash-n with a real newline", func(t *testing.T) { + input := `## Summary\nThis is the body.\n\n## Changes\n- one\n- two` + result := unescapeLiteralNewlines(input) + assert.Equal(t, "## Summary\nThis is the body.\n\n## Changes\n- one\n- two", result) + assert.NotContains(t, result, `\n`) + }) + + t.Run("leaves real newlines untouched", func(t *testing.T) { + input := "## Summary\nAlready a real newline." + assert.Equal(t, input, unescapeLiteralNewlines(input)) + }) + + t.Run("empty string stays empty", func(t *testing.T) { + assert.Equal(t, "", unescapeLiteralNewlines("")) + }) +} + func TestGetGenerateConfig(t *testing.T) { t.Run("default config", func(t *testing.T) { cfg := GetGenerateConfig("gemini-1.5-flash", "", nil) diff --git a/internal/ai/gemini/issue_content_generator.go b/internal/ai/gemini/issue_content_generator.go index fd9d5e8..b6e1efb 100644 --- a/internal/ai/gemini/issue_content_generator.go +++ b/internal/ai/gemini/issue_content_generator.go @@ -325,7 +325,7 @@ func (s *GeminiIssueContentGenerator) parseIssueResponse(content string) (*model result := &models.IssueGenerationResult{ Title: strings.TrimSpace(jsonResult.Title), - Description: strings.TrimSpace(jsonResult.Description), + Description: unescapeLiteralNewlines(strings.TrimSpace(jsonResult.Description)), Labels: jsonResult.Labels, } diff --git a/internal/ai/gemini/pull_requests_summarizer_service.go b/internal/ai/gemini/pull_requests_summarizer_service.go index baf5f4f..0fe7475 100644 --- a/internal/ai/gemini/pull_requests_summarizer_service.go +++ b/internal/ai/gemini/pull_requests_summarizer_service.go @@ -225,7 +225,7 @@ func (gps *GeminiPRSummarizer) GeneratePRSummary(ctx context.Context, prContent return models.PRSummary{ Title: jsonSummary.Title, - Body: jsonSummary.Body, + Body: unescapeLiteralNewlines(jsonSummary.Body), Labels: CleanLabels(jsonSummary.Labels, availableLabels), Usage: usage, }, nil diff --git a/internal/commands/config/doctor.go b/internal/commands/config/doctor.go index ff43188..316cbdb 100644 --- a/internal/commands/config/doctor.go +++ b/internal/commands/config/doctor.go @@ -99,7 +99,7 @@ func (d *DoctorCommand) runHealthCheck(ctx context.Context, t *i18n.Translations } else if len(errors) == 0 { ui.PrintWarning(t.GetMessage("doctor.has_warnings", 0, nil)) } else { - ui.PrintError(os.Stdout, t.GetMessage("doctor.has_errors", 0, nil)) + _ = ui.PrintError(os.Stdout, t.GetMessage("doctor.has_errors", 0, nil)) } fmt.Println() diff --git a/internal/commands/config/init.go b/internal/commands/config/init.go index 35fb25d..5983172 100644 --- a/internal/commands/config/init.go +++ b/internal/commands/config/init.go @@ -55,8 +55,7 @@ func initConfigAction(cfg *config.Config, t *i18n.Translations) cli.ActionFunc { return func(ctx context.Context, command *cli.Command) error { localCfg, useLocal, err := resolveTargetConfig(command, cfg, t) if err != nil { - ui.PrintError(os.Stdout, err.Error()) - return err + return ui.PrintError(os.Stdout, err.Error()) } if useLocal { @@ -454,7 +453,7 @@ func validateGeminiAPIKey(ctx context.Context, apiKey string, t *i18n.Translatio summarizer, err := gemini.NewGeminiCommitSummarizer(testCtx, testCfg, nil) if err != nil { spinner.Error(t.GetMessage("config.api_key_invalid", 0, nil)) - ui.PrintError(os.Stdout, t.GetMessage("config.check_api_key_error", 0, struct{ Error string }{err.Error()})) + _ = ui.PrintError(os.Stdout, t.GetMessage("config.check_api_key_error", 0, struct{ Error string }{err.Error()})) return false } @@ -463,7 +462,7 @@ func validateGeminiAPIKey(ctx context.Context, apiKey string, t *i18n.Translatio // is authorized — CountTokens is the cheapest real call available. if _, err := summarizer.CountTokens(testCtx, "ping"); err != nil { spinner.Error(t.GetMessage("config.api_key_invalid", 0, nil)) - ui.PrintError(os.Stdout, t.GetMessage("config.check_api_key_error", 0, struct{ Error string }{err.Error()})) + _ = ui.PrintError(os.Stdout, t.GetMessage("config.check_api_key_error", 0, struct{ Error string }{err.Error()})) return false } @@ -488,7 +487,7 @@ func validateGitHubToken(ctx context.Context, token string, t *i18n.Translations user, resp, err := client.Users.Get(testCtx, "") if err != nil { spinner.Error(t.GetMessage("config.github_token_invalid", 0, nil)) - ui.PrintError(os.Stdout, t.GetMessage("config.check_token_error", 0, struct{ Error string }{err.Error()})) + _ = ui.PrintError(os.Stdout, t.GetMessage("config.check_token_error", 0, struct{ Error string }{err.Error()})) return false } @@ -573,7 +572,7 @@ func validateJiraConnection(ctx context.Context, baseURL, email, token string, t req, err := http.NewRequestWithContext(testCtx, "GET", testURL, nil) if err != nil { spinner.Error(t.GetMessage("config.jira_connection_failed", 0, nil)) - ui.PrintError(os.Stdout, t.GetMessage("config.jira_error_creating_request", 0, struct{ Error error }{err})) + _ = ui.PrintError(os.Stdout, t.GetMessage("config.jira_error_creating_request", 0, struct{ Error error }{err})) return false } @@ -583,7 +582,7 @@ func validateJiraConnection(ctx context.Context, baseURL, email, token string, t resp, err := client.Do(req) if err != nil { spinner.Error(t.GetMessage("config.jira_connection_failed", 0, nil)) - ui.PrintError(os.Stdout, t.GetMessage("config.check_jira_error", 0, struct{ Error string }{err.Error()})) + _ = ui.PrintError(os.Stdout, t.GetMessage("config.check_jira_error", 0, struct{ Error string }{err.Error()})) return false } defer func() { @@ -603,7 +602,7 @@ func validateJiraConnection(ctx context.Context, baseURL, email, token string, t default: errorMsg = t.GetMessage("config.jira_http_error", 0, struct{ Code int }{resp.StatusCode}) } - ui.PrintError(os.Stdout, errorMsg) + _ = ui.PrintError(os.Stdout, errorMsg) return false } diff --git a/internal/commands/config/set.go b/internal/commands/config/set.go index 435646c..5d4d367 100644 --- a/internal/commands/config/set.go +++ b/internal/commands/config/set.go @@ -32,8 +32,7 @@ func (c *ConfigCommandFactory) newSetCommand(t *i18n.Translations, cfg *config.C }, Action: func(ctx context.Context, command *cli.Command) error { if command.Args().Len() < 2 { - ui.PrintError(os.Stdout, t.GetMessage("config_set_error_args", 0, nil)) - return fmt.Errorf("missing arguments") + return ui.PrintError(os.Stdout, t.GetMessage("config_set_error_args", 0, nil)) } key := strings.ToLower(command.Args().Get(0)) @@ -41,8 +40,7 @@ func (c *ConfigCommandFactory) newSetCommand(t *i18n.Translations, cfg *config.C targetCfg, useLocal, err := resolveTargetConfig(command, cfg, t) if err != nil { - ui.PrintError(os.Stdout, err.Error()) - return err + return ui.PrintError(os.Stdout, err.Error()) } switch key { @@ -50,24 +48,18 @@ func (c *ConfigCommandFactory) newSetCommand(t *i18n.Translations, cfg *config.C if isValidLanguage(value) { targetCfg.Language = value } else { - err := fmt.Errorf("invalid language: %s", value) - ui.PrintError(os.Stdout, err.Error()) - return err + return ui.PrintError(os.Stdout, fmt.Sprintf("invalid language: %s", value)) } case "emoji", "use_emoji": boolVal, parseErr := strconv.ParseBool(value) if parseErr != nil { - err := fmt.Errorf("invalid boolean value: %s", value) - ui.PrintError(os.Stdout, err.Error()) - return err + return ui.PrintError(os.Stdout, fmt.Sprintf("invalid boolean value: %s", value)) } targetCfg.UseEmoji = boolVal case "count", "suggestions_count": intVal, parseErr := strconv.Atoi(value) if parseErr != nil || intVal < 1 || intVal > 10 { - err := fmt.Errorf("invalid count (must be 1-10): %s", value) - ui.PrintError(os.Stdout, err.Error()) - return err + return ui.PrintError(os.Stdout, fmt.Sprintf("invalid count (must be 1-10): %s", value)) } targetCfg.SuggestionsCount = intVal case "active-ai", "active_ai": @@ -79,9 +71,7 @@ func (c *ConfigCommandFactory) newSetCommand(t *i18n.Translations, cfg *config.C } targetCfg.AIConfig.Models[targetCfg.AIConfig.ActiveAI] = config.Model(value) } else { - err := fmt.Errorf("no active AI provider configured") - ui.PrintError(os.Stdout, err.Error()) - return err + return ui.PrintError(os.Stdout, "no active AI provider configured") } case "active-vcs", "active_vcs": targetCfg.ActiveVCSProvider = value @@ -90,9 +80,7 @@ func (c *ConfigCommandFactory) newSetCommand(t *i18n.Translations, cfg *config.C case "git.email", "git-email": targetCfg.GitFallback.UserEmail = value default: - err := fmt.Errorf("unknown configuration key: %s", key) - ui.PrintError(os.Stdout, err.Error()) - return err + return ui.PrintError(os.Stdout, fmt.Sprintf("unknown configuration key: %s", key)) } if useLocal { @@ -102,8 +90,7 @@ func (c *ConfigCommandFactory) newSetCommand(t *i18n.Translations, cfg *config.C } if err != nil { - ui.PrintError(os.Stdout, t.GetMessage("ui_error.error_saving_config", 0, nil)) - return err + return ui.PrintError(os.Stdout, t.GetMessage("ui_error.error_saving_config", 0, nil)) } scope := "global" diff --git a/internal/commands/handler/suggestions.go b/internal/commands/handler/suggestions.go index 3be6ec6..cfac814 100644 --- a/internal/commands/handler/suggestions.go +++ b/internal/commands/handler/suggestions.go @@ -209,8 +209,7 @@ func (h *SuggestionHandler) handleCommitSelection(ctx context.Context, suggestio if _, err := fmt.Scan(&input); err != nil { logger.Error(ctx, "failed to read user selection", err) msg := h.t.GetMessage("commit.error_reading_selection", 0, struct{ Error error }{err}) - ui.PrintError(os.Stdout, msg) - return fmt.Errorf("%s", msg) + return ui.PrintError(os.Stdout, msg) } input = strings.TrimSpace(strings.ToLower(input)) @@ -234,8 +233,7 @@ func (h *SuggestionHandler) handleCommitSelection(ctx context.Context, suggestio if _, err := fmt.Sscanf(input, "%d", &selection); err != nil || selection < 1 || selection > len(suggestions) { log.Warn("invalid selection", "input", input, "max", len(suggestions)) msg := h.t.GetMessage("commit.invalid_selection", 0, struct{ Number int }{len(suggestions)}) - ui.PrintError(os.Stdout, msg) - return fmt.Errorf("%s", msg) + return ui.PrintError(os.Stdout, msg) } log.Info("processing selected commit", "selection", selection) @@ -279,8 +277,7 @@ func (h *SuggestionHandler) processCommit(ctx context.Context, suggestion models editedMessage, err := ui.EditCommitMessage(commitTitle, editorError) if err != nil { logger.Error(ctx, "failed to edit commit message", err) - ui.PrintError(os.Stdout, h.t.GetMessage("ui_preview.error_editing_message", 0, struct{ Error error }{err})) - return err + return ui.PrintError(os.Stdout, h.t.GetMessage("ui_preview.error_editing_message", 0, struct{ Error error }{err})) } if editedMessage == "" { ui.PrintWarning(h.t.GetMessage("ui_preview.commit_cancelled", 0, nil)) diff --git a/internal/commands/issues/from_plan.go b/internal/commands/issues/from_plan.go index 7991222..b970370 100644 --- a/internal/commands/issues/from_plan.go +++ b/internal/commands/issues/from_plan.go @@ -114,8 +114,7 @@ func (f *IssuesCommandFactory) createFromPlanAction(t *i18n.Translations, cfg *c Number int Error string }{1, err.Error()}) - ui.PrintError(os.Stdout, errMsg) - return err + return ui.PrintError(os.Stdout, errMsg) } emoji := "" diff --git a/internal/commands/issues/issues.go b/internal/commands/issues/issues.go index ed61d64..15a69e0 100644 --- a/internal/commands/issues/issues.go +++ b/internal/commands/issues/issues.go @@ -177,15 +177,13 @@ func (f *IssuesCommandFactory) createGenerateAction(t *i18n.Translations, cfg *c if sourcesCount == 0 { log.Error("no input source provided", "duration_ms", time.Since(start).Milliseconds()) - ui.PrintError(os.Stdout, t.GetMessage("issue.error_no_input", 0, nil)) - return fmt.Errorf("%s", t.GetMessage("issue.error_no_input", 0, nil)) + return ui.PrintError(os.Stdout, t.GetMessage("issue.error_no_input", 0, nil)) } if sourcesCount > 1 { log.Error("multiple input sources provided", "duration_ms", time.Since(start).Milliseconds()) - ui.PrintError(os.Stdout, t.GetMessage("issue.error_multiple_sources", 0, nil)) - return fmt.Errorf("%s", t.GetMessage("issue.error_multiple_sources", 0, nil)) + return ui.PrintError(os.Stdout, t.GetMessage("issue.error_multiple_sources", 0, nil)) } ui.PrintSectionBanner(t.GetMessage("issue.banner", 0, nil)) @@ -195,8 +193,7 @@ func (f *IssuesCommandFactory) createGenerateAction(t *i18n.Translations, cfg *c log.Error("failed to create issue service", "error", err, "duration_ms", time.Since(start).Milliseconds()) - ui.PrintError(os.Stdout, fmt.Sprintf("%s: %v", t.GetMessage("issue.error_generating", 0, nil), err)) - return err + return ui.PrintError(os.Stdout, fmt.Sprintf("%s: %v", t.GetMessage("issue.error_generating", 0, nil), err)) } var spinnerMsg string @@ -229,8 +226,7 @@ func (f *IssuesCommandFactory) createGenerateAction(t *i18n.Translations, cfg *c "from_diff", fromDiff, "from_pr", fromPR, "duration_ms", time.Since(start).Milliseconds()) - ui.HandleAppError(err) - return err + return ui.HandleAppError(err) } log.Debug("issue generated", @@ -283,8 +279,7 @@ func (f *IssuesCommandFactory) createGenerateAction(t *i18n.Translations, cfg *c log.Error("failed to create issue", "error", err, "duration_ms", time.Since(start).Milliseconds()) - ui.HandleAppError(err) - return err + return ui.HandleAppError(err) } log.Info("issue created successfully", @@ -394,16 +389,14 @@ func (f *IssuesCommandFactory) createLinkAction(t *i18n.Translations, _ *config. log.Error("invalid PR number", "pr_number", prNumber, "duration_ms", time.Since(start).Milliseconds()) - ui.PrintError(os.Stdout, t.GetMessage("issue.error_invalid_pr", 0, nil)) - return fmt.Errorf("invalid PR number") + return ui.PrintError(os.Stdout, t.GetMessage("issue.error_invalid_pr", 0, nil)) } if issueNumber <= 0 { log.Error("invalid issue number", "issue_number", issueNumber, "duration_ms", time.Since(start).Milliseconds()) - ui.PrintError(os.Stdout, t.GetMessage("issue.error_invalid_issue", 0, nil)) - return fmt.Errorf("invalid issue number") + return ui.PrintError(os.Stdout, t.GetMessage("issue.error_invalid_issue", 0, nil)) } ui.PrintSectionBanner(t.GetMessage("issue.link_banner", 0, nil)) @@ -413,8 +406,7 @@ func (f *IssuesCommandFactory) createLinkAction(t *i18n.Translations, _ *config. log.Error("failed to create issue service", "error", err, "duration_ms", time.Since(start).Milliseconds()) - ui.HandleAppError(err) - return err + return ui.HandleAppError(err) } spinner := ui.NewSmartSpinner(t.GetMessage("issue.linking", 0, struct { @@ -432,8 +424,7 @@ func (f *IssuesCommandFactory) createLinkAction(t *i18n.Translations, _ *config. "pr_number", prNumber, "issue_number", issueNumber, "duration_ms", time.Since(start).Milliseconds()) - ui.PrintError(os.Stdout, fmt.Sprintf("%s: %v", t.GetMessage("issue.error_linking", 0, nil), err)) - return err + return ui.PrintError(os.Stdout, fmt.Sprintf("%s: %v", t.GetMessage("issue.error_linking", 0, nil), err)) } log.Info("issue linked to PR successfully", diff --git a/internal/commands/issues/templates.go b/internal/commands/issues/templates.go index 96049d3..7cc2ecb 100644 --- a/internal/commands/issues/templates.go +++ b/internal/commands/issues/templates.go @@ -37,8 +37,7 @@ func (f *IssuesCommandFactory) newTemplateCommand(t *i18n.Translations, _ *confi ui.PrintInfo(t.GetMessage("issue.template_init_info", 0, nil)) if err := templateService.InitializeTemplates(ctx, force); err != nil { - ui.HandleAppError(err, t) - return err + return ui.HandleAppError(err, t) } templatesDir, _ := templateService.GetTemplatesDir(ctx) @@ -54,8 +53,7 @@ func (f *IssuesCommandFactory) newTemplateCommand(t *i18n.Translations, _ *confi Action: func(ctx context.Context, cmd *cli.Command) error { templates, err := templateService.ListTemplates(ctx) if err != nil { - ui.HandleAppError(err, t) - return err + return ui.HandleAppError(err, t) } if len(templates) == 0 { diff --git a/internal/commands/pull_requests/summarize.go b/internal/commands/pull_requests/summarize.go index d527c0a..fa5ed7c 100644 --- a/internal/commands/pull_requests/summarize.go +++ b/internal/commands/pull_requests/summarize.go @@ -103,8 +103,8 @@ func (c *SummarizeCommand) CreateCommand(t *i18n.Translations, _ *cfg.Config) *c "pr_number", prNumber, "duration_ms", time.Since(start).Milliseconds()) spinner.Error(t.GetMessage("ui.error_generating_pr_summary", 0, nil)) - ui.HandleAppError(err) - return fmt.Errorf(t.GetMessage("error.pr_summary_error", 0, nil)+": %w", err) + _ = ui.HandleAppError(err) + return ui.Shown(fmt.Errorf(t.GetMessage("error.pr_summary_error", 0, nil)+": %w", err)) } log.Info("PR summarized successfully", diff --git a/internal/commands/release/edit.go b/internal/commands/release/edit.go index b3f873d..e9f815b 100644 --- a/internal/commands/release/edit.go +++ b/internal/commands/release/edit.go @@ -60,8 +60,7 @@ func editReleaseAction(releaseSvc releaseService, gitSvc gitService, trans *i18n existingRelease, err := releaseSvc.GetRelease(ctx, version) if err != nil { - ui.HandleAppError(err) - return fmt.Errorf("%s", trans.GetMessage("release.error_fetching_release", 0, struct{ Error string }{err.Error()})) + return ui.HandleAppError(err) } content := existingRelease.Body diff --git a/internal/commands/release/flow.go b/internal/commands/release/flow.go index 278f8f2..29b75da 100644 --- a/internal/commands/release/flow.go +++ b/internal/commands/release/flow.go @@ -27,8 +27,7 @@ func analyzeNextRelease(ctx context.Context, releaseSvc releaseService, trans *i log.Error("failed to analyze next release", "error", err, "duration_ms", time.Since(start).Milliseconds()) - ui.HandleAppError(err) - return nil, fmt.Errorf("%s", trans.GetMessage("release.error_analyzing", 0, struct{ Error string }{err.Error()})) + return nil, ui.HandleAppError(err) } log.Debug("release analyzed", @@ -55,8 +54,7 @@ func generateReleaseNotes(ctx context.Context, releaseSvc releaseService, trans log.Error("failed to generate release notes", "error", err, "duration_ms", time.Since(start).Milliseconds()) - ui.HandleAppError(err) - return nil, fmt.Errorf("%s", trans.GetMessage("release.error_generating_notes", 0, struct{ Error string }{err.Error()})) + return nil, ui.HandleAppError(err) } log.Debug("release notes generated", diff --git a/internal/commands/release/push.go b/internal/commands/release/push.go index 214b00c..0461e2f 100644 --- a/internal/commands/release/push.go +++ b/internal/commands/release/push.go @@ -39,7 +39,7 @@ func pushReleaseAction(releaseSvc releaseService, trans *i18n.Translations) cli. if version == "" { release, err := releaseSvc.AnalyzeNextRelease(ctx) if err != nil { - return fmt.Errorf("%s", trans.GetMessage("release.error_analyzing", 0, struct{ Error string }{err.Error()})) + return ui.HandleAppError(err) } version = release.Version } @@ -48,8 +48,7 @@ func pushReleaseAction(releaseSvc releaseService, trans *i18n.Translations) cli. err := releaseSvc.PushTag(ctx, version) if err != nil { - ui.HandleAppError(err) - return fmt.Errorf("%s", trans.GetMessage("release.error_pushing_tag", 0, struct{ Error string }{err.Error()})) + return ui.HandleAppError(err) } fmt.Println(trans.GetMessage("release.push_success", 0, struct{ Version string }{version})) diff --git a/internal/commands/suggests_commits/suggests_commits.go b/internal/commands/suggests_commits/suggests_commits.go index 67d6806..e3870d1 100644 --- a/internal/commands/suggests_commits/suggests_commits.go +++ b/internal/commands/suggests_commits/suggests_commits.go @@ -132,8 +132,7 @@ func (f *SuggestCommandFactory) createAction(cfg *config.Config, t *i18n.Transla Min int Max int }{1, 10}) - ui.PrintError(os.Stdout, msg) - return fmt.Errorf("%s", msg) + return ui.PrintError(os.Stdout, msg) } cfg.Language = command.String("lang") @@ -149,18 +148,16 @@ func (f *SuggestCommandFactory) createAction(cfg *config.Config, t *i18n.Transla ui.PrintSectionBanner(t.GetMessage("ui.generating_suggestions_banner", 0, nil)) if err := f.gitService.ValidateGitConfig(ctx); err != nil { - ui.HandleAppError(err, t) - return err + return ui.HandleAppError(err, t) } var selectedFiles []string if interactive { changedFiles, err := f.gitService.GetChangedFiles(ctx) if err != nil { - ui.HandleAppError(err, t) - return err + return ui.HandleAppError(err, t) } - + if len(changedFiles) == 0 { ui.PrintWarning("No changed files to select.") return nil @@ -168,8 +165,7 @@ func (f *SuggestCommandFactory) createAction(cfg *config.Config, t *i18n.Transla selectedFiles, err = ui.PromptMultiSelect(t, t.GetMessage("ui.multi_select_default_msg", 0, nil), changedFiles) if err != nil { - ui.HandleAppError(err, t) - return err + return ui.HandleAppError(err, t) } if len(selectedFiles) == 0 { @@ -213,8 +209,8 @@ func (f *SuggestCommandFactory) createAction(cfg *config.Config, t *i18n.Transla "error", err, "duration_ms", duration.Milliseconds()) spinner.Error(t.GetMessage("ui.error_generating_suggestions", 0, nil)) - ui.HandleAppError(err, t) - return fmt.Errorf("%s", t.GetMessage("suggestion_generation_error", 0, struct{ Error error }{err})) + _ = ui.HandleAppError(err, t) + return ui.Shown(fmt.Errorf("%s", t.GetMessage("suggestion_generation_error", 0, struct{ Error error }{err}))) } log.Info("suggestions generated successfully", @@ -243,14 +239,12 @@ func (f *SuggestCommandFactory) handleDryRun(ctx context.Context, t *i18n.Transl fmt.Println() if err := f.gitService.ValidateGitConfig(ctx); err != nil { - ui.HandleAppError(err, t) - return err + return ui.HandleAppError(err, t) } files, err := f.gitService.GetChangedFiles(ctx) if err != nil { - ui.HandleAppError(err, t) - return err + return ui.HandleAppError(err, t) } if len(files) == 0 { @@ -261,8 +255,7 @@ func (f *SuggestCommandFactory) handleDryRun(ctx context.Context, t *i18n.Transl diff, err := f.gitService.GetDiff(ctx) if err != nil { - ui.HandleAppError(err, t) - return err + return ui.HandleAppError(err, t) } _, _ = cyan.Printf(t.GetMessage("stats.dry_run_changed_files", 0, nil)+"\n", len(files)) diff --git a/internal/commands/update/cmd.go b/internal/commands/update/cmd.go index f8165ea..e434a55 100644 --- a/internal/commands/update/cmd.go +++ b/internal/commands/update/cmd.go @@ -30,8 +30,7 @@ func (f *UpdateCommandFactory) CreateCommand(trans *i18n.Translations, _ *config fmt.Println(trans.GetMessage("update.updating", 0, nil)) if err := updater.UpdateCLI(ctx); err != nil { - ui.HandleAppError(err) - return err + return ui.HandleAppError(err) } fmt.Println(trans.GetMessage("update.success", 0, nil)) diff --git a/internal/ui/ui.go b/internal/ui/ui.go index 1fa4d39..47f4e6c 100644 --- a/internal/ui/ui.go +++ b/internal/ui/ui.go @@ -100,7 +100,7 @@ func (s *SmartSpinner) Success(msg string) { func (s *SmartSpinner) Error(msg string) { s.Stop() - PrintError(os.Stdout, msg) + _ = PrintError(os.Stdout, msg) } func (s *SmartSpinner) Warning(msg string) { @@ -118,8 +118,33 @@ func PrintSuccess(w io.Writer, msg string) { _, _ = fmt.Fprintf(w, "%s %s\n", SuccessEmoji, Success.Sprint(msg)) } -func PrintError(w io.Writer, msg string) { +// shownError marks an error as already displayed to the user, so the +// top-level handler in main() can exit with the right code without +// printing the same message a second time. +type shownError struct{ err error } + +func (e *shownError) Error() string { return e.err.Error() } +func (e *shownError) Unwrap() error { return e.err } + +// Shown wraps err to mark it as already displayed to the user. +func Shown(err error) error { + if err == nil { + return nil + } + return &shownError{err: err} +} + +// IsShown reports whether err (or something it wraps) was already +// displayed to the user via PrintError/HandleAppError, so a top-level +// handler knows not to print it again. +func IsShown(err error) bool { + var se *shownError + return errors.As(err, &se) +} + +func PrintError(w io.Writer, msg string) error { _, _ = fmt.Fprintf(w, "%s %s\n", Error.Sprint("❌"), Error.Sprint(msg)) + return Shown(errors.New(msg)) } func PrintWarning(msg string) { @@ -142,11 +167,13 @@ func PrintDuration(msg string, duration time.Duration) { fmt.Printf("%s %s %s\n", SuccessEmoji, Success.Sprint(msg), durationStr) } -// HandleAppError handles an application error and displays it in a friendly way. -// If translations is nil, it will use English defaults. -func HandleAppError(err error, translations ...*i18n.Translations) { +// HandleAppError handles an application error and displays it in a friendly +// way, then returns it wrapped with Shown so callers can propagate it +// (`return ui.HandleAppError(err, t)`) without a top-level handler printing +// it again. If translations is nil, it will use English defaults. +func HandleAppError(err error, translations ...*i18n.Translations) error { if err == nil { - return + return nil } var t *i18n.Translations @@ -185,10 +212,11 @@ func HandleAppError(err error, translations ...*i18n.Translations) { } fmt.Println() - return + return Shown(err) } - PrintError(os.Stdout, err.Error()) + _, _ = fmt.Fprintf(os.Stdout, "%s %s\n", Error.Sprint("❌"), Error.Sprint(err.Error())) + return Shown(err) } func PrintKeyValue(key, value string) { @@ -526,7 +554,7 @@ func PromptMultiSelect(t *i18n.Translations, message string, options []string) ( var idx int _, err := fmt.Sscanf(p, "%d", &idx) if err != nil || idx < 1 || idx > len(options) { - PrintError(os.Stdout, t.GetMessage("ui.multi_select_invalid", 0, struct{ Selection string }{p})) + _ = PrintError(os.Stdout, t.GetMessage("ui.multi_select_invalid", 0, struct{ Selection string }{p})) continue } selected = append(selected, options[idx-1]) diff --git a/internal/ui/ui_test.go b/internal/ui/ui_test.go new file mode 100644 index 0000000..c356053 --- /dev/null +++ b/internal/ui/ui_test.go @@ -0,0 +1,75 @@ +package ui + +import ( + "bytes" + "errors" + "testing" + + "github.com/stretchr/testify/assert" + domainErrors "github.com/thomas-vilte/matecommit/internal/errors" +) + +func TestShownAndIsShown(t *testing.T) { + t.Run("nil error stays nil", func(t *testing.T) { + assert.Nil(t, Shown(nil)) + }) + + t.Run("wrapped error reports IsShown true", func(t *testing.T) { + err := Shown(errors.New("boom")) + assert.True(t, IsShown(err)) + assert.Equal(t, "boom", err.Error()) + }) + + t.Run("a plain, never-shown error reports IsShown false", func(t *testing.T) { + assert.False(t, IsShown(errors.New("boom"))) + }) + + t.Run("IsShown sees through additional wrapping (errors.As walks Unwrap)", func(t *testing.T) { + err := wrapForTest(Shown(errors.New("boom"))) + assert.True(t, IsShown(err)) + }) +} + +func TestPrintError_ReturnsShownError(t *testing.T) { + var buf bytes.Buffer + + err := PrintError(&buf, "something broke") + + assert.True(t, IsShown(err), "the error PrintError returns must be marked as already displayed") + assert.Equal(t, "something broke", err.Error()) + assert.Contains(t, buf.String(), "something broke") +} + +func TestHandleAppError_ReturnsShownError(t *testing.T) { + t.Run("nil error returns nil", func(t *testing.T) { + assert.Nil(t, HandleAppError(nil)) + }) + + t.Run("AppError is marked shown and preserves the original error for errors.As", func(t *testing.T) { + original := domainErrors.NewAppError(domainErrors.TypeGit, "something git-related broke", nil) + + result := HandleAppError(original) + + assert.True(t, IsShown(result), "must be marked as already displayed so main() doesn't print it again") + + var appErr *domainErrors.AppError + assert.True(t, errors.As(result, &appErr), "the original AppError must still be reachable via errors.As") + assert.Equal(t, "something git-related broke", appErr.Message) + }) + + t.Run("plain (non-AppError) error is marked shown too", func(t *testing.T) { + result := HandleAppError(errors.New("generic failure")) + + assert.True(t, IsShown(result)) + assert.Equal(t, "generic failure", result.Error()) + }) +} + +func wrapForTest(err error) error { + return &wrappedForTest{err: err} +} + +type wrappedForTest struct{ err error } + +func (w *wrappedForTest) Error() string { return "wrapped: " + w.err.Error() } +func (w *wrappedForTest) Unwrap() error { return w.err }