Skip to content

feat(output)!: mandatory/essential columns and explicit --fields opt-out - #120

Merged
jpage-godaddy merged 1 commit into
mainfrom
column-dropping-tweaks
Oct 5, 2026
Merged

jpage-godaddy merged 1 commit into
mainfrom
column-dropping-tweaks

Conversation

@jpage-godaddy

@jpage-godaddy jpage-godaddy commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Add TableColumn::essential(bool): a column marked essential is never hidden or truncated for terminal width, even if it alone exceeds the display width (still capped at NO_TRUNCATE_MAX_WIDTH for pathological values). Use it for fields a view is useless without — e.g. a DNS record list needs type/name/data at minimum.
  • An explicit --fields selection (as opposed to a command's default_fields fallback) now disables width-based hiding and truncation entirely for the columns it selects, regardless of their essential flag — the user named exactly what they want to see, so the row simply overflows the terminal if it must. Hiding/truncation remain a convenience reserved for the default view, where cli-engine (not the user) picked the field set.
  • Reworked the column-dropping algorithm in output/human/{columns,body}.rs from a contiguous-prefix model to an index-set model so essential columns can be exempted regardless of their declared priority position.
  • Breaking: render_human_with_view, render_human_with_registry_selected, and the two now-dead registry wrapper functions (render_human_with_registry, render_human_with_registry_for_schema, deleted outright) are now crate-internal (pub(crate)). They had no production callers beyond middleware's single internal use — their only other callers were command-module tests reaching past the engine to render a view against fixture data, which is exactly how essential/fields_explicit ended up breaking those call sites in the first place.
  • Added preview_human_view(data, columns) as the one stable, intentionally minimal public entry point for that test use case: render a TableColumn view against fixture data the way a default view would, no Envelope or CLI run required. It's deliberately decoupled from the internal renderers so future rendering-behavior changes won't need to break it the way this one did. Migrated cli-engine's own tests and the downstream gddy consumer's ~12 call sites to it; moved the handful of tests that genuinely exercise internal registry/custom-renderer dispatch contracts into crate-internal unit tests instead.
  • Split output/human/tests.rs into a tests/ directory module (grouped by concern: footer, alignment, field_selection, width_fitting, nested, value_format, registry) since it grew past the 1000-line module cap.
  • Updated docs/concepts.md to document the essential/explicit-fields behavior and the new preview_human_view testing entry point.

Test plan

  • cargo fmt --all --check
  • cargo clippy --all-targets -- -D warnings (default and --features pkce-auth)
  • cargo test --all-targets (293 passed; 338 with --features pkce-auth)
  • cargo test --doc (default and --features pkce-auth)
  • RUSTDOCFLAGS='-D warnings' cargo doc --no-deps (default and --features pkce-auth)
  • cargo rustdoc --lib -- -W missing-docs (zero missing docs)
  • ./cli-engine/scripts/check-module-size.sh
  • All of the above re-verified after rebasing onto main (rustc 1.99.0, as pinned by rust-toolchain.toml)
  • Manually verified against a real consumer command (gddy dns list) in a sibling worktree: 17-column view with type/name/data essential correctly hides 7 non-essential columns at 80-column width while the three essential ones always survive
  • Migrated and re-tested the gddy consumer's ~12 render_human_with_view call sites against preview_human_view — all 991 of its tests pass

BREAKING CHANGE: render_human_with_view and render_human_with_registry_selected are now crate-internal (no longer exported from the crate root or output); render_human_with_registry and render_human_with_registry_for_schema are removed entirely. Any consumer calling these directly (expected mainly in command-module tests) should switch to the new preview_human_view(data, columns).

🤖 Generated with Claude Code

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Two exported renderer signatures are source-breaking, and the row-selection implementation unnecessarily duplicates retained payload data.

Review effort: Balanced
Findings: 2 Medium severity · 1 Low severity

Open (3)
What changed in this PR

Adds essential human-output columns and honors explicit --fields selections without terminal-width fitting.

Changes:

  • Adds TableColumn::essential and index-based column fitting.
  • Propagates explicit field-selection state through rendering.
  • Splits and expands human-output tests and documentation.
File Description
cli-engine/​tests/​foundation.rs Updates renderer calls.
cli-engine/​tests/​exhaustive_output.rs Updates renderer calls.
cli-engine/​tests/​custom_human_views.rs Updates registry rendering call.
cli-engine/​src/​output/​human/​tests/​width_fitting.rs Tests width and essential-column behavior.
cli-engine/​src/​output/​human/​tests/​value_format.rs Houses value-format tests.
cli-engine/​src/​output/​human/​tests/​nested.rs Houses nested-output tests.
cli-engine/​src/​output/​human/​tests/​mod.rs Declares split test modules.
cli-engine/​src/​output/​human/​tests/​footer.rs Houses footer tests.
cli-engine/​src/​output/​human/​tests/​field_selection.rs Tests field ordering and selection.
cli-engine/​src/​output/​human/​tests/​alignment.rs Houses alignment tests.
cli-engine/​src/​output/​human/​tests.rs Removes the monolithic test module.
cli-engine/​src/​output/​human/​mod.rs Adds essential columns and explicit-selection APIs.
cli-engine/​src/​output/​human/​columns.rs Implements index-based width fitting.
cli-engine/​src/​output/​human/​body.rs Applies essential and explicit-field behavior.
cli-engine/​src/​middleware/​run.rs Passes explicit-selection state to rendering.
cli-engine/​docs/​concepts.md Documents width-fitting semantics.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread cli-engine/src/output/human/body.rs
Comment thread cli-engine/src/output/human/mod.rs
Comment thread cli-engine/docs/concepts.md Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

The rendering logic appears coherent and well tested, but the intentional removal of exported renderer APIs warrants final human review for downstream compatibility.

Review effort: Balanced
Findings: None

Resolved since last review (3)

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The rendering behavior, middleware plumbing, documentation, and focused regression coverage are consistent with the stated design.

Review effort: Balanced
Findings: None

@jpage-godaddy jpage-godaddy changed the title feat(output): mandatory/essential columns and explicit --fields opt-out feat(output)!: mandatory/essential columns and explicit --fields opt-out Oct 1, 2026

@mguerrero3-godaddy mguerrero3-godaddy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I see this has merge conflicts. Let me know If this get stalled and needs reapproval.

…th fitting

Add `TableColumn::essential`: essential columns are never hidden or
truncated to fit the terminal width. An explicit (user-typed) `--fields`
disables all width-based column hiding and truncation; both only apply to
the default view.

Hide the view-rendering internals and add `preview_human_view` as the
stable, test-oriented way to render a view from a command module's tests.

BREAKING CHANGE: `render_human_with_view` and
`render_human_with_registry_selected` are now crate-internal (no longer
exported from the crate root or `output`); `render_human_with_registry`
and `render_human_with_registry_for_schema` are removed entirely. Any
consumer calling these directly (expected mainly in command-module tests)
should switch to `preview_human_view(data, columns)`.

Co-Authored-By: Claude Code <noreply@anthropic.com>
@jpage-godaddy
jpage-godaddy force-pushed the column-dropping-tweaks branch from 995a54e to 03c784e Compare October 5, 2026 15:45
@jpage-godaddy
jpage-godaddy merged commit 4ff2418 into main Oct 5, 2026
4 checks passed
@jpage-godaddy
jpage-godaddy deleted the column-dropping-tweaks branch October 5, 2026 15:49
@github-actions github-actions Bot mentioned this pull request Oct 5, 2026
jpage-godaddy pushed a commit that referenced this pull request Oct 5, 2026
🤖 I have created a release *beep* *boop*
---


<details><summary>cli-engine: 0.10.0</summary>

##
[0.10.0](cli-engine-v0.9.5...cli-engine-v0.10.0)
(2026-10-05)


### ⚠ BREAKING CHANGES

* **output:** `render_human_with_view` and
`render_human_with_registry_selected` are now crate-internal (no longer
exported from the crate root or `output`); `render_human_with_registry`
and `render_human_with_registry_for_schema` are removed entirely. Any
consumer calling these directly (expected mainly in command-module
tests) should switch to the new `preview_human_view(data, columns)`.

### Features

* **output:** mandatory/essential columns and explicit --fields opt-out
([#120](#120))
([4ff2418](4ff2418))


### Bug Fixes

* **ci:** unblock Rust CI on newer clippy/rustdoc lints and pin the
toolchain ([#122](#122))
([6fe18d7](6fe18d7))


### Documentation

* propose cursor-first pagination (--limit/--continue)
([#114](#114))
([d67f9d0](d67f9d0))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants