Skip to content

refactor: delegate ol changelog to @doist/cli-core/commands#69

Merged
scottlovegrove merged 2 commits into
mainfrom
scottl/cli-core-4
May 11, 2026
Merged

refactor: delegate ol changelog to @doist/cli-core/commands#69
scottlovegrove merged 2 commits into
mainfrom
scottl/cli-core-4

Conversation

@scottlovegrove
Copy link
Copy Markdown
Contributor

Summary

  • Changelog commandsrc/commands/changelog.ts is now a thin wrapper over registerChangelogCommand from @doist/cli-core/commands, passing path, repoUrl, version, and bulletMarkers: ['*', '-'] (preserves outline's local support for both bullet styles). Drops ~110 lines of local parsing/formatting now owned by cli-core.
  • Top-level error handlersrc/index.ts now matches instanceof BaseCliError so errors thrown from cli-core helpers (e.g. the delegated changelog command's INVALID_TYPE / FILE_READ_ERROR) route through formatError / formatErrorJson. The local CliError extends the same base, so existing throws still match.
  • Global JSON flag — declared --json and --ndjson at the program level with enablePositionalOptions(), plus an isJsonMode() helper in src/lib/global-args.ts backed by cli-core's parseGlobalArgs argv scan. ol --json <subcommand> now produces a JSON error envelope for any BaseCliError thrown after parsing.
  • formatError / formatErrorJson — widened in src/lib/output.ts to accept either positional (code, message, hints?) or a BaseCliError instance.
  • src/lib/errors.ts — re-exports CoreCliError as BaseCliError so the top-level instanceof check covers both.

Mirrors todoist-cli#319. No @doist/cli-core version bump (already on 0.9.0).

Behavior changes

  • ol changelog -n abc now exits non-zero with Error: INVALID_TYPE / Count must be a positive integer (was a custom chalk.red('Error:') line with Count must be a positive number) — intentional alignment with the cli-core contract.
  • ol --json changelog -n abc now emits {"error":{"code":"INVALID_TYPE","message":"Count must be a positive integer"}}.
  • --json / --ndjson now appear as global options in ol --help (existing per-subcommand --json flags still work the same).

Test plan

  • npm run type-check
  • npm run lint:check + npm run format:check
  • npm test (108 tests pass; changelog suite slimmed from 6 tests to 2 wrapper smoke tests covering path-passthrough and footer URL; output suite gains BaseCliError-instance + formatErrorJson cases)
  • Smoke ol changelog -n 1 — version block + footer link unchanged
  • Smoke ol changelog -n abc — human red error, exit 1
  • Smoke ol --json changelog -n abc — JSON error envelope, exit 1
  • Smoke ol doc list --json — per-subcommand --json still routes to output formatter

🤖 Generated with Claude Code

…te cli-core errors through formatter

- `src/commands/changelog.ts` is now a thin wrapper over `registerChangelogCommand` from `@doist/cli-core/commands`, configured with outline's repo URL, package version, and `bulletMarkers: ['*', '-']`. Local parsing/formatting (~110 lines) deleted — cli-core covers the behavior.
- `src/lib/errors.ts` re-exports `CoreCliError` as `BaseCliError` so the top-level handler matches both outline-thrown and cli-core-thrown errors.
- `src/lib/output.ts` widens `formatError` to accept a `BaseCliError` instance and adds `formatErrorJson` for `{error: {code, message, hints}}` envelopes.
- `src/lib/global-args.ts` adds `isJsonMode()` backed by cli-core's `parseGlobalArgs` argv scan.
- `src/index.ts` declares `--json` / `--ndjson` at the program level (with `enablePositionalOptions()`) and routes `BaseCliError` through `formatError` / `formatErrorJson` based on `isJsonMode()`.
- Local changelog tests slimmed to two wrapper smoke tests; output tests cover the new instance/JSON paths.

Mirrors todoist-cli#319.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@scottlovegrove scottlovegrove self-assigned this May 11, 2026
Copy link
Copy Markdown
Member

@doistbot doistbot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR cleanly delegates the changelog command to @doist/cli-core and introduces helpful top-level error handling for JSON output. These updates significantly streamline the codebase by reusing core utilities and establishing a more consistent error-reporting contract. A few adjustments are noted regarding the impact of positional options on existing global flags, ensuring consistent JSON flag behavior across successful commands, and refining the test suite and agent skill documentation to fully capture the new functionality.

Share FeedbackReview Logs

Comment thread src/index.ts Outdated
Comment thread src/index.ts Outdated
Comment thread src/lib/output.ts Outdated
Comment thread src/index.ts Outdated
Comment thread src/commands/changelog.ts Outdated
Comment thread src/__tests__/changelog.test.ts Outdated
Comment thread src/__tests__/changelog.test.ts Outdated
Comment thread src/__tests__/changelog.test.ts Outdated
Comment thread src/__tests__/output.test.ts
Comment thread src/index.ts Outdated
- Drop the program-level --json/--ndjson + enablePositionalOptions experiment. It regressed `ol <sub> --no-spinner` / `--accessible` (root-only flags became unknown options on subcommands) and the description was a lie (root --json never reached getOutputOptions). isJsonMode() still works via cli-core's argv scan; the catch in src/index.ts is unchanged.
- Simplify resolveErrorParts → toCliError: when given a string, return a real BaseCliError instead of mapping properties by hand.
- Derive changelog repoUrl from package.json's repository.url (strip git+/.git) instead of hard-coding the URL.
- Rewrite changelog.test.ts to mock @doist/cli-core/commands directly and assert the config object (path basename, repoUrl, version, bulletMarkers ['*', '-']). Drops the brittle coupling to cli-core's internal fs reads and uses basename() for Windows-portable assertions.
- Add changelog-integration.test.ts: registers the real changelog command and asserts parseAsync rejects with BaseCliError('INVALID_TYPE') for `-n abc`, then runs that error through formatError + formatErrorJson.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@scottlovegrove scottlovegrove requested review from a team and Bloomca and removed request for a team May 11, 2026 16:36
@scottlovegrove scottlovegrove added the 👀 Show PR PR must be reviewed before or after merging label May 11, 2026
@scottlovegrove scottlovegrove merged commit 6dc01e3 into main May 11, 2026
4 checks passed
@scottlovegrove scottlovegrove deleted the scottl/cli-core-4 branch May 11, 2026 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

👀 Show PR PR must be reviewed before or after merging

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants