CAMEL-23237: camel-jbang catalog tables fill the terminal width

[ammachado]

Description

The camel catalog commands (component, dataformat, language, transformer, kamelet, ...) previously truncated the DESCRIPTION column at a fixed 80 characters regardless of terminal size, and each command hand-rolled its column definitions with inconsistent widths (NAME was 60 for transformer and 30 elsewhere).

This PR sizes the last free-text column to fill the detected terminal width and standardizes the shared column definitions.

Changes:

• New common/CamelTableColumns.java toolkit: name(), since(), lastText(), measure(), lastColumnWidth(). measure() computes a column's actual rendered width (header vs. longest cell, capped); lastColumnWidth() gives the remaining terminal width to the last column and truncates it with an ellipsis so each row stays on one line.
• TerminalWidthHelper:
  • Added fillWidth() (no upper cap, unlike flexWidth()).
  • Terminal width is now also detected on Windows (cmd/PowerShell) via mode con, in addition to the existing COLUMNS / stty size detection. Column parsing is positional (second integer) so it survives localized Windows output where the Columns: label is translated.
  • DEFAULT_WIDTH stays at 120 (the fallback when no terminal can be detected, e.g. piped/redirected output).
• Migrated CatalogBaseCommand, CatalogTransformer (dropped its nameWidth() override), CatalogKamelet, UpdateList, and InfraBaseCommand (camel infra list) onto the toolkit.
• Documented in the 4.22 upgrade guide. For full, untruncated output suitable for scripting, use --json.

Follow-up fix included in this PR (found during self-review): noBorderOverhead(n) returned (n-1)*2, but AsciiTable NO_BORDERS pads every column with one leading and one trailing space, so the real overhead is n*2. This was harmless while every table used capped flexWidth, but the new uncapped fill sizes the last column to the exact remainder, so the 2-column underestimate made the widest rows render at terminalWidth + 2 and soft-wrap. Fixed at the root (safe for all existing flexWidth callers, which clamp to [min, max]) and covered by a new rendering-level regression test.

Review updates:

• An earlier revision lowered DEFAULT_WIDTH from 120 to 80; that shared-default change is reverted (80 is too narrow for modern terminals, and it narrowed the no-TTY fallback for every not-yet-migrated NO_BORDERS table). UpdateList is now migrated onto the toolkit rather than left as follow-up, which is the structural fix for the previous UpdateListTest CI failure: its DESCRIPTION column is sized from measured column widths instead of the shared fallback constant.
• camel infra list (InfraBaseCommand) is now migrated too: DESCRIPTION fills the terminal, and IMPLEMENTATION / SERVICE_DATA truncate with an ellipsis instead of letting the raw JSON service data overflow the terminal. The --json service-data handling was extracted into a testable parseServiceData() helper (embedded as nested JSON, not an escaped string; unparseable data is omitted rather than emitted, and no longer risks an NPE on missing data), covered by two new InfraTest cases.

Target

• I checked that the commit is targeting the correct branch (Camel 4 uses the main branch)

Tracking

• If this is a large change, bug fix, or code improvement, I checked there is a JIRA issue filed for the change (usually before you start working on it).

CAMEL-23237

Apache Camel coding standards and style

• I checked that each commit in the pull request has a meaningful subject line and body.
• I have run mvn clean install -DskipTests locally from root folder and I have committed all auto-generated changes.

AI-assisted contributions

• If this PR includes AI-generated code, commits have proper co-authorship attribution (e.g., Co-authored-by trailers) and the PR description identifies the AI tool used.

AI-assisted with Claude Code (Claude Opus 4.8) on behalf of Adriano Machado.

[github-actions]

🌟 Thank you for your contribution to the Apache Camel project! 🌟
🤖 CI automation will test this PR automatically.

🐫 Apache Camel Committers, please review the following items:

• First-time contributors require MANUAL approval for the GitHub Actions to run
• You can use the command /component-test (camel-)component-name1 (camel-)component-name2.. to request a test from the test bot although they are normally detected and executed by CI.
• You can label PRs using skip-tests and test-dependents to fine-tune the checks executed by this PR.
• Build and test logs are available in the summary page. Only Apache Camel committers have access to the summary.

