Enhanced TUI with Filtering and Context View#2
Conversation
- Added severity filtering (All, High, Medium, Low) using keys 1-4. - Added context view popup (Enter key) showing +/- 2 lines of code. - Updated status bar to show current filter mode. - Refactored finding management to support filtering while maintaining the ability to repair/ignore findings from the filtered list. Co-authored-by: 09Catho <169603149+09Catho@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
📝 WalkthroughWalkthroughThe TUI application gains context-aware navigation with file viewing capabilities, severity-based filtering (keys 1–4), and synchronized data structures that properly propagate fixes across all findings rather than just the visible list. Input handling expands to support toggling and exiting context views. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TUI as TUI Handler
participant FileSystem as File System
participant UI as UI Renderer
User->>TUI: Press Enter (finding selected)
TUI->>FileSystem: get_file_context(path, line_num)
FileSystem-->>TUI: Vec<(line_num, content)>
TUI->>TUI: Set app.context_lines
TUI->>TUI: Set app.show_context = true
TUI->>UI: Render with context popup
UI-->>User: Display file context with highlighted line
User->>TUI: Press Enter (context shown)
TUI->>TUI: Set app.show_context = false
TUI->>UI: Render findings list
UI-->>User: Hide context, show findings
sequenceDiagram
participant User
participant TUI as TUI Handler
participant App as App State
participant UI as UI Renderer
User->>TUI: Press key 1–4 (select filter)
TUI->>App: Set filter_mode (All/High/Medium/Low)
TUI->>App: Call update_visible_findings()
App->>App: Filter all_findings by severity
App->>App: Reset selection index
TUI->>UI: Render filtered findings
UI-->>User: Display findings matching severity level
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 1
🤖 Fix all issues with AI agents
In `@src/ui.rs`:
- Around line 110-131: get_file_context currently silently skips lines when
reader.lines() yields Err (e.g., invalid UTF-8); update the loop in
get_file_context to preserve line count by pushing a placeholder entry for
unreadable lines (e.g., return (current_line, "<unreadable line>".to_string())
or similar) instead of skipping, or alternatively log the error via your logger
from inside that branch; ensure the change touches the lines enumeration/if let
Ok(l) = line block so every current_line between start and end is appended
(either the real line or the placeholder/logged error).
🧹 Nitpick comments (3)
src/ui.rs (1)
88-107: Selection reset may surprise users navigating filtered lists.When the filter changes, the selection always resets to index 0. If a user was inspecting a specific finding and toggles filters, they lose their position. Consider preserving the selection by fingerprint if the currently selected finding remains visible after filtering.
💡 Optional enhancement to preserve selection
pub fn update_visible_findings(&mut self) { + // Capture current selection's fingerprint before filtering + let current_fingerprint = self.state.selected() + .and_then(|idx| self.findings.get(idx)) + .map(|f| f.fingerprint.clone()); + self.findings = self .all_findings .iter() .filter(|f| match self.filter_mode { FilterMode::All => true, FilterMode::High => f.severity == Severity::High, FilterMode::Medium => f.severity == Severity::Medium, FilterMode::Low => f.severity == Severity::Low, }) .cloned() .collect(); - // Reset selection if out of bounds if self.findings.is_empty() { self.state.select(None); + } else if let Some(fp) = current_fingerprint { + // Try to preserve selection + let new_idx = self.findings.iter().position(|f| f.fingerprint == fp); + self.state.select(Some(new_idx.unwrap_or(0))); } else { self.state.select(Some(0)); } }src/main.rs (2)
395-410: Consider consolidating filter key handling.The four filter key handlers follow an identical pattern. While functional, this could be simplified with a helper or match arm combination.
♻️ Optional consolidation
- KeyCode::Char('1') => { - app.filter_mode = FilterMode::All; - app.update_visible_findings(); - } - KeyCode::Char('2') => { - app.filter_mode = FilterMode::High; - app.update_visible_findings(); - } - KeyCode::Char('3') => { - app.filter_mode = FilterMode::Medium; - app.update_visible_findings(); - } - KeyCode::Char('4') => { - app.filter_mode = FilterMode::Low; - app.update_visible_findings(); - } + KeyCode::Char(c @ '1'..='4') => { + app.filter_mode = match c { + '1' => FilterMode::All, + '2' => FilterMode::High, + '3' => FilterMode::Medium, + _ => FilterMode::Low, + }; + app.update_visible_findings(); + }
448-470: Extract duplicated finding removal logic.The pattern of removing a finding from
all_findingsby fingerprint and refreshing visible findings is duplicated in both the repair (r) and ignore (g) handlers. Consider adding a helper method toApp.♻️ Extract to App method
Add to
Appinsrc/ui.rs:pub fn remove_finding(&mut self, fingerprint: &str) { if let Some(idx) = self.all_findings.iter().position(|x| x.fingerprint == fingerprint) { self.all_findings.remove(idx); } self.update_visible_findings(); }Then simplify both handlers in
src/main.rs:match fixer::fix_file(&f.file_path, vec![replacement]) { Ok(_) => { - let fingerprint = f.fingerprint.clone(); - // Remove from all_findings - if let Some(idx) = app - .all_findings - .iter() - .position(|x| x.fingerprint == fingerprint) - { - app.all_findings.remove(idx); - } - app.update_visible_findings(); + app.remove_finding(&f.fingerprint.clone()); app.clipboard_status = Some("Fixed!".to_string());Same pattern for the
ghandler.Also applies to: 484-497
| pub fn get_file_context(path: &str, line_num: usize) -> io::Result<Vec<(usize, String)>> { | ||
| let file = File::open(path)?; | ||
| let reader = BufReader::new(file); | ||
|
|
||
| // Context window: +/- 2 lines | ||
| let start = line_num.saturating_sub(2); | ||
| let end = line_num + 2; | ||
|
|
||
| let mut lines = Vec::new(); | ||
| for (i, line) in reader.lines().enumerate() { | ||
| let current_line = i + 1; | ||
| if current_line >= start && current_line <= end { | ||
| if let Ok(l) = line { | ||
| lines.push((current_line, l)); | ||
| } | ||
| } | ||
| if current_line > end { | ||
| break; | ||
| } | ||
| } | ||
| Ok(lines) | ||
| } |
There was a problem hiding this comment.
Line read errors are silently skipped.
If a line fails to read (e.g., invalid UTF-8), it's silently omitted from the context. This could result in fewer than 5 lines displayed with no indication of why. Consider logging or including a placeholder for unreadable lines.
💡 Optional: indicate unreadable lines
if current_line >= start && current_line <= end {
- if let Ok(l) = line {
- lines.push((current_line, l));
+ match line {
+ Ok(l) => lines.push((current_line, l)),
+ Err(_) => lines.push((current_line, "<unable to read line>".to_string())),
}
}🤖 Prompt for AI Agents
In `@src/ui.rs` around lines 110 - 131, get_file_context currently silently skips
lines when reader.lines() yields Err (e.g., invalid UTF-8); update the loop in
get_file_context to preserve line count by pushing a placeholder entry for
unreadable lines (e.g., return (current_line, "<unreadable line>".to_string())
or similar) instead of skipping, or alternatively log the error via your logger
from inside that branch; ensure the change touches the lines enumeration/if let
Ok(l) = line block so every current_line between start and end is appended
(either the real line or the placeholder/logged error).
1 participant
Implemented enhanced TUI features including severity filtering and source code context view. This allows users to better manage large numbers of findings and inspect secrets in context without leaving the tool.
PR created automatically by Jules for task 5784107876017081356 started by @09Catho
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.