You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "gnodet (via GitHub)" <gi...@apache.org> on 2026/06/30 07:02:05 UTC
[PR] chore: shell panel UX and reliability improvements [camel]
gnodet opened a new pull request, #24328:
URL: https://github.com/apache/camel/pull/24328
## Summary
_Claude Code on behalf of Guillaume Nodet_
Six improvements to the embedded shell panel in the TUI dashboard (`ShellPanel.java`, `CamelMonitor.java`, `Shell.java`):
### Reliability
- **Replace reflection for ScreenTerminal history access**: cache the `Field` lookup at class load time, narrow `catch (Exception)` to `catch (NoSuchFieldException)` / `catch (IllegalAccessException)`, warn on first failure instead of silently returning empty results. Added TODO with JLine issue reference for when a public API is available.
- **Add logging to silently swallowed exceptions**: introduce `java.lang.System.Logger` (JDK platform logger, no dependency) to `ShellPanel` and `CamelMonitor`. Race condition catches (`ArrayIndexOutOfBoundsException` from concurrent `ScreenTerminal` resize) get `DEBUG`/`TRACE` logging. Real errors like failed `.mcp.json` writes get `WARNING`. No behavior changes — just visibility.
### UX
- **Render blinking cursor**: connect `ScreenTerminal` cursor position from `dump()` to `Frame.setCursorPosition()` so the terminal cursor blinks at the shell prompt. Only set when `scrollOffset == 0` (live view) and cursor is within bounds.
- **Fullscreen shell mode**: at 100% the tab content area is fully hidden (skip `renderContent` call). Footer hint now shows the **next** percentage instead of the current one, so users know what Shift+F6 will do.
- **Add scrollbar to shell panel**: follow the existing `StartupTab` pattern — `ScrollbarState` field, `Layout.horizontal()` split into content (fill) + scrollbar (1 col), `Scrollbar` widget. Maps `scrollOffset` (lines-from-bottom) to top-down position. Only shown when history exceeds viewport height.
- **Simplify shell prompt**: remove version string from prompt in both embedded `ShellPanel` and standalone `Shell` — saves ~20 chars of horizontal space. Version is already visible in the TUI header bar and the shell's startup banner respectively.
**Before:** `camel 4.21.0-SNAPSHOT> `
**After:** `camel> `
## Test plan
- [x] All 260 existing TUI tests pass (`mvn test -pl dsl/camel-jbang/camel-jbang-plugin-tui`)
- [x] Code formatted with `mvn formatter:format impsort:sort`
- [ ] Manual: open TUI with `camel tui`, press F6 to open shell, verify cursor blinks
- [ ] Manual: cycle Shift+F6 through 25% → 50% → 75% → 100% → 25%, verify footer shows next %
- [ ] Manual: run enough commands to create scrollback, verify scrollbar appears
- [ ] Manual: verify `camel shell` prompt shows `camel> ` without version
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: shell panel UX and reliability improvements [camel]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #24328:
URL: https://github.com/apache/camel/pull/24328#discussion_r3497287502
##########
dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/ShellPanel.java:
##########
@@ -269,17 +283,46 @@ void render(Frame frame, Rect area) {
lines = renderLiveView(screen, innerWidth, innerHeight);
}
+ // Split the inner area: content (fill) + scrollbar (1 col) when history exists
+ int totalLines = histSize + innerHeight;
+ boolean showScrollbar = totalLines > innerHeight;
+ Rect contentArea;
+ if (showScrollbar) {
+ List<Rect> hChunks = Layout.horizontal()
+ .constraints(Constraint.fill(), Constraint.length(1))
+ .split(inner);
+ contentArea = hChunks.get(0);
+
+ // Map scrollOffset (lines-from-bottom) to top-down position for ScrollbarState
+ int viewStart = Math.max(0, totalLines - scrollOffset - innerHeight);
+ scrollbarState
+ .contentLength(totalLines)
+ .viewportContentLength(innerHeight)
+ .position(viewStart);
+ frame.renderStatefulWidget(Scrollbar.builder().build(), hChunks.get(1), scrollbarState);
+ } else {
+ contentArea = inner;
+ }
+
frame.renderWidget(
Paragraph.builder()
.text(Text.from(lines))
.overflow(Overflow.CLIP)
.build(),
- inner);
+ contentArea);
+
+ // Set the real terminal cursor position so it blinks at the shell prompt.
+ // Only show the cursor in live view (not scrolled back) and when the shell is focused.
+ if (scrollOffset == 0 && cursor[1] >= 0 && cursor[1] < innerHeight
+ && cursor[0] >= 0 && cursor[0] < innerWidth) {
+ frame.setCursorPosition(contentArea.x() + cursor[0], contentArea.y() + cursor[1]);
+ }
}
void renderFooter(List<Span> spans) {
MonitorContext.hint(spans, "F6", "close");
- MonitorContext.hint(spans, "Shift+F6", SPLIT_PERCENTS[splitIndex] + "%");
+ int nextIndex = (splitIndex + 1) % SPLIT_PERCENTS.length;
+ MonitorContext.hint(spans, "Shift+F6", SPLIT_PERCENTS[nextIndex] + "%");
Review Comment:
Fixed — reverted to show the current percentage (`splitIndex`). Good point that it matches the zoom-indicator pattern. Also added tests that verify `renderFooter()` shows the correct percentage before and after `cycleHeight()`.
_Claude Code on behalf of Guillaume Nodet_
##########
dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/ShellPanel.java:
##########
@@ -616,14 +653,37 @@ static byte[] encodeKeyEvent(KeyEvent ke) {
};
}
+ // TODO: ScreenTerminal.history is private with no public accessor in JLine 4.2.1.
+ // File a JLine issue to add getHistory()/getHistorySize() public methods,
+ // then replace this reflection with direct calls.
+ // See: https://github.com/jline/jline3/issues/1266
+ private static final Field HISTORY_FIELD;
+ private static final AtomicBoolean HISTORY_WARN_LOGGED = new AtomicBoolean(false);
+
+ static {
+ Field f = null;
+ try {
+ f = ScreenTerminal.class.getDeclaredField("history");
+ f.setAccessible(true);
+ } catch (NoSuchFieldException e) {
+ System.err.println("[ShellPanel] ScreenTerminal.history field not found — "
+ + "scrollback will be unavailable. JLine API may have changed.");
+ }
+ HISTORY_FIELD = f;
Review Comment:
Good catch — replaced both `System.err.println` calls with `LOG.log(Level.WARNING, ...)` for consistency with the rest of the file.
Regarding alignment with PR #24322 (`org.slf4j.Logger` vs `java.lang.System.Logger`): since `System.Logger` is zero-dependency and SLF4J is not currently a dependency of this module, we went with the platform logger. Happy to align whichever direction the project prefers when both PRs merge.
_Claude Code on behalf of Guillaume Nodet_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: shell panel UX and reliability improvements [camel]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #24328:
URL: https://github.com/apache/camel/pull/24328#issuecomment-4841407251
Thanks for the review @davsclaus! All three points addressed in the latest push:
1. **Footer hint** — Reverted to show current percentage (`splitIndex`), matching the zoom-indicator pattern. ✅
2. **`System.err.println` → `LOG`** — Both calls in the static initializer and `getHistory()` now use `LOG.log(Level.WARNING, ...)`. ✅
3. **Tests** — Added 5 new tests: `panelPercentDefaultIs50`, `cycleHeightCyclesThroughPercents` (verifies full 50→75→100→25→50 wrap), `renderFooterShowsCurrentPercentage`, `renderFooterUpdatesAfterCycleHeight`, and `renderFooterContainsExpectedHints`. All 265 module tests pass. ✅
Re: SLF4J vs `System.Logger` alignment with #24322 — happy to converge whichever direction the project prefers once both PRs land.
_Claude Code on behalf of Guillaume Nodet_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: shell panel UX and reliability improvements [camel]
Posted by "davsclaus (via GitHub)" <gi...@apache.org>.
davsclaus commented on code in PR #24328:
URL: https://github.com/apache/camel/pull/24328#discussion_r3497337438
##########
dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/ShellPanel.java:
##########
@@ -616,14 +653,37 @@ static byte[] encodeKeyEvent(KeyEvent ke) {
};
}
+ // TODO: ScreenTerminal.history is private with no public accessor in JLine 4.2.1.
+ // File a JLine issue to add getHistory()/getHistorySize() public methods,
+ // then replace this reflection with direct calls.
+ // See: https://github.com/jline/jline3/issues/1266
+ private static final Field HISTORY_FIELD;
+ private static final AtomicBoolean HISTORY_WARN_LOGGED = new AtomicBoolean(false);
+
+ static {
+ Field f = null;
+ try {
+ f = ScreenTerminal.class.getDeclaredField("history");
+ f.setAccessible(true);
+ } catch (NoSuchFieldException e) {
+ System.err.println("[ShellPanel] ScreenTerminal.history field not found — "
+ + "scrollback will be unavailable. JLine API may have changed.");
+ }
+ HISTORY_FIELD = f;
Review Comment:
The TUI also has a notification system where it shows a message in the top to the user, so it can use that instead as the logger is not visibile for humans
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: shell panel UX and reliability improvements [camel]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #24328:
URL: https://github.com/apache/camel/pull/24328#issuecomment-4840745863
:star2: Thank you for your contribution to the Apache Camel project! :star2:
:robot: CI automation will test this PR automatically.
:camel: 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](https://camel.apache.org/community/team/#committers) have access to the summary.
:warning: Be careful when sharing logs. Review their contents before sharing them publicly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: shell panel improvements and cursor fix [camel]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet merged PR #24328:
URL: https://github.com/apache/camel/pull/24328
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: shell panel UX and reliability improvements [camel]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #24328:
URL: https://github.com/apache/camel/pull/24328#discussion_r3499266496
##########
dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/ShellPanel.java:
##########
@@ -616,14 +653,37 @@ static byte[] encodeKeyEvent(KeyEvent ke) {
};
}
+ // TODO: ScreenTerminal.history is private with no public accessor in JLine 4.2.1.
+ // File a JLine issue to add getHistory()/getHistorySize() public methods,
+ // then replace this reflection with direct calls.
+ // See: https://github.com/jline/jline3/issues/1266
+ private static final Field HISTORY_FIELD;
+ private static final AtomicBoolean HISTORY_WARN_LOGGED = new AtomicBoolean(false);
+
+ static {
+ Field f = null;
+ try {
+ f = ScreenTerminal.class.getDeclaredField("history");
+ f.setAccessible(true);
+ } catch (NoSuchFieldException e) {
+ System.err.println("[ShellPanel] ScreenTerminal.history field not found — "
+ + "scrollback will be unavailable. JLine API may have changed.");
+ }
+ HISTORY_FIELD = f;
Review Comment:
Yes, this will be included in the next release of JLine very soon.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: shell panel UX and reliability improvements [camel]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #24328:
URL: https://github.com/apache/camel/pull/24328#issuecomment-4840916428
<!-- ci-tested-modules -->
:test_tube: **CI tested the following changed modules:**
- `dsl/camel-jbang/camel-jbang-core`
- `dsl/camel-jbang/camel-jbang-plugin-tui`
:warning: **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
:bulb: **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
> ```
<details><summary>All tested modules (7 modules)</summary>
- 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
</details>
---
:gear: [View full build and test results](https://github.com/apache/camel/actions/runs/28426455705)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: shell panel UX and reliability improvements [camel]
Posted by "davsclaus (via GitHub)" <gi...@apache.org>.
davsclaus commented on code in PR #24328:
URL: https://github.com/apache/camel/pull/24328#discussion_r3496961930
##########
dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/ShellPanel.java:
##########
@@ -269,17 +283,46 @@ void render(Frame frame, Rect area) {
lines = renderLiveView(screen, innerWidth, innerHeight);
}
+ // Split the inner area: content (fill) + scrollbar (1 col) when history exists
+ int totalLines = histSize + innerHeight;
+ boolean showScrollbar = totalLines > innerHeight;
+ Rect contentArea;
+ if (showScrollbar) {
+ List<Rect> hChunks = Layout.horizontal()
+ .constraints(Constraint.fill(), Constraint.length(1))
+ .split(inner);
+ contentArea = hChunks.get(0);
+
+ // Map scrollOffset (lines-from-bottom) to top-down position for ScrollbarState
+ int viewStart = Math.max(0, totalLines - scrollOffset - innerHeight);
+ scrollbarState
+ .contentLength(totalLines)
+ .viewportContentLength(innerHeight)
+ .position(viewStart);
+ frame.renderStatefulWidget(Scrollbar.builder().build(), hChunks.get(1), scrollbarState);
+ } else {
+ contentArea = inner;
+ }
+
frame.renderWidget(
Paragraph.builder()
.text(Text.from(lines))
.overflow(Overflow.CLIP)
.build(),
- inner);
+ contentArea);
+
+ // Set the real terminal cursor position so it blinks at the shell prompt.
+ // Only show the cursor in live view (not scrolled back) and when the shell is focused.
+ if (scrollOffset == 0 && cursor[1] >= 0 && cursor[1] < innerHeight
+ && cursor[0] >= 0 && cursor[0] < innerWidth) {
+ frame.setCursorPosition(contentArea.x() + cursor[0], contentArea.y() + cursor[1]);
+ }
}
void renderFooter(List<Span> spans) {
MonitorContext.hint(spans, "F6", "close");
- MonitorContext.hint(spans, "Shift+F6", SPLIT_PERCENTS[splitIndex] + "%");
+ int nextIndex = (splitIndex + 1) % SPLIT_PERCENTS.length;
+ MonitorContext.hint(spans, "Shift+F6", SPLIT_PERCENTS[nextIndex] + "%");
Review Comment:
The footer hint should show the current shell panel size, not what it will become after pressing the key. This change is wrong — please revert to using `splitIndex` instead of `nextIndex`.
```suggestion
MonitorContext.hint(spans, "Shift+F6", SPLIT_PERCENTS[splitIndex] + "%");
```
##########
dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/ShellPanel.java:
##########
@@ -616,14 +653,37 @@ static byte[] encodeKeyEvent(KeyEvent ke) {
};
}
+ // TODO: ScreenTerminal.history is private with no public accessor in JLine 4.2.1.
+ // File a JLine issue to add getHistory()/getHistorySize() public methods,
+ // then replace this reflection with direct calls.
+ // See: https://github.com/jline/jline3/issues/1266
+ private static final Field HISTORY_FIELD;
+ private static final AtomicBoolean HISTORY_WARN_LOGGED = new AtomicBoolean(false);
+
+ static {
+ Field f = null;
+ try {
+ f = ScreenTerminal.class.getDeclaredField("history");
+ f.setAccessible(true);
+ } catch (NoSuchFieldException e) {
+ System.err.println("[ShellPanel] ScreenTerminal.history field not found — "
+ + "scrollback will be unavailable. JLine API may have changed.");
+ }
+ HISTORY_FIELD = f;
Review Comment:
Minor: `LOG` (the `System.Logger` introduced earlier in this file) is a static field initialized before this static block runs, so it's available here. Using `LOG.log(Level.WARNING, ...)` would be more consistent with the rest of the file.
Same applies to the `System.err.println` in `getHistory()` below.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: shell panel UX and reliability improvements [camel]
Posted by "davsclaus (via GitHub)" <gi...@apache.org>.
davsclaus commented on PR #24328:
URL: https://github.com/apache/camel/pull/24328#issuecomment-4840784225
> hows the next percentage instead of the current one, so users know what Shift+F6 will do.
No it should show current - its like a zoom setting in a web browser
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: shell panel UX and reliability improvements [camel]
Posted by "davsclaus (via GitHub)" <gi...@apache.org>.
davsclaus commented on code in PR #24328:
URL: https://github.com/apache/camel/pull/24328#discussion_r3497341309
##########
dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/ShellPanel.java:
##########
@@ -616,14 +653,37 @@ static byte[] encodeKeyEvent(KeyEvent ke) {
};
}
+ // TODO: ScreenTerminal.history is private with no public accessor in JLine 4.2.1.
+ // File a JLine issue to add getHistory()/getHistorySize() public methods,
+ // then replace this reflection with direct calls.
+ // See: https://github.com/jline/jline3/issues/1266
+ private static final Field HISTORY_FIELD;
+ private static final AtomicBoolean HISTORY_WARN_LOGGED = new AtomicBoolean(false);
+
+ static {
+ Field f = null;
+ try {
+ f = ScreenTerminal.class.getDeclaredField("history");
+ f.setAccessible(true);
+ } catch (NoSuchFieldException e) {
+ System.err.println("[ShellPanel] ScreenTerminal.history field not found — "
+ + "scrollback will be unavailable. JLine API may have changed.");
+ }
+ HISTORY_FIELD = f;
Review Comment:
Also this reflection will be fixed if jline makes a release with a public api for this, right ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org