TUI #16
TUI #16
Conversation
📝 WalkthroughWalkthroughAdds a new Terminal UI (TUI) to the Khronos CLI: app state, event loop, widgets, theming, rendering, and a TUI entrypoint. CLI defaults to launching the TUI when no script is provided. New dependencies and docs included; one platform-gated error path adjusted. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Terminal
participant EventHandler
participant App
participant Cli
rect rgb(220,235,250)
note over Terminal,App: Initialization
Terminal->>Terminal: enable raw mode, alt screen, mouse
Terminal->>App: create App state
Terminal->>EventHandler: create EventHandler(tick_rate)
end
rect rgb(250,245,220)
loop until App.should_quit
Terminal->>App: render(frame, app)
User->>Terminal: key/resize/input
Terminal->>EventHandler: poll event
EventHandler-->>App: emit Event (Key/Resize/Tick)
alt Theme switcher active
App->>App: update filter/selection/apply
else Help/Modal active
App->>App: handle modal keys (ESC/Enter)
else Normal REPL mode
App->>Cli: on Enter -> spawn_script / execute
Cli-->>App: execution result -> App.add_output(...)
App->>App: save history, update scroll/time
end
end
end
rect rgb(220,245,220)
note over Terminal: Teardown
Terminal->>Terminal: disable raw mode, restore screen
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (8)
crates/cli/src/tui/widgets/output_panel.rs (1)
128-133: Loose substring matching may cause false positives.The check
line.contains("nil")will match words like "vanilla" or "nillable". Similarly,line.contains("true")matches "truetype" andline.contains("false")matches "falsehood". Consider using word boundary matching or checking for standalone values.🔎 Proposed fix using word boundaries
- } else if line.contains("nil") || line.contains("true") || line.contains("false") { + } else if line.split_whitespace().any(|w| matches!(w, "nil" | "true" | "false")) {Alternatively, use a regex with word boundaries if more complex patterns are needed.
crates/cli/src/tui/widgets/dashboard.rs (2)
117-135: Duplicated helper functioncentered_rect.This function is identical to
centered_rectincrates/cli/src/tui/ui.rs(lines 72-90). Extract it to a shared utility module to avoid duplication.🔎 Suggested approach
Create a shared utility module, e.g.,
crates/cli/src/tui/utils.rs:// crates/cli/src/tui/utils.rs use ratatui::layout::{Constraint, Direction, Layout, Rect}; pub fn centered_rect(percent_x: u16, percent_y: u16, r: Rect) -> Rect { let popup_layout = Layout::default() .direction(Direction::Vertical) .constraints([ Constraint::Percentage((100 - percent_y) / 2), Constraint::Percentage(percent_y), Constraint::Percentage((100 - percent_y) / 2), ]) .split(r); Layout::default() .direction(Direction::Horizontal) .constraints([ Constraint::Percentage((100 - percent_x) / 2), Constraint::Percentage(percent_x), Constraint::Percentage((100 - percent_x) / 2), ]) .split(popup_layout[1])[1] }Then import from both
ui.rsanddashboard.rs.
52-60: Consider naming the magic number for spacing.The value
5used for spacing could be extracted to a named constant for clarity.🔎 Proposed fix
+ const INPUT_BAR_CLEARANCE: u16 = 5; + // Spacer to push commands down below the input bar - let commands_area_y = chunks[2].y + 5; // Leave room for input bar - let commands_height = chunks[2].height.saturating_sub(5); + let commands_area_y = chunks[2].y + INPUT_BAR_CLEARANCE; + let commands_height = chunks[2].height.saturating_sub(INPUT_BAR_CLEARANCE);crates/cli/src/tui/ui.rs (2)
72-91: Duplicatecentered_rectfunction.This helper is duplicated in
theme_switcher.rs(lines 112-130). Consider extracting to a shared utility module to avoid code duplication.
32-32: Unnecessary clone on Copy type.
ThemederivesCopy(pertheme.rsline 5), so.clone()is redundant and can be removed.🔎 Proposed fix
- frame.render_widget(Dashboard::new(theme.clone()), main_chunks[0]); + frame.render_widget(Dashboard::new(*theme), main_chunks[0]);crates/cli/src/tui/widgets/theme_switcher.rs (1)
93-95:highlight_stylehas no effect withoutListState.The
highlight_styleonListonly applies when rendering withStatefulWidget::renderand aListState. Since you're manually styling selected items in lines 79-89, this is redundant and can be removed.🔎 Proposed fix
List::new(list_items) - .highlight_style(Style::default().bg(theme.card)) .render(chunks[1], buf);crates/cli/src/tui/theme.rs (1)
334-338: Potentially confusingDefaultimplementation.
Self::default()insideimpl Defaultcalls the inherentTheme::default()method due to method resolution rules, but this pattern is fragile and confusing. If someone renames or removes the inherentdefault()method, this becomes infinite recursion.🔎 Proposed fix - be explicit
impl Default for Theme { fn default() -> Self { - Self::default() + Self::zen_dark() } }crates/cli/src/tui/app.rs (1)
164-173: O(n) removal from front ofVec.
self.command_history.remove(0)is O(n) since it shifts all elements. WithMAX_COMMAND_HISTORY = 1000, this is noticeable on frequent command entries. Consider usingVecDequelike you do foroutput.🔎 Proposed fix
In the struct definition:
- pub command_history: Vec<String>, + pub command_history: VecDeque<String>,In
save_to_history:pub fn save_to_history(&mut self) { let input = self.get_input(); if !input.trim().is_empty() { - self.command_history.push(input); + self.command_history.push_back(input); if self.command_history.len() > MAX_COMMAND_HISTORY { - self.command_history.remove(0); + self.command_history.pop_front(); } } self.history_index = None; }Note: You'll need to update
history_prevandhistory_nextto use index-based access whichVecDequesupports.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
crates/cli/Cargo.tomlcrates/cli/README.mdcrates/cli/src/cli.rscrates/cli/src/main.rscrates/cli/src/tui/app.rscrates/cli/src/tui/events.rscrates/cli/src/tui/mod.rscrates/cli/src/tui/theme.rscrates/cli/src/tui/ui.rscrates/cli/src/tui/widgets/dashboard.rscrates/cli/src/tui/widgets/mod.rscrates/cli/src/tui/widgets/output_panel.rscrates/cli/src/tui/widgets/repl_input.rscrates/cli/src/tui/widgets/status_bar.rscrates/cli/src/tui/widgets/theme_switcher.rscrates/cli/src/tui_entrypoint.rscrates/runtime/src/plugins/antiraid/httpserver/mod.rs
🧰 Additional context used
🧬 Code graph analysis (8)
crates/cli/src/cli.rs (1)
crates/cli/src/tui_entrypoint.rs (1)
run_tui(16-221)
crates/cli/src/tui/widgets/output_panel.rs (2)
crates/cli/src/tui/app.rs (2)
new(73-97)default(258-260)crates/cli/src/tui/ui.rs (1)
render(11-70)
crates/cli/src/tui/ui.rs (3)
crates/cli/src/tui/widgets/status_bar.rs (2)
render(30-163)new(16-18)crates/cli/src/tui/widgets/theme_switcher.rs (3)
render(22-109)new(16-18)centered_rect(113-131)crates/cli/src/tui/app.rs (2)
default(258-260)new(73-97)
crates/cli/src/tui/widgets/dashboard.rs (2)
crates/cli/src/tui/ui.rs (2)
render(11-70)centered_rect(73-91)crates/cli/src/tui/theme.rs (2)
default(307-309)default(335-337)
crates/cli/src/tui/events.rs (8)
crates/cli/src/cli.rs (1)
new(88-90)crates/cli/src/tui/app.rs (1)
new(73-97)crates/cli/src/tui/widgets/dashboard.rs (1)
new(16-18)crates/cli/src/tui/widgets/output_panel.rs (1)
new(16-18)crates/cli/src/tui/widgets/repl_input.rs (1)
new(10-12)crates/cli/src/tui/widgets/status_bar.rs (1)
new(16-18)crates/cli/src/tui/widgets/theme_switcher.rs (1)
new(16-18)crates/cli/src/tui/theme.rs (1)
next(313-318)
crates/cli/src/tui/widgets/repl_input.rs (2)
crates/cli/src/tui/app.rs (2)
new(73-97)default(258-260)crates/cli/src/tui/ui.rs (1)
render(11-70)
crates/cli/src/tui/widgets/status_bar.rs (1)
crates/cli/src/tui/app.rs (2)
new(73-97)default(258-260)
crates/cli/src/tui/app.rs (6)
crates/cli/src/tui/widgets/dashboard.rs (1)
new(16-18)crates/cli/src/tui/widgets/output_panel.rs (1)
new(16-18)crates/cli/src/tui/widgets/repl_input.rs (1)
new(10-12)crates/cli/src/tui/widgets/status_bar.rs (1)
new(16-18)crates/cli/src/tui/widgets/theme_switcher.rs (1)
new(16-18)crates/cli/src/tui/theme.rs (3)
default(307-309)default(335-337)all(291-304)
🪛 markdownlint-cli2 (0.18.1)
crates/cli/README.md
132-132: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
200-200: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
205-205: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (21)
crates/runtime/src/plugins/antiraid/httpserver/mod.rs (2)
411-415: LGTM! Proper cross-platform compilation fix.The
#[cfg(unix)]attribute correctly gates the catch-all error arm, ensuring the match is exhaustive on non-Unix platforms (where only theTcpvariant exists) and correct on Unix platforms (where bothUnixandTcpvariants exist).
417-421: LGTM! Consistent cross-platform fix.The
#[cfg(unix)]attribute properly mirrors the fix applied to theaddrgetter, ensuring correct compilation across all platforms.crates/cli/Cargo.toml (1)
24-35: LGTM - TUI dependencies are appropriate.The selected Ratatui ecosystem libraries (crossterm, tui-textarea) and supporting crates for syntax highlighting (syntect), progress indication (indicatif), and text handling (unicode-width, unicode-segmentation, textwrap) are well-suited for building a terminal UI.
crates/cli/src/cli.rs (1)
310-319: LGTM - TUI entrypoint integration is clean.The new
Tuivariant follows the existing pattern for other entrypoint actions with consistent verbose logging and appropriate error handling to stderr.crates/cli/src/main.rs (2)
8-9: LGTM - Module declarations added correctly.
447-448: Behavior change: Default is now TUI instead of REPL.This changes the default interactive mode from the classic REPL to the new TUI. Users who prefer the traditional REPL will need to use the
/replcommand within the TUI or explicitly request it. Consider documenting this in the README or changelog.crates/cli/src/tui/mod.rs (1)
1-8: LGTM - Clean module structure.The TUI module organization with separate concerns (app state, events, theme, UI rendering, widgets) follows good architectural principles.
crates/cli/src/tui/widgets/output_panel.rs (1)
20-36: LGTM - Scroll calculation logic is correct.The visible lines calculation correctly implements a "tail-follow" behavior where the most recent output is shown by default, and increasing
scroll_offsetreveals older history.crates/cli/src/tui/widgets/repl_input.rs (1)
15-63: LGTM - Widget implementation follows standard patterns.Cloning the
TextAreaon each render is a common pattern withtui-textareasince theWidgettrait consumesself. The conditional styling for dashboard vs REPL mode provides appropriate visual distinction.crates/cli/src/tui/widgets/dashboard.rs (1)
36-47: LGTM - ASCII logo rendering is well-implemented.The logo is properly styled with the theme's primary color and centered alignment.
crates/cli/src/tui/widgets/status_bar.rs (1)
1-27: LGTM!The struct definition and
get_mode_texthelper are clean and well-organized. Mode mapping to icons and text labels is appropriate.crates/cli/src/tui/ui.rs (1)
29-58: LGTM!The conditional rendering logic for dashboard vs REPL mode is clear and handles the state transitions appropriately.
crates/cli/src/tui/widgets/mod.rs (1)
1-13: LGTM!Clean module facade pattern with proper public re-exports for all TUI widgets.
crates/cli/src/tui_entrypoint.rs (1)
99-168: LGTM!The command execution flow is well-structured with proper history management, timing capture, and error handling for Lua execution.
crates/cli/src/tui/events.rs (2)
29-45: Blocking poll in async context.
event::poll()andevent::read()are blocking calls. While they work correctly here since the TUI runs on a dedicated thread context, be aware this blocks the current task. If you later need concurrent operations (e.g., background updates), consider usingcrossterm::event::EventStreamwith async support.
48-73: LGTM!Key matching utilities are well-defined and cover common shortcuts. The helper functions provide a clean abstraction for key event handling.
crates/cli/src/tui/widgets/theme_switcher.rs (1)
74-91: LGTM!Theme list rendering with per-item selection styling is correctly implemented.
crates/cli/src/tui/theme.rs (2)
340-370: LGTM!The HSL to RGB conversion is correctly implemented following the standard algorithm.
372-393: LGTM!Good test coverage for theme cycling behavior and verification that all themes are present.
crates/cli/src/tui/app.rs (2)
71-97: LGTM!App constructor properly initializes all state with sensible defaults and adds a welcome message.
175-214: LGTM!History navigation logic correctly handles edge cases (empty history, boundaries) and properly reconstructs the TextArea from historical commands.
| pub async fn run_tui(cli: &mut Cli) -> io::Result<()> { | ||
| // Setup terminal | ||
| enable_raw_mode()?; | ||
| let mut stdout = io::stdout(); | ||
| execute!(stdout, EnterAlternateScreen, EnableMouseCapture)?; | ||
| let backend = CrosstermBackend::new(stdout); | ||
| let mut terminal = Terminal::new(backend)?; |
There was a problem hiding this comment.
Terminal state not restored on error during setup or main loop.
If any operation fails between enable_raw_mode() and the cleanup block (lines 212-218), the terminal will be left in raw mode with the alternate screen active, making it unusable. This includes panics and early ? returns.
Consider using a guard pattern (drop guard or scopeguard crate) to ensure cleanup always runs.
🔎 Proposed fix using a cleanup helper
+fn restore_terminal(terminal: &mut Terminal<CrosstermBackend<std::io::Stdout>>) -> io::Result<()> {
+ disable_raw_mode()?;
+ execute!(
+ terminal.backend_mut(),
+ LeaveAlternateScreen,
+ DisableMouseCapture
+ )?;
+ terminal.show_cursor()?;
+ Ok(())
+}
pub async fn run_tui(cli: &mut Cli) -> io::Result<()> {
// Setup terminal
enable_raw_mode()?;
let mut stdout = io::stdout();
- execute!(stdout, EnterAlternateScreen, EnableMouseCapture)?;
+ let setup_result = execute!(stdout, EnterAlternateScreen, EnableMouseCapture);
+ if setup_result.is_err() {
+ let _ = disable_raw_mode();
+ return setup_result;
+ }
let backend = CrosstermBackend::new(stdout);
let mut terminal = Terminal::new(backend)?;
+ let result = run_tui_inner(cli, &mut terminal).await;
+
+ // Always restore terminal
+ let _ = restore_terminal(&mut terminal);
+ result
+}
+
+async fn run_tui_inner(cli: &mut Cli, terminal: &mut Terminal<CrosstermBackend<std::io::Stdout>>) -> io::Result<()> {
// ... main loop code ...Committable suggestion skipped: line range outside the PR's diff.
| let build_status = " 🛠️ READY "; | ||
| let builder_badge_width = build_status.len() as u16 + 4; // prefix/suffix | ||
|
|
||
| if current_width + builder_badge_width < available_width { | ||
| let padding = available_width - current_width - builder_badge_width; | ||
| spans.push(Span::raw(" ".repeat(padding as usize))); |
There was a problem hiding this comment.
Incorrect width calculation for emoji-containing string.
build_status.len() returns byte length, not display width. The string " 🛠️ READY " contains multi-byte emoji characters, so .len() will overestimate the width (returns ~17 bytes vs ~12 display columns). This causes the right-aligned badge positioning to be incorrect.
Use unicode_width or Ratatui's Span::width() for consistent measurement.
🔎 Proposed fix
// Right-aligned: Build Status (Badge style)
let build_status = " 🛠️ READY ";
- let builder_badge_width = build_status.len() as u16 + 4; // prefix/suffix
+ let build_status_span = Span::raw(build_status);
+ let builder_badge_width = build_status_span.width() as u16 + 4; // prefix/suffix📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let build_status = " 🛠️ READY "; | |
| let builder_badge_width = build_status.len() as u16 + 4; // prefix/suffix | |
| if current_width + builder_badge_width < available_width { | |
| let padding = available_width - current_width - builder_badge_width; | |
| spans.push(Span::raw(" ".repeat(padding as usize))); | |
| let build_status = " 🛠️ READY "; | |
| let build_status_span = Span::raw(build_status); | |
| let builder_badge_width = build_status_span.width() as u16 + 4; // prefix/suffix | |
| if current_width + builder_badge_width < available_width { | |
| let padding = available_width - current_width - builder_badge_width; | |
| spans.push(Span::raw(" ".repeat(padding as usize))); |
🤖 Prompt for AI Agents
In crates/cli/src/tui/widgets/status_bar.rs around lines 136-141, the code uses
build_status.len() to compute display width but len() returns byte length and
miscalculates strings with emoji; replace the byte-length calculation with a
display-width measurement (e.g., use
unicode_width::UnicodeWidthStr::width(build_status) and cast to u16) or use
Ratatui's Span::width() to compute the actual column width, update
builder_badge_width accordingly, and add the necessary use/import so the padding
calculation uses the display width rather than .len().
| // Clear the area before rendering to simulation floating | ||
| Clear.render(switcher_area, buf); |
There was a problem hiding this comment.
Minor typo.
"simulation" should be "simulate".
🔎 Proposed fix
- // Clear the area before rendering to simulation floating
+ // Clear the area before rendering to simulate floating📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Clear the area before rendering to simulation floating | |
| Clear.render(switcher_area, buf); | |
| // Clear the area before rendering to simulate floating | |
| Clear.render(switcher_area, buf); |
🤖 Prompt for AI Agents
In crates/cli/src/tui/widgets/theme_switcher.rs around lines 29-30, the inline
comment contains a typo: "simulation" should be "simulate"; update the comment
to read "Clear the area before rendering to simulate floating" (or similar) so
the verb is correct while leaving the surrounding code unchanged.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
crates/cli/src/tui_entrypoint.rs (1)
16-22: Terminal state not restored on error during setup or main loop.This concern from a previous review remains unaddressed. If any operation fails between
enable_raw_mode()(line 18) and the cleanup block (lines 242-248), the terminal will be left in raw mode with the alternate screen active. Consider using a drop guard or thescopeguardcrate to ensure cleanup always runs.
🧹 Nitpick comments (5)
crates/cli/src/tui/widgets/quit_modal.rs (1)
58-76: Extractcentered_rectinto a shared utility module.This helper function is duplicated verbatim in
help_modal.rs,about_modal.rs,theme_switcher.rs, andui.rs. Consider extracting it to a shared module (e.g.,crates/cli/src/tui/utils.rsorcrates/cli/src/tui/widgets/mod.rs) to reduce duplication and ensure consistency.crates/cli/src/tui_entrypoint.rs (1)
189-198: Useis_some()instead ofif let Some(_).The pattern
if let Some(_) = ...is less idiomatic thanif ... .is_some()when the value isn't used.🔎 Suggested fix
- KeyCode::Tab => { - // Autocomplete on Tab - if let Some(_) = app.get_active_suggestion() { - app.autocomplete(); - } else { + KeyCode::Tab => { + // Autocomplete on Tab + if app.get_active_suggestion().is_some() { + app.autocomplete(); + } else {crates/cli/src/tui/ui.rs (1)
25-27: Remove duplicate comment.The comment "Render status bar" appears twice (lines 25-26 and 27).
🔎 Suggested fix
- - // Render status bar - // Render status bar at the bottom + // Render status bar at the bottomcrates/cli/src/tui/widgets/repl_input.rs (1)
36-36: Unused variableprompt.The
promptvariable is defined but only used in the interactive block's title. In the non-interactive branch, it's never referenced.🔎 Suggested fix
- let prompt = if self.app.is_interactive { "> " } else { " " }; - let block = if !self.app.is_interactive { // Dashboard Style: Search bar look ratatui::widgets::Block::default() @@ -46,6 +44,7 @@ // .title(" Ask anything... or type /help ") // Removed for cleaner look } else { // REPL Style: Minimal top separator + let prompt = "> "; ratatui::widgets::Block::default() .borders(ratatui::widgets::Borders::TOP) .border_style(Style::default().fg(theme.border))crates/cli/src/tui/app.rs (1)
183-186: Consider usingVecDequeforcommand_historyto avoid O(n) removal.
Vec::remove(0)is O(n) because it shifts all elements. WithMAX_COMMAND_HISTORY = 1000, this could have noticeable overhead when the history is full. UsingVecDequewould makepop_front()O(1), matching the pattern already used foroutput.🔎 Suggested changes
In the struct definition:
- /// Command history - pub command_history: Vec<String>, + /// Command history + pub command_history: VecDeque<String>,In
save_to_history:if !input.trim().is_empty() { self.command_history.push(input); if self.command_history.len() > MAX_COMMAND_HISTORY { - self.command_history.remove(0); + self.command_history.pop_front(); } }Note: This would require updating index-based access in
history_prevandhistory_nextto useget()or iterator patterns.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
crates/cli/src/tui/app.rscrates/cli/src/tui/ui.rscrates/cli/src/tui/widgets/about_modal.rscrates/cli/src/tui/widgets/help_modal.rscrates/cli/src/tui/widgets/mod.rscrates/cli/src/tui/widgets/quit_modal.rscrates/cli/src/tui/widgets/repl_input.rscrates/cli/src/tui_entrypoint.rsdocs/primitives.md
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/cli/src/tui/widgets/mod.rs
🧰 Additional context used
🧬 Code graph analysis (4)
crates/cli/src/tui/widgets/about_modal.rs (3)
crates/cli/src/tui/app.rs (2)
new(86-113)default(341-343)crates/cli/src/tui/widgets/quit_modal.rs (3)
new(15-17)render(21-55)centered_rect(58-76)crates/cli/src/tui/ui.rs (2)
render(11-88)centered_rect(91-109)
crates/cli/src/tui/ui.rs (3)
crates/cli/src/tui/widgets/about_modal.rs (1)
centered_rect(58-76)crates/cli/src/tui/widgets/help_modal.rs (1)
centered_rect(79-97)crates/cli/src/tui/widgets/theme_switcher.rs (1)
centered_rect(113-131)
crates/cli/src/tui_entrypoint.rs (9)
crates/cli/src/tui/app.rs (1)
new(86-113)crates/cli/src/tui/widgets/repl_input.rs (2)
new(10-12)render(16-105)crates/cli/src/tui/events.rs (5)
new(24-26)is_quit(54-57)is_help(71-73)is_theme_switch(60-62)is_clear(66-68)crates/cli/src/cli.rs (1)
new(88-90)crates/cli/src/tui/widgets/dashboard.rs (2)
new(16-18)render(22-113)crates/cli/src/tui/widgets/output_panel.rs (2)
new(16-18)render(40-158)crates/cli/src/tui/widgets/status_bar.rs (2)
new(16-18)render(30-163)crates/cli/src/tui/widgets/theme_switcher.rs (2)
new(16-18)render(22-109)crates/cli/src/tui/ui.rs (1)
render(11-88)
crates/cli/src/tui/widgets/help_modal.rs (3)
crates/cli/src/tui/app.rs (2)
new(86-113)default(341-343)crates/cli/src/tui/widgets/quit_modal.rs (3)
new(15-17)render(21-55)centered_rect(58-76)crates/cli/src/tui/ui.rs (2)
render(11-88)centered_rect(91-109)
🪛 markdownlint-cli2 (0.18.1)
docs/primitives.md
522-522: Hard tabs
Column: 1
(MD010, no-hard-tabs)
523-523: Hard tabs
Column: 1
(MD010, no-hard-tabs)
🔇 Additional comments (13)
docs/primitives.md (2)
1-1: Header capitalization looks good.Standard title-case formatting for the documentation heading.
Also applies to: 3-3
722-729: Documentation section for HTTPClient is well-structured.The new documentation section follows the established pattern for plugin fields, with correct module reference and proper placement within the TemplateContext documentation.
crates/cli/src/tui/widgets/quit_modal.rs (1)
10-56: Modal implementation looks good.The QuitModal follows a consistent pattern with other modals: themed styling, centered rendering via
Clear+Block, and clear user instructions. The destructive color scheme appropriately signals the quit action.crates/cli/src/tui_entrypoint.rs (1)
35-239: Main event loop is well-structured.The event loop correctly handles modal state transitions, input processing, and command execution. The modal priority order (theme switcher → quit → help/about → normal input) ensures proper UX. The async script execution with timing metrics is a nice touch.
crates/cli/src/tui/widgets/help_modal.rs (2)
10-76: Help modal implementation is clean and consistent.The widget follows the established modal pattern and provides clear, styled help content. The section headers with empty values for styling differentiation is a reasonable approach.
79-97: Duplicatecentered_recthelper.Already flagged in
quit_modal.rs- this should be extracted to a shared location.crates/cli/src/tui/ui.rs (1)
11-88: Well-organized render function with proper modal layering.The render function correctly handles the different UI modes (dashboard vs REPL) and layers modals in the proper z-order. The theme switcher renders last, ensuring it's always on top.
crates/cli/src/tui/widgets/repl_input.rs (2)
64-104: Suggestion overlay has limitations acknowledged in comments.The cursor position calculation (line 73) assumes single-line input without scrolling, which is noted in the comments. This is acceptable for slash commands but may need refinement if multi-line input with suggestions is needed later.
15-62: Widget implementation follows ratatui patterns correctly.The textarea cloning and styling application is the standard approach when working with
tui-textareain a Widget context. The dual block styles for dashboard vs REPL modes provide good visual distinction.crates/cli/src/tui/widgets/about_modal.rs (1)
10-56: About modal implementation is clean.Good use of
env!("CARGO_PKG_VERSION")to pull the version at compile time. The styling is consistent with other modals.crates/cli/src/tui/app.rs (3)
272-303: Modal toggle methods correctly ensure mutual exclusivity.The toggle methods close other modals when opening a new one, preventing confusing overlapping states. This is a good UX pattern.
84-113: App initialization is comprehensive and well-documented.The
new()method properly initializes all state with sensible defaults. The placeholder text and welcome message provide good onboarding UX.
305-337: Autocomplete logic is correct and safe.
get_active_suggestioncorrectly returnsNonefor non-slash inputs and only suggests when the input is a strict prefix. Theautocompletemethod adds a trailing space for convenience, which is a nice UX touch.
| // If help is shown, overlay it (using Dashboard/Help widget) | ||
| if app.show_help { | ||
| let help_area = centered_rect(80, 80, frame.area()); | ||
| frame.render_widget(Dashboard::new(theme.clone()), help_area); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for all assignments to show_help
rg -n 'show_help\s*=' --type rustRepository: Anti-Raid/khronos
Length of output: 134
🏁 Script executed:
# Search for show_help in the codebase to understand its usage
rg -n 'show_help' --type rust -B 2 -A 2Repository: Anti-Raid/khronos
Length of output: 5073
🏁 Script executed:
# Find and examine app.rs to see the initialization
fd -n 'app.rs' --type f | head -5Repository: Anti-Raid/khronos
Length of output: 291
🏁 Script executed:
# Search for where toggle_help_modal or Help key is handled
rg -n 'toggle_help_modal|KeyCode::F\(1\)|Help' --type rust -B 2 -A 2Repository: Anti-Raid/khronos
Length of output: 10144
🏁 Script executed:
# Also check tui_entrypoint.rs more thoroughly to see the full input handling
sed -n '70,150p' crates/cli/src/tui_entrypoint.rsRepository: Anti-Raid/khronos
Length of output: 4337
Remove the dead show_help code path and field.
The show_help field and the render logic at lines 60-64 are unused. The toggle method was explicitly removed in favor of toggle_help_modal() (see app.rs comment at line 232), and show_help is never set to true anywhere in the codebase. All help functionality is properly implemented through show_help_modal, which is toggled by F1 (and the /help command) and renders a proper HelpModal. Remove the field from the App struct, delete the render block at lines 60-64, and clean up the conditional guards (!app.show_help) in ui.rs and tui_entrypoint.rs.
🤖 Prompt for AI Agents
In crates/cli/src/tui/ui.rs around lines 60-64, remove the dead show_help render
path and the now-unused show_help boolean: delete the conditional block that
centers and renders Dashboard when app.show_help is true, remove any remaining
uses/guards of app.show_help in this file and in
crates/cli/src/tui/tui_entrypoint.rs, and also remove the show_help field from
the App struct (wherever it is declared). After removal, update any conditionals
that used !app.show_help to only use the current help-modal flag
(show_help_modal) and run a build to fix any remaining references or
unused-import warnings.
| --- HTTP Client plugin | ||
| HTTPClient: httpclientP.Plugin, | ||
There was a problem hiding this comment.
Fix hard tabs in HTTPClient field declaration.
Lines 522–523 use hard tabs instead of spaces, violating the markdown linting rule (MD010). Replace with spaces to match the indentation style of surrounding fields.
🔎 Proposed fix
- --- HTTP Client plugin
- HTTPClient: httpclientP.Plugin,
+ --- HTTP Client plugin
+ HTTPClient: httpclientP.Plugin,The field addition itself is correct and follows the established plugin pattern in TemplateContext.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| --- HTTP Client plugin | |
| HTTPClient: httpclientP.Plugin, | |
| --- HTTP Client plugin | |
| HTTPClient: httpclientP.Plugin, | |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
522-522: Hard tabs
Column: 1
(MD010, no-hard-tabs)
523-523: Hard tabs
Column: 1
(MD010, no-hard-tabs)
🤖 Prompt for AI Agents
In docs/primitives.md around lines 522 to 524, the HTTPClient field declaration
uses hard tabs for indentation which violates MD010; replace the hard tabs with
spaces so the line indentation matches surrounding fields (use the same number
of spaces used elsewhere for field alignment) and save the file.
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.