⚠️ Be careful when sharing logs. Review their contents before sharing them publicly.

[github-actions]

🧪 CI tested the following changed modules:

• docs
• dsl/camel-jbang/camel-jbang-core

---

🔬 Scalpel shadow comparison — Scalpel: 8 tested, 8 compile-only — current: 6 all tested

Maveniverse Scalpel detected 16 affected modules (current approach: 6).

⚠️ Modules only in Scalpel (10)

• camel-jbang-core
• camel-jbang-it
• camel-jbang-main
• camel-jbang-plugin-edit
• camel-jbang-plugin-generate
• camel-jbang-plugin-kubernetes
• camel-jbang-plugin-test
• camel-launcher
• coverage
• docs

Skip-tests mode would test 8 modules (2 direct + 6 downstream), skip tests for 8 (generated code, meta-modules)

Modules Scalpel would test (8)

• camel-jbang-core
• camel-jbang-mcp
• camel-jbang-plugin-mcp
• camel-jbang-plugin-route-parser
• camel-jbang-plugin-tui
• camel-jbang-plugin-validate
• camel-launcher-container
• docs

Modules with tests skipped (8)

• camel-jbang-it
• camel-jbang-main
• camel-jbang-plugin-edit
• camel-jbang-plugin-generate
• camel-jbang-plugin-kubernetes
• camel-jbang-plugin-test
• camel-launcher
• coverage

ℹ️ Shadow mode — Scalpel observes but does not affect test execution. Learn more

⚠️ Some tests are disabled on GitHub Actions (@DisabledIfSystemProperty(named = "ci.env.name")) and require manual verification:

• dsl/camel-jbang/camel-jbang-core: 1 test(s) disabled on GitHub Actions

💡 Manual integration tests recommended:

You modified dsl/camel-jbang/camel-jbang-core. The related integration tests in dsl/camel-jbang/camel-jbang-it are excluded from CI. Consider running them manually:

mvn verify -f dsl/camel-jbang/camel-jbang-it -Djbang-it-test

All tested modules (16 modules)

• Camel :: Coverage
• Camel :: Docs
• Camel :: JBang :: Core
• Camel :: JBang :: Integration tests
• Camel :: JBang :: MCP
• Camel :: JBang :: Main
• Camel :: JBang :: Plugin :: Edit
• Camel :: JBang :: Plugin :: Generate
• Camel :: JBang :: Plugin :: Kubernetes
• Camel :: JBang :: Plugin :: MCP
• Camel :: JBang :: Plugin :: Route Parser
• Camel :: JBang :: Plugin :: TUI
• Camel :: JBang :: Plugin :: Testing
• Camel :: JBang :: Plugin :: Validate
• Camel :: Launcher
• Camel :: Launcher :: Container

---

⚙️ View full build and test results

[gnodet]

Nice work, @ammachado — this is a well-structured improvement to the catalog table rendering. The new CamelTableColumns toolkit centralizes what was previously hand-rolled in every command with differing widths, and the noBorderOverhead bug fix (from (n-1)*2 to n*2) is correct — AsciiTable NO_BORDERS pads every column with one leading and one trailing space.

The test coverage is good: CamelTableColumnsTest covers measure(), lastColumnWidth(), and the rendering-level regression test that verifies tables don't overflow the terminal width. The TerminalWidthHelperTest additions for parseColumns, fillWidth, and the localized Windows mode con output are thorough.

A few observations:

1. [observation] DEFAULT_WIDTH change from 120 to 80

This is documented in the upgrade guide, which is good. Worth noting that this changes the behavior for all existing callers of getTerminalWidth() that fall back to the default — not just the catalog commands. Any table/command that previously got 120 as the fallback will now get 80, which may truncate more aggressively when output is piped or redirected. The PR description says this is intentional ("the standard fallback when no terminal can be detected"), and 80 is indeed the standard terminal width assumption. Just flagging it since it's a cross-cutting default change.

2. [nit] parseColumns assumes column count is always the second integer

