Skip to content

refactor: delegate markdown + changelog to @doist/cli-core#210

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

refactor: delegate markdown + changelog to @doist/cli-core#210
scottlovegrove merged 2 commits into
mainfrom
scottl/cli-core-2

Conversation

@scottlovegrove
Copy link
Copy Markdown
Collaborator

Summary

Ports todoist-cli#319 to twist-cli — delegates markdown rendering and the changelog command to @doist/cli-core@0.9.0 (already pinned), and widens the top-level error handler so cli-core-thrown errors route through twist's formatter.

  • Markdown renderingsrc/lib/markdown.ts now delegates preloadMarkdown / renderMarkdown to @doist/cli-core/markdown, keeping the local twist-mention://@mention preprocessor on top. Drops the local marked-terminal-renderer ambient .d.ts shim now that cli-core owns the typing.
  • Bootstrap preloadsrc/index.ts kicks off preloadMarkdown() in parallel with the dynamic command-module load when the output mode renders markdown (skipped for --json/--ndjson/--raw and non-rendering commands like changelog/update/completion/doctor/auth/config/skill). The import cost overlaps with module load — zero added latency.
  • Changelog commandsrc/commands/changelog.ts is now a thin wrapper over registerChangelogCommand from @doist/cli-core/commands, passing twist-specific options (headingLevel: 'flexible', continuationIndent: true, filterEmptyVersions: true) so rendered output is unchanged. Local behavior tests deleted — cli-core covers them.
  • Top-level error handlersrc/index.ts now matches instanceof BaseCliError (cli-core class) so errors thrown from cli-core commands route through the existing formatError path. Local CliError extends base, so subclass throws still match. formatError / formatErrorJson parameter types widened to BaseCliError<string>.

Net diff: −301 / +114 lines.

Test plan

  • npm run lint:check (oxlint + oxfmt)
  • npm run type-check (tsc --noEmit)
  • npm test — 583 tests pass
  • npm run build
  • Smoke tw changelog -n 1 — output unchanged (heading green-bold, dimmed bullets, footer link with current version)
  • Smoke tw changelog -n abc — cli-core throws CliError('INVALID_TYPE'); twist's widened handler renders Error: INVALID_TYPE\nCount must be a positive integer, exit 1
  • Smoke tw thread view <id> — markdown body renders with ANSI styling (not exercised in CI)
  • Smoke tw msg view <id> --json — markdown rendering skipped, output unchanged

🤖 Generated with Claude Code

@scottlovegrove scottlovegrove marked this pull request as ready for review May 9, 2026 20:53
@doistbot doistbot requested a review from craigcarlyle May 9, 2026 20:53
@scottlovegrove scottlovegrove self-assigned this May 9, 2026
@scottlovegrove scottlovegrove added the 👀 Show PR PR must be reviewed before or after merging label May 9, 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 is a great refactor that successfully streamlines the CLI by delegating markdown rendering and the changelog command to @doist/cli-core. Relying on shared core utilities will significantly reduce maintenance overhead and improve long-term consistency. A few edge cases require attention, including safely handling parallel markdown preloading to prevent unhandled rejections, preventing unsupported core flags from leaking into the global arguments shape, improving the robustness of lazy markdown initialization and NDJSON parsing, and refining tests to properly cover Twist-specific changelog options and preload bypass behavior.

Share FeedbackReview Logs

Comment thread src/lib/markdown.ts Outdated
Comment thread src/index.ts Outdated
Comment thread src/lib/global-args.ts
Comment thread src/lib/markdown.test.ts Outdated
Comment thread src/commands/changelog.test.ts
Comment thread src/index.ts Outdated
scottlovegrove added a commit that referenced this pull request May 9, 2026
- src/lib/markdown.ts: renderMarkdown now self-inits via preloadMarkdown()
  rather than silently returning raw content when not preloaded. The
  bootstrap preload in src/index.ts becomes a pure perf optimization
  (parallelize with module load); correctness no longer depends on it.
