CAMEL-23635: camel-jbang - Add shell panel to TUI#23605
Conversation
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
|
Oh can the toggle be 25%/50%/100% or does that not work to well ? |
|
Updated — the toggle now cycles through three sizes:
The split uses Claude Code on behalf of Guillaume Nodet |
|
There is a merge conflict, thanks |
|
Merge conflict resolved — rebased on latest main which added the Claude Code on behalf of Guillaume Nodet |
|
🧪 CI tested the following changed modules:
💡 Manual integration tests recommended:
All tested modules (6 modules)
|
davsclaus
left a comment
There was a problem hiding this comment.
Nice feature — the JLine ScreenTerminal + LineDisciplineTerminal wiring is solid, and the convertAttrToStyle() bit extraction correctly matches JLine's own generateSpanTag(). A couple of minor observations below.
This review does not replace specialized AI review tools (CodeRabbit, Sourcery) or static analysis (SonarCloud).
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
a3025ba to
c78d71a
Compare
|
Are we waiting for a tamboui fix ? |
c78d71a to
e7779f1
Compare
|
Camel JBang has been renamed to Camel CLI so there are some stale doc files you need to regen |
e7779f1 to
fc3dbae
Compare
|
No, we don't need a TamboUI fix. TamboUI 0.3.0 is the latest release and works fine for this feature. Claude Code on behalf of Guillaume Nodet |
|
Our branch is rebased on top of the latest main which already includes the Camel JBang → Camel CLI rename. No stale doc files in our diff. Claude Code on behalf of Guillaume Nodet |
ac4c76b to
2978033
Compare
0f566d3 to
cd2b518
Compare
cd2b518 to
56556b1
Compare
a3f6ba3 to
b3c74ab
Compare
davsclaus
left a comment
There was a problem hiding this comment.
Late follow-up review against project conventions — the PR is already approved and comments addressed.
Findings (all low severity)
-
ShellPanel.destroy()never called —CamelMonitornever callsdestroy()on TUI exit. The shell thread is daemon so the JVM won't hang, but the virtual terminal won't be cleanly shut down. Minor resource leak if TUI is restarted within the same JVM. -
Overlays suppressed when shell is open — draw overlay, kill confirm, caption, and help overlay (
F1) don't render while the shell panel is visible. Likely intentional (keyboard focus goes to shell), but worth confirming users know to close the shell first forF1help. -
No tests — consistent with the current coverage pattern for TUI and
Ask.javamodules. TheEndpointsTabNPE fix is a straightforward null-guard. Thelist_processes/select_processtools could be tested in isolation but would require mockingRuntimeHelper.
What looks good
ScreenTerminal→ TamboUIconvertAttrToStyle()bit extraction is correctDelegateOutputStreamcleanly breaks theLineDisciplineTerminal↔ScreenTerminalOutputStreamcircular dependencyActionsPopup.resolveAction()refactor from index arithmetic to explicitbuildVisualActionList()is much more maintainableEndpointsTab.unbox()NPE fix is correct and minimalencodeKeyEvent()ANSI encoding covers all standard keys
No blocking issues found.
This review does not replace specialized AI review tools (CodeRabbit, Sourcery) or static analysis (SonarCloud).
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Embed a JLine interactive shell inside the TUI using a virtual terminal. ShellPanel wiring (same pattern as JLine's WebTerminal): - LineDisciplineTerminal provides the master/slave virtual terminal - ScreenTerminal acts as VT100 emulator with readable screen buffer - ScreenTerminalOutputStream bridges terminal output to screen buffer - Shell runs in background thread via ShellBuilder + PicocliCommandRegistry - Screen buffer dumped each frame, converted to TamboUI Span/Line widgets - Key events encoded as ANSI escape sequences and forwarded to terminal TUI integration: - F6 shortcut to toggle shell panel open/close - Shift+F6 cycles shell height (25%/50%/75%) - F2 Actions menu: Shell entry at the bottom - Split-screen layout with monitoring tabs - Separator line with "Shell" title between panels - Footer hints updated to show F6 Process selection: - TUI's selected integration propagated via EnvironmentHelper - camel ask auto-targets the selected process (no --name needed) - list_processes / select_process tools for runtime switching Additional fixes: - EndpointsTab: null guard for history LinkedList entries (NPE fix) - ActionsPopup: explicit action list in resolveAction (fixes F2 mapping) - Printer output redirected through virtual terminal writer Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
b3c74ab to
d56eac1
Compare
gnodet
left a comment
There was a problem hiding this comment.
Thanks for the review. All 3 findings addressed:
-
destroy()never called — fixed:shellPanel.destroy()now called inCamelMonitor.doCall()finally block on TUI exit. -
Overlays suppressed — fixed: overlays (help, caption, kill confirm, draw, actions popup) now render on top of the full content area regardless of shell panel state. F1 help works even when shell is open.
-
No tests — acknowledged. The TUI module has no test infrastructure for interactive rendering. The
EndpointsTab.unbox()fix andAskprocess discovery are straightforward. Will add tests if/when a test harness is available.
Claude Code on behalf of Guillaume Nodet
CAMEL-23635
Summary
Embeds a JLine interactive shell inside the TUI, accessible via F6 or F2 → Shell.
Architecture
Uses JLine's virtual terminal infrastructure (same pattern as
WebTerminal):LineDisciplineTerminal— master/slave virtual terminalScreenTerminal— VT100 emulator with readable screen bufferScreenTerminalOutputStream— bridges terminal output to screen bufferShellBuilder+PicocliCommandRegistrySpan/LinewidgetsFeatures
camel asklist_processes/select_processtools for runtime process switchingAdditional fixes
EndpointsTab: null guard for historyLinkedListentries (NPE fix)ActionsPopup: explicit action list inresolveAction()(fixes F2→Shell mapping when MCP is disabled)Test plan
catalog component --filter=kafka) — output with colorscd,ls,pwdworkaskauto-targets the TUI's selected integrationClaude Code on behalf of Guillaume Nodet