This works for both stty size ("rows cols") and English/French mode con output. However, some Windows locales or non-standard mode con output might have the integers in a different order (e.g., if a header line contains a number before the "Lines" field). The positional approach is pragmatic — label parsing would be fragile across locales — but worth documenting this assumption in the Javadoc (which it already does, so this is fine).

3. [positive] Bug fix in noBorderOverhead is backward-compatible

The fix changes the formula but all existing callers used flexWidth() which clamps to [min, max], so the 2-column shift only matters for the new uncapped fillWidth(). Nice that the PR identified and fixed this proactively.

CI: The failures (UpdateListTest.listUpdateVersions) are in version-update listing, unrelated to this PR's catalog table changes. Appears to be a pre-existing flaky test.

Note: this review does not replace specialized tools such as CodeRabbit, Sourcery, or SonarCloud.

Reviewed with Claude Code on behalf of Guillaume Nodet (@gnodet). This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.

[oscerd]

Thanks @ammachado — filling the DESCRIPTION column to the terminal width is a real improvement, the off-by-2 noBorderOverhead fix is correct, and I like that it's now backed by a rendering-level regression test (CamelTableColumnsTest.lastColumnWidthKeepsRenderedTableWithinTerminal) instead of an arithmetic-only assertion. There's a blocking CI regression though, so I'm requesting changes.

Blocker: the shared width change breaks UpdateListTest (deterministic, in CI)

This PR changes two things in the shared TerminalWidthHelper:

• DEFAULT_WIDTH 120 → 80 (the no-TTY / piped fallback)
• noBorderOverhead(n) from (columnCount - 1) * 2 to columnCount * 2

But UpdateList (commands/update/UpdateList.java) was intentionally left for follow-up and still calls flexWidth(terminalWidth(), 45, noBorderOverhead(4), 20, 80). In CI there is no TTY, so terminalWidth() returns the fallback, and the DESCRIPTION column width collapses:

• before: max(20, min(80, 120 − 45 − 6)) = 69 → the 43-char description fits on one line
• after: max(20, min(80, 80 − 45 − 8)) = 27 → the description word-wraps

UpdateListTest.listUpdateVersions (line 37) asserts .contains("Migrates Camel 4 application to Camel 4.9.0"), and that contiguous string no longer exists once the column wraps. The CI log confirms it: build (17, false) fails deterministically (through all surefire reruns) at UpdateListTest.listUpdateVersions:37, with the rendered table showing the wrapped Migrates Camel 4 … description. build (25, false) is the matrix cancel-cascade of the same failure.

Suggested fix

Either migrate UpdateList in this PR as well, or make UpdateListTest width-robust (e.g. pin COLUMNS, or assert against a wrap-tolerant substring).

Blast-radius note (non-blocking)

Lowering DEFAULT_WIDTH 120 → 80 narrows the piped/redirected fallback for every not-yet-migrated NO_BORDERS table, not only the catalog commands — UpdateList is just the one with a test pinning the old width. Worth a quick scan of the other process/action snapshot tests for the same coupling before the follow-up migration lands.

Everything else looks good — the upgrade-guide entry is clear and the new tests are strong. Once CI is green this should be an easy re-review.

---

Reviewed with Claude Code on behalf of Andrea Cosentino (@oscerd). This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.

[davsclaus]

80 width is way to small today

[ammachado]

The rationale for 80-column width is that this is (or was) the default width for the Windows Command Prompt terminal.

I'll revert this change.

[davsclaus]

Review: CAMEL-23237 — camel-jbang catalog tables fill the terminal width

Blocking Issue

1. Merge conflicts — The PR currently has CONFLICTING merge state. It needs a rebase onto main before CI can run and the changes can be verified.

Prior Review Status

The two prior CHANGES_REQUESTED reviews from @oscerd and @davsclaus appear to be addressed by commit a70d811 (reverts DEFAULT_WIDTH back to 120 and migrates UpdateList onto the CamelTableColumns toolkit). However, because of the merge conflict, CI cannot verify this.

Confirmed Findings

2. noBorderOverhead formula change is cross-cutting (non-blocking, informational)