- src/lib/global-args.ts: narrow exported TwGlobalArgs to drop quiet/
  verbose, which twist does not register with Commander. The internal
  store keeps the full cli-core shape so the shared gate helpers
  (createSpinnerGate, createAccessibleGate) still satisfy
  `T extends GlobalArgs`. Add isNdjsonMode() helper matching isJsonMode().
- src/index.ts: use isNdjsonMode() instead of an argv string-search;
  await loader() and preloadMarkdown() via Promise.all so a preload
  rejection co-rejects with the loader rather than leaking as an
  unhandledRejection.
- src/commands/changelog.test.ts: extend the wrapper-test fixture with
  flexible heading levels, a deps-only release, and a wrapped-bullet
  continuation line. Add assertions that lock down the three twist-
  specific options (headingLevel: 'flexible', filterEmptyVersions: true,
  continuationIndent: true) — regressions in any one would now fail the
  suite rather than slip through.
- src/lib/markdown.test.ts: drop the beforeAll(preloadMarkdown) hook now
  that renderMarkdown self-inits.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
scottlovegrove and others added 2 commits May 9, 2026 22:07
- src/lib/markdown.ts now delegates preloadMarkdown / renderMarkdown to
  @doist/cli-core/markdown, keeping the local twist-mention:// → @mention
  preprocessor on top.
- src/commands/changelog.ts is a thin wrapper over registerChangelogCommand
  from @doist/cli-core/commands, passing path, repoUrl, version, plus
  twist-specific options (headingLevel: 'flexible', continuationIndent,
  filterEmptyVersions) so the rendered output is unchanged.
- src/index.ts kicks off preloadMarkdown() in parallel with the dynamic
  command-module load when the output mode renders markdown, so the import
  cost overlaps with module load and adds no perceptible latency. Skipped
  for json/ndjson/raw and non-rendering commands (changelog/update/etc.).
- src/index.ts top-level error handler now matches BaseCliError so errors
  thrown from cli-core helpers (e.g. CliError('INVALID_TYPE') from the
  shared changelog command) route through the existing formatError path.
  src/lib/errors.ts re-exports BaseCliError; src/lib/output.ts widens the
  formatError / formatErrorJson parameter types accordingly.
- Drops the local marked-terminal-renderer ambient .d.ts shim now that
  cli-core owns the typing surface.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- src/lib/markdown.ts: renderMarkdown now self-inits via preloadMarkdown()
  rather than silently returning raw content when not preloaded. The
  bootstrap preload in src/index.ts becomes a pure perf optimization
  (parallelize with module load); correctness no longer depends on it.
- src/lib/global-args.ts: narrow exported TwGlobalArgs to drop quiet/
  verbose, which twist does not register with Commander. The internal
  store keeps the full cli-core shape so the shared gate helpers
  (createSpinnerGate, createAccessibleGate) still satisfy
  `T extends GlobalArgs`. Add isNdjsonMode() helper matching isJsonMode().
- src/index.ts: use isNdjsonMode() instead of an argv string-search;
  await loader() and preloadMarkdown() via Promise.all so a preload
  rejection co-rejects with the loader rather than leaking as an
  unhandledRejection.
- src/commands/changelog.test.ts: extend the wrapper-test fixture with
  flexible heading levels, a deps-only release, and a wrapped-bullet
  continuation line. Add assertions that lock down the three twist-
  specific options (headingLevel: 'flexible', filterEmptyVersions: true,
  continuationIndent: true) — regressions in any one would now fail the
  suite rather than slip through.
- src/lib/markdown.test.ts: drop the beforeAll(preloadMarkdown) hook now
  that renderMarkdown self-inits.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@scottlovegrove scottlovegrove merged commit c885708 into main May 9, 2026
4 checks passed
@scottlovegrove scottlovegrove deleted the scottl/cli-core-2 branch May 9, 2026 21:09
@craigcarlyle
Copy link
Copy Markdown

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.

3 participants