Skip to content

refactor: delegate global-args + ViewOptions + empty-test helper to @doist/cli-core#67

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

refactor: delegate global-args + ViewOptions + empty-test helper to @doist/cli-core#67
scottlovegrove merged 4 commits into
mainfrom
scottl/cli-core-2

Conversation

@scottlovegrove
Copy link
Copy Markdown
Contributor

Summary

Mirrors twist-cli #209 on top of outline's cli-core 0.9.0 baseline (PR #66). Both CLIs now share one source of truth for global-flag parsing, env-var gating, the ViewOptions shape, and the empty-machine-output test contract.

PR 2 of a 3-PR stack:

  1. refactor: Start integrating @doist/cli-core #66 — cli-core wiring (errors, config, spinner, output, markdown)
  2. This PR — global-args + ViewOptions + empty-output test helper
  3. Next — delegate ol update to @doist/cli-core/commands (mirror of twist-cli #214)

What's wired through cli-core now

Module Was Now
src/lib/global-args.ts n/a new module — createGlobalArgsStore() + createAccessibleGate({ envVar: 'OL_ACCESSIBLE' }) + createSpinnerGate({ envVar: 'OL_SPINNER' }).
src/lib/spinner.ts inline shouldDisableSpinner reading OL_SPINNER/CI/argv imports shouldDisableSpinner from global-args.js.
src/lib/output.ts local OutputOptions = { json?, ndjson?, full? } CoreViewOptions & { full?, raw? }. Adds a thin printEmpty(message, opts) wrapper around cli-core's printEmpty.
src/commands/document.ts ol document list printed nothing on an empty result calls printEmpty('No documents found.', outputOpts) so machine consumers get []\n / nothing for --json / --ndjson and humans get the message.
src/__tests__/empty-output.test.ts n/a new — uses describeEmptyMachineOutput from @doist/cli-core/testing to lock in the ol document list empty contract.

Behaviour worth flagging

  • --accessible flag and OL_ACCESSIBLE=1 env var are now recognised globally (no consumer reads isAccessible yet — wired for parity with twist + todoist; reserves the seam).
  • ol document list against an empty result now prints No documents found. in human mode (was silent). --json / --ndjson behaviour matches the canonical empty contract.
  • Spinner gate now lives in cli-core's createSpinnerGate — adds --progress-jsonl and --verbose as additional disable triggers (forward-compatible; no consumer in outline today).

Test plan

  • npm run type-check
  • npm run lint:check — 0 warnings, 0 errors
  • npm test127 / 127 pass
  • npm run build
  • npm run check:skill-sync
  • Smoke (gates):
    • OL_SPINNER=false ol search foo, ol --no-spinner search foo → spinner off
    • CI=1 ol search foo → spinner off; CI=false ol search foo → spinner ON
  • Smoke (empty list contract, against an empty collection):
    • ol document list --collection <empty>No documents found.
    • ol document list --collection <empty> --json[]
    • ol document list --collection <empty> --ndjson | wc -c0

🤖 Generated with Claude Code

…doist/cli-core

Mirrors twist-cli #209 on top of outline's cli-core 0.9.0 baseline. Both CLIs
now share one source of truth for global-flag parsing, env-var gating, the
`ViewOptions` shape, and the empty-machine-output test contract.

| Module | Was | Now |
| --- | --- | --- |
| `src/lib/global-args.ts` | n/a | new module — `createGlobalArgsStore()` + `createAccessibleGate({ envVar: 'OL_ACCESSIBLE' })` + `createSpinnerGate({ envVar: 'OL_SPINNER' })`. |
| `src/lib/spinner.ts` | inline `shouldDisableSpinner` reading `OL_SPINNER`/`CI`/argv | imports `shouldDisableSpinner` from `global-args.js`. |
| `src/lib/output.ts` | local `OutputOptions = { json?, ndjson?, full? }` | `CoreViewOptions & { full?, raw? }`. Adds `printEmpty` wrapper around cli-core's `printEmpty`. |
| `src/commands/document.ts` | `ol document list` printed nothing on an empty result | calls `printEmpty('No documents found.', outputOpts)` so machine consumers get `[]\n` / nothing for `--json` / `--ndjson` and humans get the message. |
| `src/__tests__/empty-output.test.ts` | n/a | new — uses `describeEmptyMachineOutput` from `@doist/cli-core/testing` to lock in the `ol document list` empty contract. |

## Behaviour worth flagging

- `--accessible` flag and `OL_ACCESSIBLE=1` env var are now recognised
  globally (no consumer reads `isAccessible` yet — wired for parity with
  twist + todoist).
- `ol document list` against an empty result now prints `No documents found.`
  in human mode (was silent). `--json` / `--ndjson` behaviour matches the
  canonical empty contract.

## Test plan

- [x] `npm run type-check`
- [x] `npm run lint:check` — 0 warnings, 0 errors
- [x] `npm test` — **127 / 127** pass
- [x] `npm run build`
- [x] `npm run check:skill-sync`
- [ ] Smoke (gates):
  - `OL_SPINNER=false ol search foo`, `ol --no-spinner search foo` → spinner off
  - `CI=1 ol search foo` → spinner off; `CI=false ol search foo` → spinner ON
  - `OL_ACCESSIBLE=1 ol search foo`, `ol --accessible search foo` → no behaviour change yet (no consumer)
- [ ] Smoke (empty list contract):
  - `ol document list --collection <empty>` → `No documents found.`
  - `ol document list --collection <empty> --json` → `[]`
  - `ol document list --collection <empty> --ndjson | wc -c` → `0`

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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 refactor successfully delegates global arguments, view options, and empty output handling to cli-core, improving alignment with shared CLI conventions. To ensure these updates integrate smoothly, the --accessible flag needs to be registered on the root command, empty list rendering should be centralized in outputList(), the raw flag should remain local to document retrieval, and test coverage should be expanded for the newly delegated spinner disable paths.

Share FeedbackReview Logs

Comment thread src/lib/global-args.ts
Comment thread src/commands/document.ts Outdated
Comment thread src/lib/output.ts
Comment thread src/__tests__/empty-output.test.ts
scottlovegrove and others added 2 commits May 11, 2026 15:40
- index.ts: register `--accessible` as a Commander root option so the gate
  has a corresponding CLI flag (cli-core's `parseGlobalArgs` reads argv
  directly but Commander rejects unknown options).
- output.ts: `outputList` accepts an optional `emptyMessage` and routes
  through `printEmpty` internally — no per-command `if (data.length === 0)`
  duplication.
- output.ts: drop `raw` from shared `OutputOptions`. It's a `document get`
  rendering flag, not a generic view option; read locally in `document.ts`.
- document.ts: list call site uses the new `emptyMessage` param.
- spinner test: cover the new disable triggers from `createSpinnerGate` —
  `--progress-jsonl`, `--progress-jsonl=path`, `--verbose`, `-v`.
- vitest.config: exclude `.claude/worktrees/**` so parallel agent worktrees
  don't pollute the local test run.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment thread src/lib/output.ts Outdated
- output.ts: list commands route through cli-core's `printEmpty` directly
  via `outputList`'s `emptyMessage` param. The local wrapper only re-shaped
  the args and added no value.
- .gitignore + vitest.config.ts: revert the `.claude/worktrees/` ignore
  and exclude rules from 5cfcda7 / 5e7ab2b. Those were workarounds for an
  agent worktree mistakenly created inside the repo; agent worktrees should
  live under `~/worktrees` and never under the project root.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@scottlovegrove scottlovegrove requested review from a team and pauloslund and removed request for a team May 11, 2026 14:55
@scottlovegrove scottlovegrove added the 👀 Show PR PR must be reviewed before or after merging label May 11, 2026
@scottlovegrove scottlovegrove merged commit d1cf0b1 into main May 11, 2026
4 checks passed
@scottlovegrove scottlovegrove deleted the scottl/cli-core-2 branch May 11, 2026 14:55
scottlovegrove added a commit that referenced this pull request May 11, 2026
## Summary

Mirrors [twist-cli #214](Doist/twist-cli#214).
cli-core 0.9.0 owns the registry fetch, package-manager detection,
install spawn, channel switch, and `--json`/`--ndjson` envelopes.
outline provides the package name, version, config path, changelog hint,
and spinner runner.

PR 3 of a 3-PR stack — **stacked on #67**:
1. ✅ #66 — cli-core wiring (errors, config, spinner, output, markdown)
2. ✅ #67 — global-args + ViewOptions + empty-output test helper
3. **This PR** — delegate `ol update` to cli-core/commands

## Changes

- `src/commands/update/index.ts` now forwards to cli-core's
`registerUpdateCommand` with `{ packageName, currentVersion, configPath,
changelogCommandName: 'ol changelog', withSpinner }`.
- Delete `src/commands/update/action.ts` (203 lines) and
`src/commands/update/switch.ts` (37 lines) — cli-core owns both.
- Delete `src/lib/update-config.ts` (12 lines) — cli-core reads/writes
`update_channel` directly via the config path we hand it.
- `src/__tests__/update.test.ts` slims to a wrapper test (option
forwarding). ~400 lines of behaviour tests removed — covered upstream by
cli-core's update suite.
- `src/lib/skills/content.ts` documents the new `--json` flag on `ol
update --check`; `skills/outline-cli/SKILL.md` regenerated.

Net: **37 insertions / 710 deletions across 7 files.**

## Behaviour changes (mirroring twist-cli #214)

- Invalid `update_channel` on disk surfaces as `INVALID_UPDATE_CHANNEL`
from `ol update` (was silently coerced to stable).
- Errors emit canonical cli-core codes (`INVALID_FLAGS`,
`UPDATE_CHECK_FAILED`, `UPDATE_INSTALL_FAILED`).
- Both subcommands accept `--json` / `--ndjson` envelopes.

## Test plan

- [x] `npm run type-check`
- [x] `npm run lint:check` — 0 warnings, 0 errors
- [x] `npm test` — 103 tests pass
- [x] `npm run build && npm run sync:skill && npm run check:skill-sync`
— SKILL.md in sync
- [ ] Smoke `ol update --channel` — reports `stable`
- [ ] Smoke `ol update --check` — reports current vs latest with channel
line
- [ ] Smoke `ol update --check --json` — single-record JSON envelope
- [ ] Manual `ol update switch --pre-release` then `ol update switch
--stable` — flips on-disk `update_channel`

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@doist-release-bot
Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 1.5.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

released 👀 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