The fix from (columnCount - 1) * 2 to columnCount * 2 is correct — AsciiTable NO_BORDERS pads every column with 1 leading + 1 trailing space. However, this affects ~30 existing callers across CamelMemoryLeak, CamelThreadDump, CamelSpanAction, LoggerAction, CamelHistoryAction, CamelBrowseAction, CamelBeanDump, CamelProcessorStatus, CamelRouteStatus, MessageTableHelper, etc.

The PR description states this is safe because all existing callers use flexWidth() which clamps to [min, max]. This is correct — the 2-column shift only becomes visible at mid-range terminal widths (where flex columns may render 2 chars narrower), and is actually a fix for the previous 2-char overflow. Worth noting for posterity but not blocking.

Questions / Observations

3. InfraBaseCommand still uses old pattern — InfraBaseCommand.java (line 213-214) calls flexWidth(...) with hand-rolled fixed column widths but was not migrated to the new CamelTableColumns toolkit. This is fine as follow-up work, but if the contributor wants this PR to be the single source of truth for catalog-style tables, InfraBaseCommand follows the same pattern.

4. Test coverage is good — CamelTableColumnsTest covers measure(), lastColumnWidth(), and importantly includes a rendering-level regression test that renders an actual AsciiTable and verifies all lines fit within the terminal width. TerminalWidthHelperTest additions for parseColumns, fillWidth, and localized Windows mode con output are thorough.

5. Upgrade guide entry — Present, clear, and appropriately scoped under the camel-jbang section. Documents the behavior change and recommends --json for scripting.

6. AI attribution — Properly disclosed in PR body and commit trailers.

Summary

This is a well-structured improvement. The CamelTableColumns toolkit centralizes duplicated column definitions, the noBorderOverhead fix is correct, and the adaptive column measurement via measure() is better than the previous hardcoded width guessing. The main action needed is resolving the merge conflict and getting CI green.

This review does not replace specialized tools such as CodeRabbit, Sourcery, or SonarCloud.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

[ammachado]

Following up on @davsclaus's review, specifically the InfraBaseCommand observation:

3. InfraBaseCommand still uses old pattern — InfraBaseCommand.java (line 213-214) calls flexWidth(...) with hand-rolled fixed column widths but was not migrated to the new CamelTableColumns toolkit. This is fine as follow-up work, but if the contributor wants this PR to be the single source of truth for catalog-style tables, InfraBaseCommand follows the same pattern.

camel infra list (InfraBaseCommand) is now migrated onto the toolkit (commit 3766f13). The infra table isn't quite the same shape as the catalog tables (its flex column was a middle column with NEWLINE wrapping, and it has two trailing unbounded columns), so the migration also made a small layout change: DESCRIPTION fills the terminal width, and IMPLEMENTATION / SERVICE_DATA truncate with an ellipsis instead of letting the raw JSON service data overflow the terminal. The full, structured service data remains available via --json (now covered by tests confirming it is embedded as nested JSON rather than an escaped string).

One design question for reviewers before considering this settled:

• Does SERVICE_DATA belong in the table at all? It currently renders raw JSON, which is not very readable in a terminal. Should the column be dropped from the table view (keeping the data available via --json only), or should it instead surface user-friendly, actionable information about the service, e.g. the port and credentials for a database?

Claude Code on behalf of Adriano Machado.

[davsclaus]

Nice work on this PR — the measured-column approach and the CamelTableColumns toolkit are a solid improvement over the hand-rolled per-command widths.

A few observations (none blocking):

• The noBorderOverhead fix from (n-1)*2 to n*2 is correct per AsciiTable's padding model. All ~50 existing flexWidth callers are safe because they clamp to [min, max], absorbing the 2-column reduction.
• The positional parseColumns() approach ("take the second integer") is robust for both stty size and localized mode con output — good call avoiding label-based parsing.
• The rendering-level regression test (lastColumnWidthKeepsRenderedTableWithinTerminal) directly validates the claim that sized tables don't overflow — that's the right test to have.
• Scope is well-bounded: migrating process/ and action/ commands can be follow-up work.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.