Skip to content

Bring comments up to idiomatic Go doc-comment standards #26

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jclusso merged 2 commits into from
Jun 2, 2026
Merged

Bring comments up to idiomatic Go doc-comment standards #26

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e8b8df3
Bring comments up to idiomatic Go doc-comment standards
jclusso Jun 1, 2026
1de09c1
Address review comments
jclusso Jun 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions AGENTS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,3 @@
- **Keep the CLI surface 1:1 with the [API
endpoints](https://emailable.com/docs/api/).** Don't split one endpoint into
multiple subcommands or merge two into one without a reason.
- **Comments are a last resort — write self-documenting code.** Never leave a
comment unless it's absolutely necessary. Name things well and structure code
so it explains itself; reach for a comment only when something is genuinely
confusing and the code truly can't convey it — a surprising trade-off, gotcha,
or spec quirk. Never restate what the code does, repeat a signature, or
narrate another package or command ("login writes here, logout clears there");
those rot when the other code moves.
1 change: 0 additions & 1 deletion cmd/account.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"github.com/spf13/cobra"
)

// newAccountCmd returns the `emailable account` command group.
func newAccountCmd() *cobra.Command {
account := &cobra.Command{
Use: "account",
Expand Down
89 changes: 9 additions & 80 deletions cmd/batch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
"github.com/spf13/cobra"
)

// newBatchCmd returns the `emailable batch` command group.
func newBatchCmd() *cobra.Command {
batch := &cobra.Command{
Use: "batch",
Expand Down Expand Up @@ -144,10 +143,7 @@ func newBatchCmd() *cobra.Command {
}

if wait {
// Surface the id before polling so a ctrl-c mid-wait still
// leaves it on screen for a later `batch get <id>`. Suppressed
// in JSON mode (keeps scripted output a single object) and in
// quiet mode (the id is chrome there).
// Print before polling so ctrl-c mid-wait still leaves the id visible.
if !jsonEff && !cctx.Quiet {
printBatchID(cmd.ErrOrStderr(), submit.ID)
}
Expand All @@ -167,9 +163,6 @@ func newBatchCmd() *cobra.Command {
return renderBatchOutcome(cmd, cctx, final, submit.ID, outPath, showAll)
}

// In JSON mode, pass the submit response (id + message, plus any
// future fields) through unchanged rather than reshaping it to a
// bare {"id":...}; f.Print emits the captured raw body.
return f.Print(submit)
},
}
Expand All @@ -178,7 +171,6 @@ func newBatchCmd() *cobra.Command {
verify.Flags().StringP("output", "o", "", "Write results to FILE (.csv or .json; format inferred from extension)")
verify.Flags().Bool("all", false, "Print the full results table inline instead of a summary")
verify.Flags().Bool("stream", false, "Emit one JSON event per line while polling (implies --wait and --json)")
// Only flags the user explicitly set are forwarded, so server defaults stay in play.
verify.Flags().String("url", "", "URL that will receive the batch results via HTTP POST")
verify.Flags().Bool("retries", true, "Retry verifications when mail servers return certain responses, increasing accuracy")
verify.Flags().StringSlice("response-fields", nil, "Fields to include in the response (default: all)")
Expand All @@ -187,8 +179,6 @@ func newBatchCmd() *cobra.Command {
return batch
}

// submitBatchOptionsFromFlags builds SubmitBatchOptions, leaving fields unset
// for flags the user didn't touch so server defaults apply.
func submitBatchOptionsFromFlags(cmd *cobra.Command) (*api.SubmitBatchOptions, error) {
opts := &api.SubmitBatchOptions{}
any := false
Expand Down Expand Up @@ -222,9 +212,6 @@ func submitBatchOptionsFromFlags(cmd *cobra.Command) (*api.SubmitBatchOptions, e
return opts, nil
}

// printBatchID writes a one-time `Batch ID: <id>` line with a dimmed label.
// Printed before the bar/spinner starts so the id survives in scrollback and a
// ctrl-c mid-wait still leaves the user something to `batch get`.
func printBatchID(w io.Writer, id string) {
stf := output.StylerFor(w)
label := stf(lipgloss.NewStyle().Foreground(lipgloss.Color("241"))).Render("Batch ID:")
Expand Down Expand Up @@ -252,12 +239,10 @@ func (s *batchStreamer) emit(payload map[string]any) error {
return err
}

// emitSubmitted emits the initial `submitted` event for a newly created batch.
func (s *batchStreamer) emitSubmitted(id string) error {
return s.emit(map[string]any{"event": "submitted", "id": id})
}

// emitProgress emits a `progress` event; fired once per poll with a known total.
func (s *batchStreamer) emitProgress(id string, processed, total int) error {
return s.emit(map[string]any{
"event": "progress",
Expand All @@ -267,14 +252,6 @@ func (s *batchStreamer) emitProgress(id string, processed, total int) error {
})
}

// emitComplete emits the terminal `complete` event carrying the final status.
//
// When the status carries the verbatim API body, its fields (emails,
// total_counts, reason_counts, …) are merged through unchanged so per-row
// nulls and unmodeled fields survive — matching the passthrough guarantee of
// the non-streaming --json output. `event` and `id` are stamped on top. When
// no raw body is present (e.g. a hand-built status in a test), it falls back to
// reconstructing the event from the typed fields.
func (s *batchStreamer) emitComplete(id string, status *api.BatchStatus) error {
payload := map[string]any{
"event": "complete",
Expand All @@ -285,8 +262,7 @@ func (s *batchStreamer) emitComplete(id string, status *api.BatchStatus) error {
var fields map[string]json.RawMessage
if err := json.Unmarshal(raw, &fields); err == nil {
for k, v := range fields {
// event/id are CLI-owned envelope keys; never let the API
// body shadow them.
// Never let the API body shadow CLI-owned envelope keys.
if k == "event" || k == "id" {
continue
}
Expand All @@ -311,35 +287,21 @@ func (s *batchStreamer) emitComplete(id string, status *api.BatchStatus) error {
return s.emit(payload)
}

// applyStreamImplications turns on --wait and --json when --stream is set, both
// being preconditions for streaming (nothing to stream without polling; events
// are JSON). Rather than error on a missing flag, we just enable them.
//
// Returns the effective (waitOut, jsonOut) pair; the caller threads jsonOut
// through downstream output-format decisions. No package-level state is touched.
func applyStreamImplications(stream, wait, jsonIn bool) (waitOut, jsonOut bool) {
if !stream {
return wait, jsonIn
}
return true, true
}

// Polling schedule for waitForCompletion. The first fastPollWindow of the
// wait uses fastPollInterval so small batches (which often finish in a few
// seconds) return promptly; after that we back off to slowPollInterval to
// avoid hammering the API on long-running batches.
// Fast-then-slow polling: short interval for the first fastPollWindow, then back off.
const (
fastPollInterval = 1 * time.Second
slowPollInterval = 5 * time.Second
fastPollWindow = 10 * time.Second
)

// waitForCompletion polls the batch status until processing is complete and
// returns the final status. In non-JSON mode it renders a progress bar; when sw
// is non-nil it emits `progress` events instead and suppresses the bar.
//
// Progress output goes to stderr so piping stdout (e.g. `verify --wait >
// results.json`) doesn't mix the bar into the result payload.
// waitForCompletion polls until completion. Progress goes to stderr so piped stdout stays clean.
func waitForCompletion(ctx context.Context, client *api.Client, id string, jsonMode bool, sw *batchStreamer, progressOut io.Writer) (*api.BatchStatus, error) {
if progressOut == nil {
progressOut = os.Stderr
Expand All @@ -352,33 +314,23 @@ func waitForCompletion(ctx context.Context, client *api.Client, id string, jsonM
)
start := time.Now()

// Queued-phase spinner: animates while we're polling with total=0 (server
// hasn't started processing yet). Stops as soon as a bar exists.
queueSpinner := ui.NewTo(progressOut, "Queued")
if uiEnabled {
queueSpinner.Start()
}

for {
// Poll with partial=false intentionally. partial=true returns the
// "completed" shape (emails populated) the moment ANY result is ready,
// so treating it as done catches the batch mid-processing. partial=false
// stays in the "processing" shape until the whole batch finishes, and
// gives reliable processed/total counts for the progress bar.
// partial=false: stays in "processing" shape until the whole batch finishes,
// giving reliable counts. partial=true would signal done as soon as any result
// is ready, catching the batch mid-run.
s, err := client.Batch(ctx, id, false)
if err != nil {
return nil, err
}

if uiEnabled && s.Total > 0 {
if bar == nil || s.Total != lastTotal {
// First known total: kill the queued spinner before drawing
// the bar so the two don't fight for the line.
queueSpinner.Stop()

// Fixed 40-cell bar (auto-capped to terminal width). A
// constrained bar reads as a progress indicator rather than a
// full-width "wall" on short jobs.
bar = ui.NewBar(progressOut, 40)
bar.SetMessage(fmt.Sprintf("Verifying %d emails", s.Total))
bar.Start()
Expand All @@ -387,20 +339,15 @@ func waitForCompletion(ctx context.Context, client *api.Client, id string, jsonM
bar.Set(s.Processed, s.Total)
}

// Stream-write errors are non-fatal; keep polling.
if sw != nil && s.Total > 0 {
_ = sw.emitProgress(id, s.Processed, s.Total)
}

if s.IsComplete() {
// Stop animations BEFORE returning so the final 100% frame
// is rendered and a newline written.
queueSpinner.Stop()

// Counts-match completion (total>0 && processed>=total) can race
// with the API switching to the "completed" payload that carries
// the Emails slice. Retry briefly so callers always get the
// canonical completed shape instead of an empty Emails snapshot.
// Counts-match completion can race with the API switching to the
// "completed" payload; retry briefly to get the canonical shape with Emails.
for i := 0; i < 3 && s.Total > 0 && len(s.Emails) == 0; i++ {
select {
case <-ctx.Done():
Expand All @@ -415,8 +362,6 @@ func waitForCompletion(ctx context.Context, client *api.Client, id string, jsonM
}

if bar != nil {
// The bar self-erases on Stop; the caller's summary line is
// the canonical "done" signal, so no need to retitle first.
bar.Stop()
}
return s, nil
Expand All @@ -434,9 +379,6 @@ func waitForCompletion(ctx context.Context, client *api.Client, id string, jsonM
}
}

// saveToFile writes v to path and prints a success line to stderr in non-JSON
// mode. The success line is suppressed in JSON mode (scripted output) and quiet
// mode.
func saveToFile(cmd *cobra.Command, cctx *cmdCtx, v any, path string) error {
n, err := output.WriteResults(v, output.SaveOptions{
Path: path,
Expand All @@ -446,18 +388,13 @@ func saveToFile(cmd *cobra.Command, cctx *cmdCtx, v any, path string) error {
return err
}
if !cctx.JSONMode {
// Success goes to stderr so stdout (the actual results file path
// or JSON payload) stays scriptable.
h := &output.Human{W: cmd.ErrOrStderr(), Quiet: cctx.Quiet}
msg := savedMessage(n, path)
return h.Success(msg)
}
return nil
}

// savedMessage returns the human "Saved …" line for a successful file
// write. Singular/plural for known row counts; falls back to a count-free
// "Saved to <file>" when the count is unknown (e.g. account JSON dumps).
func savedMessage(n int, path string) string {
switch {
case n <= 0:
Expand All @@ -469,12 +406,6 @@ func savedMessage(n int, path string) string {
}
}

// renderBatchOutcome handles the final stdout/file rendering for a batch
// retrieval, dispatching on --output, --json, download/in-progress state, and
// whether per-email results are present.
//
// The per-email table is opt-in via --all regardless of batch size: UX feedback
// was that the table is rarely useful at a glance and the summary reads better.
func renderBatchOutcome(cmd *cobra.Command, cctx *cmdCtx, status *api.BatchStatus, batchID, outPath string, showAll bool) error {
if outPath != "" {
return saveToFile(cmd, cctx, status, outPath)
Expand All @@ -483,8 +414,6 @@ func renderBatchOutcome(cmd *cobra.Command, cctx *cmdCtx, status *api.BatchStatu
return newOutput(cmd.OutOrStdout(), true).Print(status)
}
if status.DownloadFile != "" || len(status.Emails) == 0 {
// >1000-emails download case, or a still-processing batch with no
// per-email results. Defer to the formatter's dispatch.
return newOutput(cmd.OutOrStdout(), false).Print(status)
}

Expand Down
51 changes: 3 additions & 48 deletions cmd/context.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,6 @@ const apiKeyEnv = "EMAILABLE_API_KEY"

const debugEnv = "EMAILABLE_DEBUG"

// cmdCtx is the shared bag of state every command needs: active environment,
// credentials (stored via the credentials package), config paths (managed by
// the config package), and persistent flags.
//
// Commands should prefer reading JSONMode/Quiet off the context rather than the
// package-level globals, so behavior stays consistent when a command-local
// helper overrides the effective value for its caller.
type cmdCtx struct {
Env *env.Environment
CredentialsPath string
Expand All @@ -44,16 +37,9 @@ type cmdCtx struct {
JSONMode bool
Quiet bool

// refreshNoticeWriter, when non-nil, receives a short stderr message the
// first time requireAuth performs an OAuth refresh during this command's
// lifetime. nil disables the notice (used in JSON mode).
refreshNoticeWriter io.Writer
refreshNoticeWriter io.Writer // non-nil enables a stderr notice on OAuth refresh; nil in JSON mode
}

// newCmdCtxFor builds a cmdCtx and pre-wires the refresh-notice writer to the
// command's stderr. jsonMode is a parameter (rather than read off the global)
// so callers that compute an effective JSON value can pass it without mutating
// the global.
func newCmdCtxFor(cmd *cobra.Command, jsonMode bool) (*cmdCtx, error) {
c, err := newCmdCtx(jsonMode)
if err != nil {
Expand All @@ -62,9 +48,6 @@ func newCmdCtxFor(cmd *cobra.Command, jsonMode bool) (*cmdCtx, error) {
return c.withRefreshNotice(cmd.ErrOrStderr()), nil
}

// newCmdCtx resolves the active environment, locates the credentials and config
// paths, and loads the credentials. It does not enforce that the user is logged
// in — that's requireAuth's job.
func newCmdCtx(jsonMode bool) (*cmdCtx, error) {
e, err := env.Current()
if err != nil {
Expand All @@ -91,10 +74,6 @@ func newCmdCtx(jsonMode bool) (*cmdCtx, error) {
}, nil
}

// apiKeySource labels where the API key came from. There's no flag-source
// variant on purpose: --api-key only exists on `login`, where it triggers a
// save rather than a per-call override, so by the time any other command runs
// the key is either stored or unused.
type apiKeySource string

const (
Expand All @@ -105,8 +84,6 @@ const (
apiKeySourceMissing apiKeySource = "none"
)

// effectiveAPIKey returns the API key the CLI will use next and a label for its
// source. Resolution order: EMAILABLE_API_KEY env, then the stored API key.
func (c *cmdCtx) effectiveAPIKey() (string, apiKeySource) {
if v := os.Getenv(apiKeyEnv); v != "" {
return v, apiKeySourceEnv
Expand All @@ -117,32 +94,17 @@ func (c *cmdCtx) effectiveAPIKey() (string, apiKeySource) {
return "", apiKeySourceNone
}

// debugEnabled reports whether HTTP debug logging is on: --debug or a non-empty
// EMAILABLE_DEBUG.
func debugEnabled() bool {
return debugMode || os.Getenv(debugEnv) != ""
}

// withRefreshNotice configures the context to emit a one-line stderr notice
// the first time it performs an OAuth refresh. Suppressed when JSONMode is
// true so machine-readable output stays clean.
func (c *cmdCtx) withRefreshNotice(w io.Writer) *cmdCtx {
if !c.JSONMode {
c.refreshNoticeWriter = w
}
return c
}

// requireAuth returns an *api.Client configured for the active environment.
// Resolution order:
// 1. EMAILABLE_API_KEY / stored API key — non-interactive auth; no refresh
// path.
// 2. Stored OAuth access token — refreshed transparently when close to
// expiry. A failed refresh caused by a permanently-dead refresh token
// (oauth.ErrInvalidGrant) collapses to errNotAuthenticated so the user
// is prompted to log in again; other failures propagate verbatim.
// 3. errNotAuthenticated — the user must `emailable login` or set an
// API key.
func (c *cmdCtx) requireAuth(ctx context.Context) (*api.Client, error) {
if key, _ := c.effectiveAPIKey(); key != "" {
return api.NewWithOptions(c.Env.APIBaseURL, key, c.clientOptions()), nil
Expand All @@ -161,26 +123,19 @@ func (c *cmdCtx) requireAuth(ctx context.Context) (*api.Client, error) {
return api.NewWithOptions(c.Env.APIBaseURL, c.Credentials.AccessToken, c.clientOptions()), nil
}

// clientOptions returns the api.Options used to build clients.
func (c *cmdCtx) clientOptions() api.Options {
return api.Options{Debug: debugEnabled()}
}

// needsRefresh reports whether the stored access token is expired or close
// enough to expiry that we should refresh before the next request. Returns
// false when ExpiresAt is unset (older credentials without expiry tracking)
// so we don't refresh-loop on tokens that have no known TTL.
func (c *cmdCtx) needsRefresh() bool {
// No ExpiresAt (older credentials without expiry tracking) means no known
// TTL — don't refresh-loop on it.
if c.Credentials.RefreshToken == "" || c.Credentials.ExpiresAt.IsZero() {
return false
}
return time.Now().Add(refreshSkew).After(c.Credentials.ExpiresAt)
}

// refresh exchanges the stored refresh_token for a fresh access_token,
// updates c.Credentials in place, and persists to disk. When
// refreshNoticeWriter is non-nil, prints a short dimmed line so an attentive
// user sees that a refresh happened during their command.
func (c *cmdCtx) refresh(ctx context.Context) error {
oc := oauth.NewClient(c.Env.OAuthBaseURL, c.Env.ClientID, nil)
tok, err := oc.Refresh(ctx, c.Credentials.RefreshToken)
Expand Down
Loading