diff --git a/CHANGELOG.md b/CHANGELOG.md index e6d48b5..2cd023a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,43 @@ Releases before `0.2.0` predate this file. Their notes are on the ## [Unreleased] +### Added +- `--wide` keeps every table column even when the table is wider than the + terminal. + +### Fixed +- A mistyped subcommand now fails instead of succeeding. Cobra checks for + unknown commands only on the root command, so `namecom domain regsiter + example.com` printed the group's help and exited **0** — meaning + `namecom domain regsiter foo.com && deploy` ran `deploy`. Every command + group now rejects an unknown subcommand as a usage error (exit 2) and + offers the same "Did you mean this?" suggestion the root command does. + Invoking a group bare still prints its help and exits 0. +- Invocation mistakes now exit **2**, as the documented exit-code table has + always claimed. Flag-value validation returned unclassified errors and + cobra's own required-flag check runs where `SetFlagErrorFunc` cannot see + it, so `--type ZZZ` and a missing `--answer` both exited 1 — the same code + a script uses to detect a server error — while `--badflag` beside them + exited 2. +- Tables no longer overflow the terminal. They rendered at natural width + regardless of it (`domain list` came to 113 columns, `order list` 99, + `dns list` 87), so in an 80-column pane the rounded borders wrapped into + fragments. Trailing columns are now dropped until the table fits, with a + footer naming what was hidden; `--wide` restores them, and piped output is + unaffected. +- Relative dates widen their unit past a quarter, so a domain paid through + 2034 reads `in 8 years` rather than `in 2750 days`. +- `--dry-run` said it printed "the API request that would be sent without + executing it", but only write operations honour it; reads always called the + API. The flag now says so rather than implying an invocation touches + nothing. +- `dns create --type` omitted `CAA` from its list of record types, which the + validator has always accepted. +- Help pages put `Examples:` directly under the usage line instead of below + the flag tables and footer, command groups show `namecom ` + instead of the uninvokable `namecom [flags]`, and non-string flag + defaults print unquoted (`default 300`, not `default "300"`). + ## [0.2.4] - 2026-08-17 ### Changed diff --git a/cmd/auth.go b/cmd/auth.go index eb9ee05..ad9e921 100644 --- a/cmd/auth.go +++ b/cmd/auth.go @@ -48,6 +48,7 @@ var logoutProfile string func init() { authLoginCmd.Flags().StringVar(&loginProfile, "profile", "default", "profile name to save credentials under") authLogoutCmd.Flags().StringVar(&logoutProfile, "profile", "", "profile to remove (defaults to the active profile)") + cmdutil.GroupCmd(authCmd) authCmd.AddCommand(authLoginCmd, authStatusCmd, authLogoutCmd) rootCmd.AddCommand(authCmd) } diff --git a/cmd/cmdutil/args.go b/cmd/cmdutil/args.go index cd94295..7f31c1e 100644 --- a/cmd/cmdutil/args.go +++ b/cmd/cmdutil/args.go @@ -81,3 +81,48 @@ func joinNames(names []string) string { return strings.Join(names[:len(names)-1], ", ") + ", and " + names[len(names)-1] } } + +// GroupCmd wires a command group — a parent that exists only to hold +// subcommands — so a mistyped subcommand fails instead of succeeding quietly. +// +// Cobra only checks for unknown commands on the ROOT command: legacyArgs() +// returns nil for any parent that itself has a parent. So `namecom domian` +// errored with a suggestion, while `namecom domain regsiter example.com` +// printed the group's help and exited 0. In a script that reads as success — +// `namecom domain regsiter foo.com && deploy` ran deploy. +// +// Bare `namecom domain` keeps its old behavior of printing help and exiting 0, +// which is what a user typing a group name to browse it expects. +func GroupCmd(cmd *cobra.Command) *cobra.Command { + // Cobra defaults this to 2, but only on the root command, inside the + // unknown-command path we are replacing here. Left at zero, SuggestionsFor + // matches nothing and every typo loses its "Did you mean" line. + if cmd.SuggestionsMinimumDistance <= 0 { + cmd.SuggestionsMinimumDistance = 2 + } + cmd.Args = func(c *cobra.Command, args []string) error { + if len(args) == 0 { + return nil + } + return NewUsageError(fmt.Errorf("unknown command %q for %q%s", + args[0], c.CommandPath(), suggestionHint(c.SuggestionsFor(args[0])))) + } + cmd.RunE = func(c *cobra.Command, _ []string) error { + return c.Help() + } + return cmd +} + +// suggestionHint renders cobra's near-miss list the way cobra renders it on the +// root command, so a typo reads the same wherever in the tree it happens. +func suggestionHint(suggestions []string) string { + if len(suggestions) == 0 { + return "" + } + var b strings.Builder + b.WriteString("\n\nDid you mean this?\n") + for _, s := range suggestions { + fmt.Fprintf(&b, "\t%s\n", s) + } + return b.String() +} diff --git a/cmd/cmdutil/args_test.go b/cmd/cmdutil/args_test.go index a88448f..b243d6f 100644 --- a/cmd/cmdutil/args_test.go +++ b/cmd/cmdutil/args_test.go @@ -1,6 +1,8 @@ package cmdutil import ( + "bytes" + "errors" "strings" "testing" @@ -161,3 +163,67 @@ func TestExactArgs_HintContainsUseLine(t *testing.T) { t.Errorf("error = %q, missing command path in hint", err.Error()) } } + +// TestGroupCmd guards the unknown-subcommand check on command groups. +// +// Cobra's legacyArgs only rejects unknown commands on the ROOT command — for +// any parent that itself has a parent it returns nil, so cobra fell through to +// "not runnable", printed help, and returned no error. `namecom domain +// regsiter example.com` therefore exited 0, and +// `namecom domain regsiter foo.com && deploy` deployed. +func TestGroupCmd(t *testing.T) { + newGroup := func() *cobra.Command { + root := &cobra.Command{Use: "namecom"} + group := GroupCmd(&cobra.Command{Use: "domain"}) + group.AddCommand(&cobra.Command{Use: "register", Run: func(*cobra.Command, []string) {}}) + group.AddCommand(&cobra.Command{Use: "list", Run: func(*cobra.Command, []string) {}}) + root.AddCommand(group) + return group + } + + t.Run("unknown subcommand is a usage error", func(t *testing.T) { + group := newGroup() + err := group.Args(group, []string{"regsiter", "example.com"}) + if err == nil { + t.Fatal("unknown subcommand accepted; it would print help and exit 0") + } + var u *UsageError + if !errors.As(err, &u) { + t.Errorf("error is not a UsageError, so it would exit 1 not 2: %v", err) + } + if !strings.Contains(err.Error(), `unknown command "regsiter"`) { + t.Errorf("error does not name the typo: %v", err) + } + if !strings.Contains(err.Error(), "register") { + t.Errorf("error carries no suggestion: %v", err) + } + }) + + t.Run("no args is allowed so the group can print help", func(t *testing.T) { + group := newGroup() + if err := group.Args(group, nil); err != nil { + t.Errorf("bare group name rejected: %v", err) + } + }) + + t.Run("bare group prints help rather than erroring", func(t *testing.T) { + group := newGroup() + var buf bytes.Buffer + group.SetOut(&buf) + if err := group.RunE(group, nil); err != nil { + t.Fatalf("group RunE returned an error: %v", err) + } + if !strings.Contains(buf.String(), "register") { + t.Errorf("bare group did not print its subcommands:\n%s", buf.String()) + } + }) + + t.Run("suggestion distance is set", func(t *testing.T) { + // Cobra defaults this to 2 only inside the root-command path being + // replaced here. Left at zero, SuggestionsFor matches nothing. + group := newGroup() + if group.SuggestionsMinimumDistance <= 0 { + t.Error("SuggestionsMinimumDistance not set; every typo loses its hint") + } + }) +} diff --git a/cmd/cmdutil/errors.go b/cmd/cmdutil/errors.go index 75616b2..f8dd2ad 100644 --- a/cmd/cmdutil/errors.go +++ b/cmd/cmdutil/errors.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "net/http" + "strings" "github.com/patramsey/namecom-cli/internal/api" ) @@ -88,3 +89,43 @@ func AsRestricted(err error, operation, program string) error { } return err } + +// cobraUsagePrefixes are the messages cobra produces for invocation mistakes it +// validates itself, after our own hooks have run. +// +// SetFlagErrorFunc in root.go covers flag *parsing*, but cobra checks required +// flags and flag groups later, inside execute(), and offers no hook for that +// path — a missing required flag surfaced as a bare error and exited 1 while +// `--badflag` right next to it exited 2. Matching the message is unpleasant but +// it is the only seam cobra exposes; the strings are stable and asserted by +// TestClassifyCobraUsage, which fails loudly if an upgrade rewords one. +var cobraUsagePrefixes = []string{ + "required flag(s) ", + "if any flags in the group ", + "unknown command ", + "unknown flag: ", + "unknown shorthand flag: ", + "invalid argument ", + "flag needs an argument", +} + +// ClassifyCobraUsage wraps cobra's own invocation errors as UsageError so they +// reach the documented exit code 2. Errors that are already classified, and +// errors from anywhere else, pass through untouched. +func ClassifyCobraUsage(err error) error { + if err == nil { + return nil + } + var usage *UsageError + var auth *AuthError + if errors.As(err, &usage) || errors.As(err, &auth) { + return err + } + msg := err.Error() + for _, p := range cobraUsagePrefixes { + if strings.Contains(msg, p) { + return NewUsageError(err) + } + } + return err +} diff --git a/cmd/cmdutil/errors_test.go b/cmd/cmdutil/errors_test.go index c7a27ba..b1baba1 100644 --- a/cmd/cmdutil/errors_test.go +++ b/cmd/cmdutil/errors_test.go @@ -84,3 +84,58 @@ func TestNewUsageError(t *testing.T) { t.Error("NewUsageError should preserve the original error in the chain") } } + +// TestClassifyCobraUsage guards the message matching that maps cobra's own +// invocation errors onto exit code 2. +// +// Cobra validates required flags and flag groups inside execute(), after +// SetFlagErrorFunc has had its chance, and exposes no hook for that path. So +// `dns create example.com` with two required flags missing exited 1 while +// `dns list example.com --badflag` right beside it exited 2. Matching the +// message is the only seam available; these cases fail if a cobra upgrade +// rewords one, which is the point of asserting them. +func TestClassifyCobraUsage(t *testing.T) { + usage := func(err error) bool { + var u *UsageError + return errors.As(err, &u) + } + + cobraMessages := []string{ + `required flag(s) "answer", "type" not set`, + `if any flags in the group [a b] are set they must all be set; missing [b]`, + `unknown command "regsiter" for "namecom domain"`, + `unknown flag: --badflag`, + `unknown shorthand flag: 'z' in -z`, + `invalid argument "abc" for "--ttl" flag: strconv.ParseInt: parsing "abc": invalid syntax`, + `flag needs an argument: --type`, + } + for _, msg := range cobraMessages { + if got := ClassifyCobraUsage(errors.New(msg)); !usage(got) { + t.Errorf("ClassifyCobraUsage(%q) did not classify as a usage error", msg) + } + } + + t.Run("leaves other errors alone", func(t *testing.T) { + runtime := errors.New("connection refused") + if got := ClassifyCobraUsage(runtime); usage(got) { + t.Errorf("misclassified a runtime error as usage: %v", got) + } + }) + + t.Run("nil stays nil", func(t *testing.T) { + if got := ClassifyCobraUsage(nil); got != nil { + t.Errorf("ClassifyCobraUsage(nil) = %v, want nil", got) + } + }) + + t.Run("does not re-wrap an already-classified error", func(t *testing.T) { + // An AuthError whose text happens to mention an unknown flag must keep + // its exit code 3 rather than being demoted to a usage error. + auth := NewAuthError(errors.New("unknown flag: --token was not usable")) + got := ClassifyCobraUsage(auth) + var a *AuthError + if !errors.As(got, &a) { + t.Errorf("ClassifyCobraUsage demoted an AuthError: %v", got) + } + }) +} diff --git a/cmd/cmdutil/validate.go b/cmd/cmdutil/validate.go index 5d1c6c9..eed6f68 100644 --- a/cmd/cmdutil/validate.go +++ b/cmd/cmdutil/validate.go @@ -8,10 +8,19 @@ import ( "time" ) +// usagef builds a UsageError. Every failure in this file is an invocation +// mistake — a malformed flag value, an out-of-range count, an argument that +// cannot be what it claims — so each one maps to exit code 2 in the documented +// table. They used to return bare fmt.Errorf values, which collapsed to exit 1 +// and made `--type ZZZ` indistinguishable from a 500 to a calling script. +func usagef(format string, a ...any) error { + return NewUsageError(fmt.Errorf(format, a...)) +} + // ValidDate checks that s is a valid YYYY-MM-DD date. func ValidDate(s, flagName string) error { if _, err := time.Parse("2006-01-02", s); err != nil { - return fmt.Errorf("--%s: %q is not a valid date — expected YYYY-MM-DD", flagName, s) + return usagef("--%s: %q is not a valid date — expected YYYY-MM-DD", flagName, s) } return nil } @@ -24,7 +33,7 @@ var validDNSTypes = map[string]bool{ // ValidDNSType checks that t is a supported DNS record type. func ValidDNSType(t string) error { if !validDNSTypes[strings.ToUpper(t)] { - return fmt.Errorf("unknown record type %q — must be one of: A, AAAA, ANAME, CAA, CNAME, MX, NS, SRV, TXT", t) + return usagef("unknown record type %q — must be one of: A, AAAA, ANAME, CAA, CNAME, MX, NS, SRV, TXT", t) } return nil } @@ -32,27 +41,27 @@ func ValidDNSType(t string) error { // ValidDNSHost checks that host is a valid relative DNS label (@ and wildcards allowed). func ValidDNSHost(host string) error { if host == "" { - return fmt.Errorf("--host cannot be empty (use @ for the zone apex)") + return usagef("--host cannot be empty (use @ for the zone apex)") } if host == "@" || host == "*" { return nil } check := strings.TrimPrefix(host, "*.") if strings.ContainsAny(check, " \t") { - return fmt.Errorf("--host %q must not contain spaces", host) + return usagef("--host %q must not contain spaces", host) } if len(check) > 253 { - return fmt.Errorf("--host %q exceeds maximum DNS name length (253 chars)", host) + return usagef("--host %q exceeds maximum DNS name length (253 chars)", host) } for label := range strings.SplitSeq(check, ".") { if label == "" { - return fmt.Errorf("--host %q has an empty label (double dot or leading/trailing dot)", host) + return usagef("--host %q has an empty label (double dot or leading/trailing dot)", host) } if len(label) > 63 { - return fmt.Errorf("--host %q label %q exceeds 63 characters", host, label) + return usagef("--host %q label %q exceeds 63 characters", host, label) } if strings.HasPrefix(label, "-") || strings.HasSuffix(label, "-") { - return fmt.Errorf("--host %q label %q must not start or end with a hyphen", host, label) + return usagef("--host %q label %q must not start or end with a hyphen", host, label) } } return nil @@ -61,50 +70,50 @@ func ValidDNSHost(host string) error { // ValidDNSAnswer validates the answer field for a given record type and host. func ValidDNSAnswer(recordType, host, answer string) error { if answer == "" { - return fmt.Errorf("--answer is required") + return usagef("--answer is required") } switch strings.ToUpper(recordType) { case "A": ip := net.ParseIP(answer) if ip == nil || ip.To4() == nil { - return fmt.Errorf("--answer must be a valid IPv4 address for A records, got %q", answer) + return usagef("--answer must be a valid IPv4 address for A records, got %q", answer) } case "AAAA": ip := net.ParseIP(answer) if ip == nil || ip.To4() != nil { - return fmt.Errorf("--answer must be a valid IPv6 address for AAAA records, got %q", answer) + return usagef("--answer must be a valid IPv6 address for AAAA records, got %q", answer) } case "CNAME": if host == "@" || host == "" { - return fmt.Errorf("CNAME record cannot be set at the zone apex (@) — use ANAME for apex aliasing") + return usagef("CNAME record cannot be set at the zone apex (@) — use ANAME for apex aliasing") } case "MX": if strings.ContainsAny(answer, " \t") { - return fmt.Errorf("MX record --answer must be a hostname (got %q) — set priority with --priority", answer) + return usagef("MX record --answer must be a hostname (got %q) — set priority with --priority", answer) } case "SRV": parts := strings.Fields(answer) if len(parts) != 3 { - return fmt.Errorf("SRV record --answer must be \"weight port target\" (e.g. \"0 443 target.example.com.\"), got %q", answer) + return usagef("SRV record --answer must be \"weight port target\" (e.g. \"0 443 target.example.com.\"), got %q", answer) } if _, err := strconv.Atoi(parts[0]); err != nil { - return fmt.Errorf("SRV record weight (first field) must be an integer, got %q", parts[0]) + return usagef("SRV record weight (first field) must be an integer, got %q", parts[0]) } if _, err := strconv.Atoi(parts[1]); err != nil { - return fmt.Errorf("SRV record port (second field) must be an integer, got %q", parts[1]) + return usagef("SRV record port (second field) must be an integer, got %q", parts[1]) } case "CAA": parts := strings.Fields(answer) if len(parts) < 3 { - return fmt.Errorf("CAA record --answer must be \"flags tag value\" (e.g. `0 issue \"letsencrypt.org\"`), got %q", answer) + return usagef("CAA record --answer must be \"flags tag value\" (e.g. `0 issue \"letsencrypt.org\"`), got %q", answer) } flags, err := strconv.Atoi(parts[0]) if err != nil || flags < 0 || flags > 255 { - return fmt.Errorf("CAA record flags (first field) must be an integer 0-255, got %q", parts[0]) + return usagef("CAA record flags (first field) must be an integer 0-255, got %q", parts[0]) } validTags := map[string]bool{"issue": true, "issuewild": true, "iodef": true} if !validTags[parts[1]] { - return fmt.Errorf("CAA record tag (second field) must be one of: issue, issuewild, iodef — got %q", parts[1]) + return usagef("CAA record tag (second field) must be one of: issue, issuewild, iodef — got %q", parts[1]) } } return nil @@ -152,7 +161,7 @@ func isPrivateIP(ip net.IP) bool { // ValidTTL checks that ttl meets the API minimum. func ValidTTL(ttl int64) error { if ttl < 300 { - return fmt.Errorf("--ttl must be at least 300 seconds (got %d)", ttl) + return usagef("--ttl must be at least 300 seconds (got %d)", ttl) } return nil } @@ -160,13 +169,13 @@ func ValidTTL(ttl int64) error { // ValidDomainName does a basic sanity check on a domain name argument. func ValidDomainName(domain string) error { if strings.Contains(domain, " ") { - return fmt.Errorf("domain name %q must not contain spaces", domain) + return usagef("domain name %q must not contain spaces", domain) } if !strings.Contains(domain, ".") { - return fmt.Errorf("domain name %q must contain at least one dot", domain) + return usagef("domain name %q must contain at least one dot", domain) } if strings.HasPrefix(domain, ".") || strings.HasSuffix(domain, ".") { - return fmt.Errorf("domain name %q must not start or end with a dot", domain) + return usagef("domain name %q must not start or end with a dot", domain) } return nil } @@ -174,20 +183,20 @@ func ValidDomainName(domain string) error { // ValidNameserver checks that ns is a plausible fully-qualified nameserver hostname. func ValidNameserver(ns string, idx int) error { if ns == "" { - return fmt.Errorf("nameserver %d is empty", idx+1) + return usagef("nameserver %d is empty", idx+1) } if !strings.Contains(ns, ".") { - return fmt.Errorf("nameserver %q must be a fully-qualified hostname (e.g. ns1.example.com)", ns) + return usagef("nameserver %q must be a fully-qualified hostname (e.g. ns1.example.com)", ns) } if strings.HasPrefix(ns, ".") || strings.HasSuffix(ns, ".") { - return fmt.Errorf("nameserver %q must not start or end with a dot", ns) + return usagef("nameserver %q must not start or end with a dot", ns) } for label := range strings.SplitSeq(ns, ".") { if label == "" { - return fmt.Errorf("nameserver %q has an empty label", ns) + return usagef("nameserver %q has an empty label", ns) } if strings.HasPrefix(label, "-") || strings.HasSuffix(label, "-") { - return fmt.Errorf("nameserver %q label %q must not start or end with a hyphen", ns, label) + return usagef("nameserver %q label %q must not start or end with a hyphen", ns, label) } } return nil @@ -205,13 +214,13 @@ func ValidSortDir(dir string) error { case "", "asc", "desc": return nil } - return NewUsageError(fmt.Errorf("invalid --sort-dir %q: valid values are asc, desc", dir)) + return usagef("invalid --sort-dir %q: valid values are asc, desc", dir) } // ValidYears checks that n is a valid domain registration/renewal period. func ValidYears(n int) error { if n < 1 || n > 10 { - return fmt.Errorf("--years must be between 1 and 10 (got %d)", n) + return usagef("--years must be between 1 and 10 (got %d)", n) } return nil } @@ -219,11 +228,11 @@ func ValidYears(n int) error { // ValidURL checks that u has an http:// or https:// scheme. func ValidURL(u, flagName string) error { if u == "" { - return fmt.Errorf("--%s is required", flagName) + return usagef("--%s is required", flagName) } lower := strings.ToLower(u) if !strings.HasPrefix(lower, "http://") && !strings.HasPrefix(lower, "https://") { - return fmt.Errorf("--%s %q must start with http:// or https://", flagName, u) + return usagef("--%s %q must start with http:// or https://", flagName, u) } return nil } @@ -231,14 +240,14 @@ func ValidURL(u, flagName string) error { // ValidEmail checks that addr looks like a valid email address. func ValidEmail(addr, flagName string) error { if addr == "" { - return fmt.Errorf("--%s is required", flagName) + return usagef("--%s is required", flagName) } at := strings.LastIndex(addr, "@") if at < 1 || at == len(addr)-1 { - return fmt.Errorf("--%s %q is not a valid email address — expected user@domain.tld", flagName, addr) + return usagef("--%s %q is not a valid email address — expected user@domain.tld", flagName, addr) } if !strings.Contains(addr[at+1:], ".") { - return fmt.Errorf("--%s %q domain part has no dot — expected user@domain.tld", flagName, addr) + return usagef("--%s %q domain part has no dot — expected user@domain.tld", flagName, addr) } return nil } @@ -249,19 +258,19 @@ func ValidURLForwardingType(t, flagName string) error { case "redirect", "302", "masked": return nil } - return fmt.Errorf("--%s %q is not valid — must be one of: redirect, 302, masked", flagName, t) + return usagef("--%s %q is not valid — must be one of: redirect, 302, masked", flagName, t) } // ValidEmailLocalPart checks that s is a valid email mailbox local-part. func ValidEmailLocalPart(s, argName string) error { if s == "" { - return fmt.Errorf("%s must not be empty", argName) + return usagef("%s must not be empty", argName) } if strings.Contains(s, "@") { - return fmt.Errorf("%s %q must not contain '@' — provide only the local part (e.g. 'info', not 'info@example.com')", argName, s) + return usagef("%s %q must not contain '@' — provide only the local part (e.g. 'info', not 'info@example.com')", argName, s) } if strings.ContainsAny(s, " \t\n") { - return fmt.Errorf("%s %q must not contain spaces", argName, s) + return usagef("%s %q must not contain spaces", argName, s) } return nil } @@ -293,10 +302,10 @@ func CanonicalDomain(s string) string { // ValidAuthCode checks that a transfer auth code is plausibly non-trivial. func ValidAuthCode(code string) error { if code == "" { - return fmt.Errorf("--auth-code is required") + return usagef("--auth-code is required") } if len(code) < 6 { - return fmt.Errorf("--auth-code is too short (got %d chars, minimum 6); EPP auth codes are typically 8+ characters", len(code)) + return usagef("--auth-code is too short (got %d chars, minimum 6); EPP auth codes are typically 8+ characters", len(code)) } return nil } diff --git a/cmd/config/config.go b/cmd/config/config.go index a249570..b689d20 100644 --- a/cmd/config/config.go +++ b/cmd/config/config.go @@ -46,6 +46,7 @@ var showCmd = &cobra.Command{ } func init() { + cmdutil.GroupCmd(Cmd) Cmd.AddCommand(listProfilesCmd, useCmd, showCmd) } diff --git a/cmd/contact/contact.go b/cmd/contact/contact.go index d8671e5..68178e9 100644 --- a/cmd/contact/contact.go +++ b/cmd/contact/contact.go @@ -81,6 +81,7 @@ contact click the link.`, func init() { unverifiedCmd.Flags().BoolVar(&listAll, "all", false, "fetch all pages") + cmdutil.GroupCmd(Cmd) Cmd.AddCommand(unverifiedCmd, resendCmd, verifyCmd) } diff --git a/cmd/dns/dns.go b/cmd/dns/dns.go index f1013e2..0554bba 100644 --- a/cmd/dns/dns.go +++ b/cmd/dns/dns.go @@ -131,7 +131,7 @@ func init() { listCmd.Flags().BoolVar(&listAll, "all", false, "fetch all pages automatically") listCmd.Flags().StringVar(&listType, "type", "", "filter by record type (A, AAAA, CNAME, MX, TXT, NS, SRV, ANAME, CAA)") - createCmd.Flags().StringVar(&createType, "type", "", "record type: A, AAAA, ANAME, CNAME, MX, NS, SRV, TXT (required)") + createCmd.Flags().StringVar(&createType, "type", "", "record type: A, AAAA, ANAME, CAA, CNAME, MX, NS, SRV, TXT (required)") createCmd.Flags().StringVar(&createHost, "host", "@", "hostname relative to the zone (@ for apex)") createCmd.Flags().StringVar(&createAnswer, "answer", "", "record value (required)") createCmd.Flags().Int64Var(&createTTL, "ttl", 300, "TTL in seconds (minimum 300)") @@ -151,6 +151,7 @@ func init() { importCmd.Flags().BoolVar(&importDryRun, "dry-run", false, "show what would be created without calling the API") _ = importCmd.MarkFlagRequired("file") + cmdutil.GroupCmd(Cmd) Cmd.AddCommand(listCmd, createCmd, updateCmd, deleteCmd, exportCmd, importCmd) } diff --git a/cmd/dnssec/dnssec.go b/cmd/dnssec/dnssec.go index e72987a..7be3efb 100644 --- a/cmd/dnssec/dnssec.go +++ b/cmd/dnssec/dnssec.go @@ -71,6 +71,7 @@ func init() { _ = createCmd.MarkFlagRequired("digest-type") _ = createCmd.MarkFlagRequired("key-tag") + cmdutil.GroupCmd(Cmd) Cmd.AddCommand(listCmd, getCmd, createCmd, deleteCmd) } diff --git a/cmd/domain/domain.go b/cmd/domain/domain.go index 20a38f8..1f8e593 100644 --- a/cmd/domain/domain.go +++ b/cmd/domain/domain.go @@ -14,6 +14,7 @@ var Cmd = &cobra.Command{ } func init() { + cmdutil.GroupCmd(Cmd) Cmd.AddCommand( listCmd, getCmd, diff --git a/cmd/domain/manage.go b/cmd/domain/manage.go index 0f78c76..e7b9749 100644 --- a/cmd/domain/manage.go +++ b/cmd/domain/manage.go @@ -285,6 +285,7 @@ var contactsFile string func init() { contactsSetCmd.Flags().StringVar(&contactsFile, "from-file", "", "JSON file with contact data (required)") _ = contactsSetCmd.MarkFlagRequired("from-file") + cmdutil.GroupCmd(contactsCmd) contactsCmd.AddCommand(contactsGetCmd, contactsSetCmd) } diff --git a/cmd/email/email.go b/cmd/email/email.go index bd1cc28..22c3173 100644 --- a/cmd/email/email.go +++ b/cmd/email/email.go @@ -80,6 +80,7 @@ func init() { createCmd.Flags().StringVar(&createEmailTo, "to", "", "destination email address (required)") updateCmd.Flags().StringVar(&updateEmailTo, "to", "", "new destination email address") + cmdutil.GroupCmd(Cmd) Cmd.AddCommand(listCmd, getCmd, createCmd, updateCmd, deleteCmd) } diff --git a/cmd/groups_test.go b/cmd/groups_test.go new file mode 100644 index 0000000..8c69ff9 --- /dev/null +++ b/cmd/groups_test.go @@ -0,0 +1,55 @@ +package cmd + +import ( + "errors" + "strings" + "testing" + + "github.com/patramsey/namecom-cli/cmd/cmdutil" + "github.com/spf13/cobra" +) + +// TestEveryGroupRejectsUnknownSubcommands walks the real command tree and +// asserts that every parent group is wired through cmdutil.GroupCmd. +// +// The unit test in cmd/cmdutil covers the helper; this one covers the wiring, +// which is the part that actually regresses. Cobra checks for unknown commands +// only on the root, so a group added later without GroupCmd silently goes back +// to printing help and exiting 0 for a typo — and nothing else would notice. +func TestEveryGroupRejectsUnknownSubcommands(t *testing.T) { + var walk func(*cobra.Command) + walk = func(c *cobra.Command) { + for _, sub := range c.Commands() { + walk(sub) + } + // Only groups: commands that hold subcommands and are not the root. + if !c.HasAvailableSubCommands() || !c.HasParent() { + return + } + // cobra's generated `completion` tree is not ours to police. + if c.Name() == "completion" { + return + } + + t.Run(c.CommandPath(), func(t *testing.T) { + if c.Args == nil { + t.Fatalf("%q has subcommands but no Args validator — a typo'd subcommand exits 0; wrap it in cmdutil.GroupCmd", c.CommandPath()) + } + err := c.Args(c, []string{"nosuchsubcommand"}) + if err == nil { + t.Fatalf("%q accepted an unknown subcommand", c.CommandPath()) + } + var u *cmdutil.UsageError + if !errors.As(err, &u) { + t.Errorf("%q rejects unknown subcommands but not as a UsageError, so it exits 1 instead of 2: %v", c.CommandPath(), err) + } + if !strings.Contains(err.Error(), "nosuchsubcommand") { + t.Errorf("%q error does not name the offending word: %v", c.CommandPath(), err) + } + if err := c.Args(c, nil); err != nil { + t.Errorf("%q rejects being invoked bare, which should print help: %v", c.CommandPath(), err) + } + }) + } + walk(rootCmd) +} diff --git a/cmd/help.go b/cmd/help.go index b49b87d..a01c62a 100644 --- a/cmd/help.go +++ b/cmd/help.go @@ -51,7 +51,19 @@ func printHelp(w io.Writer, cmd *cobra.Command, color bool) { // Usage line fmt.Fprintln(w, style(helpHeading, "Usage:")) - fmt.Fprintf(w, " %s\n\n", style(helpUsage, cmd.UseLine())) + fmt.Fprintf(w, " %s\n\n", style(helpUsage, usageLine(cmd))) + + // Aliases and examples sit directly under the usage line. Examples used to + // print last, below the flag tables and the "see all global options" + // footer, which put the most-read part of a help page furthest down it. + if len(cmd.Aliases) > 0 { + fmt.Fprintf(w, "%s %s\n\n", style(helpHeading, "Aliases:"), strings.Join(cmd.Aliases, ", ")) + } + if cmd.Example != "" { + fmt.Fprintln(w, style(helpHeading, "Examples:")) + fmt.Fprintln(w, cmd.Example) + fmt.Fprintln(w) + } // Subcommands — rendered grouped when groups are defined, flat otherwise. cmds := cmd.Commands() @@ -144,18 +156,6 @@ func printHelp(w io.Writer, cmd *cobra.Command, color bool) { fmt.Fprintln(w) } - // Aliases - if len(cmd.Aliases) > 0 { - fmt.Fprintf(w, "%s %s\n\n", style(helpHeading, "Aliases:"), strings.Join(cmd.Aliases, ", ")) - } - - // Examples - if cmd.Example != "" { - fmt.Fprintln(w, style(helpHeading, "Examples:")) - fmt.Fprintln(w, cmd.Example) - fmt.Fprintln(w) - } - // Footer hint if cmd.HasAvailableSubCommands() { hint := `Use "` + cmd.CommandPath() + ` [command] --help" for more information about a command.` @@ -164,6 +164,21 @@ func printHelp(w io.Writer, cmd *cobra.Command, color bool) { } } +// usageLine renders the usage line, correcting it for command groups. +// +// UseLine() appends "[flags]" whenever a command has flags, so every group +// printed as "namecom domain [flags]" — an invocation that does nothing. A +// group's Use string is a bare name with no argument placeholders; when it also +// has subcommands, what it actually takes is one of them. The root command is +// left alone: "namecom [command] [flags]" is accurate there, since its +// persistent flags are the ones being documented. +func usageLine(cmd *cobra.Command) string { + if cmd.HasAvailableSubCommands() && cmd.HasParent() && !strings.Contains(cmd.Use, " ") { + return cmd.CommandPath() + " " + } + return cmd.UseLine() +} + // essentialGlobalFlagNames returns the subset of global flags shown on subcommand help pages. // Noisy flags (--debug, --timeout, --token, etc.) are omitted; users can run "namecom --help" // to see the full list. @@ -215,9 +230,16 @@ func printFlags(w io.Writer, fs *pflag.FlagSet, _ bool, style func(lipgloss.Styl if len(nameType) > maxLen { maxLen = len(nameType) } + // Quote string defaults, print everything else bare. Quoting + // uniformly rendered numbers as (default "1") and durations as + // (default "30s"), which reads like the flag wants a quoted literal. defVal := "" if f.DefValue != "" && f.DefValue != "false" && f.DefValue != "0" && f.DefValue != "0s" && f.DefValue != "[]" { - defVal = fmt.Sprintf(" (default %q)", f.DefValue) + if f.Value.Type() == "string" { + defVal = fmt.Sprintf(" (default %q)", f.DefValue) + } else { + defVal = fmt.Sprintf(" (default %s)", f.DefValue) + } } entries = append(entries, flagEntry{nameType: nameType, usage: f.Usage, defVal: defVal}) }) diff --git a/cmd/help_test.go b/cmd/help_test.go index 81c9059..54ae8c8 100644 --- a/cmd/help_test.go +++ b/cmd/help_test.go @@ -5,6 +5,7 @@ import ( "regexp" "strings" "testing" + "time" "github.com/charmbracelet/lipgloss" "github.com/spf13/cobra" @@ -193,3 +194,86 @@ func TestPrintFilteredFlags_ShowsOnlyAllowedFlags(t *testing.T) { t.Errorf("a flag outside the allow list must not be rendered, got:\n%s", got) } } + +// TestHelpLayout pins the three help-page fixes. None of them had a test, and +// all three are the kind of thing that silently reverts when someone edits the +// template for an unrelated reason. +func TestHelpLayout(t *testing.T) { + noop := func(*cobra.Command, []string) {} + + t.Run("examples appear above the flag tables", func(t *testing.T) { + // Examples used to print last — below Flags, below Global Flags, and + // below the "see all global options" footer — which put the most-read + // part of a help page furthest down it. + root := &cobra.Command{Use: "namecom"} + cmd := &cobra.Command{ + Use: "create ", + Short: "Create a DNS record", + Example: " namecom dns create example.com --type A --answer 1.2.3.4", + Run: noop, + } + cmd.Flags().String("answer", "", "record value") + root.AddCommand(cmd) + + var buf bytes.Buffer + printHelp(&buf, cmd, false) + got := buf.String() + + examples := strings.Index(got, "Examples:") + flags := strings.Index(got, "Flags:") + if examples < 0 || flags < 0 { + t.Fatalf("help is missing a section:\n%s", got) + } + if examples > flags { + t.Errorf("Examples renders below Flags:\n%s", got) + } + }) + + t.Run("a command group asks for a subcommand, not flags", func(t *testing.T) { + // UseLine() appends "[flags]" to anything with flags, so every group + // advertised `namecom domain [flags]` — an invocation that does nothing. + root := &cobra.Command{Use: "namecom"} + group := &cobra.Command{Use: "domain", Short: "Manage domains"} + group.AddCommand(&cobra.Command{Use: "list", Short: "List domains", Run: noop}) + root.AddCommand(group) + + if got := usageLine(group); got != "namecom domain " { + t.Errorf("usageLine(group) = %q, want %q", got, "namecom domain ") + } + // A leaf command keeps cobra's own usage line, placeholders and all. + leaf := &cobra.Command{Use: "get ", Run: noop} + root.AddCommand(leaf) + if got := usageLine(leaf); !strings.Contains(got, "") { + t.Errorf("usageLine(leaf) = %q, want it to keep its argument placeholder", got) + } + // The root keeps "[command] [flags]": its persistent flags are the + // ones the page is documenting. + if got := usageLine(root); strings.Contains(got, "") { + t.Errorf("usageLine(root) = %q, want cobra's own line", got) + } + }) + + t.Run("only string defaults are quoted", func(t *testing.T) { + // Quoting uniformly rendered (default "300") and (default "30s"), + // which reads like the flag wants a quoted literal. + fs := pflag.NewFlagSet("test", pflag.ContinueOnError) + fs.String("host", "@", "hostname") + fs.Int64("ttl", 300, "TTL in seconds") + fs.Duration("timeout", 30*time.Second, "per-request timeout") + + var buf bytes.Buffer + printFlags(&buf, fs, false, noStyle) + got := buf.String() + + for _, want := range []string{`(default "@")`, `(default 300)`, `(default 30s)`} { + if !strings.Contains(got, want) { + t.Errorf("missing %s in:\n%s", want, got) + } + } + for _, unwanted := range []string{`(default "300")`, `(default "30s")`} { + if strings.Contains(got, unwanted) { + t.Errorf("non-string default rendered as %s:\n%s", unwanted, got) + } + } + }) +} diff --git a/cmd/order/order.go b/cmd/order/order.go index d7ad726..112b1f0 100644 --- a/cmd/order/order.go +++ b/cmd/order/order.go @@ -75,6 +75,7 @@ func init() { _ = refundCmd.MarkFlagRequired("order-id") _ = refundCmd.MarkFlagRequired("item-ids") + cmdutil.GroupCmd(Cmd) Cmd.AddCommand(listCmd, getCmd, refundCmd) } diff --git a/cmd/root.go b/cmd/root.go index 8f3ec36..72ad7e6 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -45,6 +45,7 @@ type globalFlags struct { output string quiet bool noHeader bool + wide bool color string timeout time.Duration debug bool @@ -110,7 +111,7 @@ func Execute() { return cmdutil.NewUsageError(err) }) - if err := rootCmd.Execute(); err != nil { + if err := cmdutil.ClassifyCobraUsage(rootCmd.Execute()); err != nil { cfg := resolvedOut if cfg == nil { cfg = output.DefaultConfig() @@ -189,12 +190,13 @@ func init() { pf.StringVarP(&gf.output, "output", "o", "", "output format: table, json, yaml (default: table in TTY, json otherwise)") pf.BoolVarP(&gf.quiet, "quiet", "q", false, "print IDs/names only (one per line)") pf.BoolVar(&gf.noHeader, "no-header", false, "omit header row from table output") + pf.BoolVar(&gf.wide, "wide", false, "keep every table column even if it overflows the terminal") pf.StringVar(&gf.color, "color", "auto", "colorize output: auto, always, never (env: NO_COLOR, CLICOLOR_FORCE)") pf.DurationVar(&gf.timeout, "timeout", 30*time.Second, "per-request timeout") pf.BoolVar(&gf.debug, "debug", false, "log HTTP requests/responses to stderr (token redacted)") pf.StringVar(&gf.debugFile, "debug-file", "", "log HTTP requests/responses to this file instead of stderr") pf.BoolVarP(&gf.yes, "yes", "y", false, "skip confirmation prompts") - pf.BoolVar(&gf.dryRun, "dry-run", false, "print the API request that would be sent without executing it") + pf.BoolVar(&gf.dryRun, "dry-run", false, "for write operations, print the request instead of sending it (reads are unaffected)") pf.StringVar(&gf.idempKey, "idempotency-key", "", "idempotency key for write operations (auto-generated per invocation if not set)") pf.StringVar(&gf.baseURL, "base-url", "", "override the API base URL (for local stubs and proxies; credentials are sent to whatever you name)") @@ -221,7 +223,7 @@ func persistentPreRunE(cmd *cobra.Command, _ []string) error { return err } -// initOutputContext applies --output, --color, --quiet, and --no-header to the +// initOutputContext applies --output, --color, --quiet, --no-header, and --wide to the // command context. It runs for every command, including those that skip API // credential setup (auth, version, etc.). func initOutputContext(cmd *cobra.Command) error { @@ -244,6 +246,7 @@ func initOutputContext(cmd *cobra.Command) error { } out.QuietMode = gf.quiet out.NoHeader = gf.noHeader + out.Wide = gf.wide cmd.SetContext(context.WithValue(cmd.Context(), cmdutil.KeyOutput, out)) // Remember it for Execute's error path. That path ran before this config // existed and fell back to output.DefaultConfig(), which decides format by diff --git a/cmd/transfer/transfer.go b/cmd/transfer/transfer.go index 42ae225..86e31e1 100644 --- a/cmd/transfer/transfer.go +++ b/cmd/transfer/transfer.go @@ -113,6 +113,7 @@ func init() { listCmd.Flags().BoolVar(&listAll, "all", false, "fetch all pages (full transfer history)") + cmdutil.GroupCmd(Cmd) Cmd.AddCommand(listCmd, getCmd, createCmd, internalCmd, cancelCmd, cancelOutboundCmd, eligibilityCmd) } diff --git a/cmd/url/url.go b/cmd/url/url.go index 386b70a..cf0e938 100644 --- a/cmd/url/url.go +++ b/cmd/url/url.go @@ -97,6 +97,7 @@ func init() { updateCmd.Flags().StringVar(&updateTitle, "title", "", "page title (masked only)") updateCmd.Flags().StringVar(&updateMeta, "meta", "", "meta tags (masked only)") + cmdutil.GroupCmd(Cmd) Cmd.AddCommand(listCmd, getCmd, createCmd, updateCmd, deleteCmd) } diff --git a/cmd/vanity/vanity.go b/cmd/vanity/vanity.go index 0a8c12f..fb6aff5 100644 --- a/cmd/vanity/vanity.go +++ b/cmd/vanity/vanity.go @@ -83,6 +83,7 @@ func init() { updateCmd.Flags().StringVar(&updateIPs, "ips", "", "comma-separated IP addresses (required)") _ = updateCmd.MarkFlagRequired("ips") + cmdutil.GroupCmd(Cmd) Cmd.AddCommand(listCmd, getCmd, createCmd, updateCmd, deleteCmd) } diff --git a/internal/output/output.go b/internal/output/output.go index 265971f..04fca51 100644 --- a/internal/output/output.go +++ b/internal/output/output.go @@ -54,6 +54,11 @@ type Config struct { Writer io.Writer // defaults to os.Stdout EWriter io.Writer // defaults to os.Stderr Sandbox bool // true when targeting the sandbox API (--sandbox / profile) + Wide bool // --wide: never drop table columns, even if they overflow + // MaxWidth is the terminal width tables must fit inside. Zero means + // unconstrained, which is what a pipe or a redirect gets: a consumer that + // is not a terminal has no width to respect and wants every column. + MaxWidth int } // DefaultConfig returns an output config with defaults resolved from the @@ -63,12 +68,16 @@ func DefaultConfig() *Config { if isStdoutTTY() { f = FormatTable } - return &Config{ + c := &Config{ Format: f, Color: ColorAuto, Writer: os.Stdout, EWriter: os.Stderr, } + if w, _, err := term.GetSize(int(os.Stdout.Fd())); err == nil && w > 0 { + c.MaxWidth = w + } + return c } // IsInteractive reports whether stdin is a TTY — i.e., a human is present. @@ -206,8 +215,20 @@ func (c *Config) YAMLList(data any, nextPage *int32, total int32) error { } // Table renders rows as a styled table. headers is the column header row. +// Table renders headers and rows as a bordered table, dropping trailing +// columns that do not fit the terminal. +// +// Tables were rendered at their natural width with no regard for the terminal: +// `domain list` came to 113 columns, `order list` 99, `dns list` 87. In an +// 80-column terminal — an SSH session, a split pane — every one of them wrapped +// and the rounded borders came apart into unreadable fragments. Columns are +// ordered most- to least-important by their callers, so the ones that go are +// the ones from the right, and a footer names them rather than letting them +// vanish. --wide opts out; so does any non-terminal writer, since a pipe has no +// width to fit and its consumer wants the whole row. func (c *Config) Table(headers []string, rows [][]string) { color := c.ColorEnabled() + headers, rows, dropped := c.fitColumns(headers, rows) cell := lipgloss.NewStyle().Padding(0, 1) header := cell.Bold(color) @@ -239,6 +260,73 @@ func (c *Config) Table(headers []string, rows [][]string) { } fmt.Fprintln(c.Writer, t.Render()) + + if len(dropped) > 0 { + fmt.Fprintln(c.Writer, c.Dim(fmt.Sprintf( + "%d column%s hidden (%s) — widen the terminal, pass --wide, or use -o json", + len(dropped), plural("", len(dropped)), strings.Join(dropped, ", ")))) + } +} + +// fitColumns drops trailing columns until the rendered table fits MaxWidth, +// returning the surviving headers and rows plus the names of what went. +// +// The first column is never dropped: a table of nothing but a row count is +// worse than one that overflows, and callers put the identifying column first. +func (c *Config) fitColumns(headers []string, rows [][]string) ([]string, [][]string, []string) { + if c.Wide || c.MaxWidth <= 0 || len(headers) == 0 { + return headers, rows, nil + } + + keep := len(headers) + for keep > 1 && tableWidth(colWidths(headers, rows, keep)) > c.MaxWidth { + keep-- + } + if keep == len(headers) { + return headers, rows, nil + } + + dropped := make([]string, 0, len(headers)-keep) + for _, h := range headers[keep:] { + dropped = append(dropped, strings.ToLower(h)) + } + trimmed := make([][]string, 0, len(rows)) + for _, r := range rows { + if len(r) > keep { + r = r[:keep] + } + trimmed = append(trimmed, r) + } + return headers[:keep], trimmed, dropped +} + +// colWidths measures the first n columns at their natural (widest-cell) width. +// lipgloss.Width is used rather than len because cells arrive pre-styled — +// ExpiryDate returns ANSI escapes — and because a domain may hold wide runes. +func colWidths(headers []string, rows [][]string, n int) []int { + w := make([]int, n) + for i := 0; i < n && i < len(headers); i++ { + w[i] = lipgloss.Width(headers[i]) + } + for _, r := range rows { + for i := 0; i < n && i < len(r); i++ { + if cw := lipgloss.Width(r[i]); cw > w[i] { + w[i] = cw + } + } + } + return w +} + +// tableWidth is the rendered width of a table with the given column widths: +// each column carries one space of padding on each side, and there is a border +// rune before the first column, between every pair, and after the last. +func tableWidth(widths []int) int { + total := len(widths) + 1 + for _, w := range widths { + total += w + 2 + } + return total } // KVTable renders a headerless two-column key-value table with styled field names. @@ -502,19 +590,34 @@ func (c *Config) ExpiryDate(t *time.Time) string { if !c.ColorEnabled() { return label } + return expiryStyle(days).Render(label) +} + +// expiryStyle picks the urgency styling for a date this many days away. +// +// Split out from ExpiryDate so the thresholds can be asserted directly: off a +// TTY lipgloss degrades every style to a no-op, so a test comparing rendered +// output cannot tell red from dim and passes whatever the code does. +func expiryStyle(days float64) lipgloss.Style { switch { - case days < 0: - return lipgloss.NewStyle().Bold(true).Foreground(acRed).Render(label) - case days < 7: - return lipgloss.NewStyle().Bold(true).Foreground(acRed).Render(label) + case days < 7: // expired, or expiring inside a week + return lipgloss.NewStyle().Bold(true).Foreground(acRed) case days < 30: - return lipgloss.NewStyle().Bold(true).Foreground(acAmber).Render(label) + return lipgloss.NewStyle().Bold(true).Foreground(acAmber) default: - return styleDim.Render(label) + return styleDim } } -// relativeTime converts a floating-point day count into a human-readable string. +// relativeTime converts a floating-point day count into a human-readable +// string, widening the unit as the distance grows. +// +// It used to speak only days, which is right near an expiry and useless far +// from one: a domain paid through 2034 rendered as "in 2750 days", a number no +// reader converts to anything meaningful. Days stay exact inside a quarter, +// where renewal decisions actually happen; past that the unit widens. The +// absolute date sits immediately before this string in every caller, so the +// parenthetical only has to convey magnitude. func relativeTime(days float64) string { abs := days if abs < 0 { @@ -523,21 +626,34 @@ func relativeTime(days float64) string { switch { case days < 0 && abs < 1: return "expired today" - case days < 0: - n := int(abs + 0.5) - if n == 1 { - return "1 day ago" - } - return fmt.Sprintf("%d days ago", n) - case days < 1: + case days < 1 && days >= 0: return "today" + } + unit, n := humanizeDays(abs) + if days < 0 { + return fmt.Sprintf("%d %s ago", n, plural(unit, n)) + } + return fmt.Sprintf("in %d %s", n, plural(unit, n)) +} + +// humanizeDays picks the coarsest unit that still says something useful about a +// span, and returns the count in that unit. +func humanizeDays(abs float64) (unit string, n int) { + switch { + case abs <= 90: + return "day", int(abs + 0.5) + case abs < 730: + return "month", int(abs/30.44 + 0.5) default: - n := int(days + 0.5) - if n == 1 { - return "in 1 day" - } - return fmt.Sprintf("in %d days", n) + return "year", int(abs/365.25 + 0.5) + } +} + +func plural(unit string, n int) string { + if n == 1 { + return unit } + return unit + "s" } // spinFrames are the animation frames for the spinner. diff --git a/internal/output/output_test.go b/internal/output/output_test.go index d5b0cc4..e421a03 100644 --- a/internal/output/output_test.go +++ b/internal/output/output_test.go @@ -9,6 +9,7 @@ import ( "testing" "time" + "github.com/charmbracelet/lipgloss" "gopkg.in/yaml.v3" ) @@ -707,3 +708,159 @@ func TestTTYPredicates_ReportNonTTYUnderTest(t *testing.T) { t.Error("IsInteractive() = true under `go test`, where stdin is not a terminal") } } + +// TestRelativeTimeWidensUnit guards the unit-widening thresholds. Day counts +// are exact inside a quarter, where a renewal decision is actually pending; +// past that they widen, because "in 2750 days" told a reader nothing about a +// domain paid through 2034. +func TestRelativeTimeWidensUnit(t *testing.T) { + tests := []struct { + days float64 + want string + }{ + {0.5, "today"}, + {-0.5, "expired today"}, + {1, "in 1 day"}, + {90, "in 90 days"}, // boundary: still exact days + {91, "in 3 months"}, // first step up + {194, "in 6 months"}, // a real expiry from `domain list` + {729, "in 24 months"}, + {730, "in 2 years"}, // boundary: months give way to years + {2750, "in 8 years"}, + {-3, "3 days ago"}, + {-1, "1 day ago"}, + {-758, "2 years ago"}, // a real expired domain from `domain list` + } + for _, tt := range tests { + if got := relativeTime(tt.days); got != tt.want { + t.Errorf("relativeTime(%.1f) = %q, want %q", tt.days, got, tt.want) + } + } +} + +// TestTableFitsTerminalWidth guards the column-dropping behavior. Tables were +// rendered at natural width regardless of the terminal: `domain list` measured +// 113 columns against an 80-column pane, and the rounded borders came apart on +// wrap. Columns now drop from the right, which is the least-important end, +// and the footer names what went so nothing disappears silently. +func TestTableFitsTerminalWidth(t *testing.T) { + headers := []string{"DOMAIN", "EXPIRES", "AUTO-RENEW", "LOCKED", "PRIVACY"} + rows := [][]string{ + {"loadtest-ff7fb52b-b51b-46c8-b254-6a557f053321.com", "2027-03-01 (in 6 months)", "yes", "yes", "no"}, + {"beers.army", "2034-03-01 (in 8 years)", "yes", "yes", "yes"}, + } + + widest := func(s string) int { + widest := 0 + for _, line := range strings.Split(strings.TrimRight(s, "\n"), "\n") { + if w := lipgloss.Width(line); w > widest { + widest = w + } + } + return widest + } + + t.Run("drops columns to fit", func(t *testing.T) { + var buf bytes.Buffer + c := &Config{Format: FormatTable, Writer: &buf, EWriter: &buf, MaxWidth: 80} + c.Table(headers, rows) + got := buf.String() + + // The footer names hidden columns; measure only the table itself. + var tableLines []string + for _, line := range strings.Split(got, "\n") { + if strings.ContainsAny(line, "│╭╰├") { + tableLines = append(tableLines, line) + } + } + if w := widest(strings.Join(tableLines, "\n")); w > 80 { + t.Errorf("table rendered %d columns wide, want <= 80:\n%s", w, got) + } + if !strings.Contains(got, "hidden") { + t.Errorf("dropped columns without telling the reader:\n%s", got) + } + if !strings.Contains(got, "DOMAIN") { + t.Errorf("dropped the identifying column:\n%s", got) + } + }) + + t.Run("--wide keeps every column", func(t *testing.T) { + var buf bytes.Buffer + c := &Config{Format: FormatTable, Writer: &buf, EWriter: &buf, MaxWidth: 80, Wide: true} + c.Table(headers, rows) + got := buf.String() + for _, h := range headers { + if !strings.Contains(got, h) { + t.Errorf("--wide dropped %q:\n%s", h, got) + } + } + if strings.Contains(got, "hidden") { + t.Errorf("--wide should not print a hidden-column footer:\n%s", got) + } + }) + + t.Run("no width constraint keeps every column", func(t *testing.T) { + var buf bytes.Buffer + c := &Config{Format: FormatTable, Writer: &buf, EWriter: &buf} // MaxWidth 0: piped + c.Table(headers, rows) + got := buf.String() + for _, h := range headers { + if !strings.Contains(got, h) { + t.Errorf("piped output dropped %q:\n%s", h, got) + } + } + }) + + t.Run("wide enough drops nothing", func(t *testing.T) { + var buf bytes.Buffer + c := &Config{Format: FormatTable, Writer: &buf, EWriter: &buf, MaxWidth: 200} + c.Table(headers, rows) + if got := buf.String(); strings.Contains(got, "hidden") { + t.Errorf("dropped columns that fit:\n%s", got) + } + }) +} + +// TestExpiryStyleThresholds pins the urgency thresholds. The expired and +// expiring-this-week cases were two byte-identical switch arms; merging them is +// only safe if something asserts both still resolve to red. +// +// It asserts on the style rather than on rendered output because lipgloss +// degrades to no-op styles off a TTY: an earlier version of this test compared +// ANSI prefixes, every expected prefix was the empty string, and all three +// cases passed without checking anything. +func TestExpiryStyleThresholds(t *testing.T) { + tests := []struct { + name string + days float64 + want lipgloss.TerminalColor + }{ + {"long expired", -758, acRed}, + {"expired today", -0.5, acRed}, + {"expiring inside a week", 3, acRed}, + {"the week boundary", 6.9, acRed}, + {"expiring inside a month", 20, acAmber}, + {"the month boundary", 29.9, acAmber}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := expiryStyle(tt.days).GetForeground(); got != tt.want { + t.Errorf("expiryStyle(%.1f) foreground = %v, want %v", tt.days, got, tt.want) + } + }) + } + + t.Run("far future is neither red nor amber", func(t *testing.T) { + fg := expiryStyle(900).GetForeground() + if fg == acRed || fg == acAmber { + t.Errorf("expiryStyle(900) = %v, want the dim style", fg) + } + }) + + t.Run("a far-future date still reads in years", func(t *testing.T) { + at := time.Now().Add(900 * 24 * time.Hour) + if got := noColor().ExpiryDate(&at); !strings.Contains(got, "in 2 years") { + t.Errorf("ExpiryDate(900 days) = %q, want a year-scale relative time", got) + } + }) +}