From 09ea6c2435910e44461ce2d1879997dac60f7054 Mon Sep 17 00:00:00 2001 From: Onur Solmaz <2453968+osolmaz@users.noreply.github.com> Date: Fri, 10 Jul 2026 20:17:53 +0800 Subject: [PATCH 1/2] feat: add shift range commenting --- README.md | 3 +- scripts/e2e-tmux.sh | 22 ++++++++ src/app.rs | 41 ++++++++++++++ src/render.rs | 129 ++++++++++++++++++++++++++++++++++++++------ src/runner.rs | 96 +++++++++++++++++++++++++++++++-- src/terminal.rs | 20 ++++++- 6 files changed, 289 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index cd1df76..06f42cc 100644 --- a/README.md +++ b/README.md @@ -67,16 +67,17 @@ Mouse controls: - Drag source lines and release to comment on the range. - Click a comment to edit it. - Use the wheel to scroll the document or active editor. +- Saved ranges keep a green rail beside every source line they cover. Keyboard controls: | Key | Action | | --- | --- | | `j` / `k`, arrows | Move between source lines | +| `Shift-Up` / `Shift-Down` | Select a range; release Shift to write the comment | | `v` | Start or cancel range selection | | `Enter` | Comment on the cursor/range, or save an active comment | | `Ctrl-O` | Insert a newline in a comment | -| `Ctrl-A` / `Ctrl-E` | Move to the beginning/end of the editor line | | `Esc` | Cancel selection or editing | | `e` / `d` | Edit or delete a comment on the cursor line | | `[` / `]` | Jump to the previous/next comment | diff --git a/scripts/e2e-tmux.sh b/scripts/e2e-tmux.sh index 2a9aade..1320642 100755 --- a/scripts/e2e-tmux.sh +++ b/scripts/e2e-tmux.sh @@ -68,6 +68,8 @@ wait_for_pane "Comment on lines 1" tmux send-keys -t "$session" -l "human comment here ..." tmux send-keys -t "$session" Enter wait_for_pane "Comment saved" +wait_for_pane "┃ quoted part line 1" +wait_for_pane "┃ quoted part line 2" finish_tui printf '> quoted part line 1\n> quoted part line 2\n\nhuman comment here ...\n' >"$tmpdir/expected.md" @@ -98,3 +100,23 @@ assert review["comments"] == [ } ] PY + +start_tui "$tmpdir/shift-output.md" "--comments '$tmpdir/shift-review.json' --no-mouse" + +# Select all three source lines with Shift-Down, then inject a left-Shift release +# using the Kitty keyboard protocol requested by the application. +tmux send-keys -t "$session" Escape '[1;2B' +tmux send-keys -t "$session" Escape '[1;2B' +tmux send-keys -t "$session" Escape '[57441;2:3u' +wait_for_pane "Comment on lines 1" + +tmux send-keys -t "$session" -l "keyboard range" +tmux send-keys -t "$session" Enter +wait_for_pane "Comment saved" +wait_for_pane "┃ quoted part line 1" +wait_for_pane "┃ quoted part line 2" +wait_for_pane "┃ unquoted line" +finish_tui + +printf '> quoted part line 1\n> quoted part line 2\n> unquoted line\n\nkeyboard range\n' >"$tmpdir/expected-shift.md" +diff -u "$tmpdir/expected-shift.md" "$tmpdir/shift-output.md" diff --git a/src/app.rs b/src/app.rs index 85bcf69..36652e7 100644 --- a/src/app.rs +++ b/src/app.rs @@ -73,6 +73,7 @@ pub struct App { pub should_quit: bool, pub follow_cursor: bool, pub mouse_drag_anchor: Option, + pub keyboard_shift_anchor: Option, pub status: Option, pub dirty: bool, } @@ -92,6 +93,7 @@ impl App { should_quit: false, follow_cursor: true, mouse_drag_anchor: None, + keyboard_shift_anchor: None, status: None, dirty: false, } @@ -122,6 +124,7 @@ impl App { pub fn begin_selection(&mut self, line: usize) { self.move_to_line(line); + self.keyboard_shift_anchor = None; self.selection = Some(Selection { anchor: self.cursor_line, current: self.cursor_line, @@ -142,9 +145,28 @@ impl App { pub fn cancel_selection(&mut self) { self.selection = None; + self.keyboard_shift_anchor = None; self.status = None; } + pub fn extend_shift_selection(&mut self, delta: isize) { + if self.keyboard_shift_anchor.is_none() { + if self.selection.is_none() { + self.begin_selection(self.cursor_line); + } + self.keyboard_shift_anchor = self.selection.map(|selection| selection.anchor); + } + self.move_cursor(delta); + } + + pub fn finish_shift_selection(&mut self) -> bool { + if self.keyboard_shift_anchor.take().is_none() || self.selection.is_none() { + return false; + } + self.open_selected_editor(); + true + } + pub fn open_selected_editor(&mut self) { let selection = self.selection.unwrap_or(Selection { anchor: self.cursor_line, @@ -152,6 +174,7 @@ impl App { }); let (start_line, end_line) = selection.normalized(); self.cursor_line = end_line; + self.keyboard_shift_anchor = None; self.editor = Some(CommentEditor::new(None, start_line, end_line, "")); self.follow_cursor = true; self.status = None; @@ -168,6 +191,7 @@ impl App { return false; }; self.cursor_line = comment.end_line; + self.keyboard_shift_anchor = None; self.active_comment_id = Some(comment.id); self.selection = Some(Selection { anchor: comment.start_line, @@ -210,6 +234,7 @@ impl App { }); self.editor = None; self.selection = None; + self.keyboard_shift_anchor = None; self.status = Some("Comment saved".into()); self.active_comment_id = Some(id); self.dirty = true; @@ -219,6 +244,7 @@ impl App { pub fn cancel_editor(&mut self) { self.editor = None; self.selection = None; + self.keyboard_shift_anchor = None; self.status = Some("Edit cancelled".into()); } @@ -389,6 +415,21 @@ mod tests { assert!(app.status.is_none()); } + #[test] + fn shift_selection_extends_and_opens_the_editor_on_finish() { + let mut app = app(); + app.extend_shift_selection(1); + app.extend_shift_selection(1); + assert_eq!(app.selection.unwrap().normalized(), (1, 3)); + assert_eq!(app.keyboard_shift_anchor, Some(1)); + + assert!(app.finish_shift_selection()); + let editor = app.editor.as_ref().unwrap(); + assert_eq!((editor.start_line, editor.end_line), (1, 3)); + assert!(app.keyboard_shift_anchor.is_none()); + assert!(!app.finish_shift_selection()); + } + #[test] fn missing_comment_actions_report_without_mutating() { let mut app = app(); diff --git a/src/render.rs b/src/render.rs index 32a66c7..6c6fa46 100644 --- a/src/render.rs +++ b/src/render.rs @@ -21,8 +21,16 @@ const TAB_STOP: usize = 4; #[derive(Debug, Clone, PartialEq, Eq)] enum DocumentRow { - Source(usize), - Comment { id: u64, text: String, first: bool }, + Source { + line_number: usize, + commented: bool, + active: bool, + }, + Comment { + id: u64, + text: String, + first: bool, + }, Editor, } @@ -98,8 +106,20 @@ fn render_document(frame: &mut Frame<'_>, app: &mut App, area: Rect) -> Vec { - render_source_row(frame, app, rect, *line_number, line_digits); + DocumentRow::Source { + line_number, + commented, + active, + } => { + render_source_row( + frame, + app, + rect, + *line_number, + line_digits, + *commented, + *active, + ); hits.push(HitArea::new(rect, HitTarget::SourceLine(*line_number))); } DocumentRow::Comment { id, text, first } => { @@ -108,6 +128,7 @@ fn render_document(frame: &mut Frame<'_>, app: &mut App, area: Rect) -> Vec Vec { let mut rows = Vec::new(); let editing_id = app.editor.as_ref().and_then(|editor| editor.comment_id); let editor_end = app.editor.as_ref().map(|editor| editor.end_line); + let active_range = app.active_comment_id.and_then(|id| { + app.review + .comments + .iter() + .find(|comment| comment.id == id) + .map(|comment| (comment.start_line, comment.end_line)) + }); let mut comments_by_end = BTreeMap::>::new(); + let mut range_events = BTreeMap::::new(); + for comment in &app.review.comments { + range_events.entry(comment.start_line).or_default().0 += 1; + if comment.end_line < app.source.line_count() { + range_events.entry(comment.end_line + 1).or_default().1 += 1; + } + } for comment in app .review .comments @@ -153,8 +188,23 @@ fn document_rows(app: &App) -> Vec { .push(comment); } + let mut events = range_events.into_iter().peekable(); + let mut comments_covering_line = 0usize; for line_number in 1..=app.source.line_count() { - rows.push(DocumentRow::Source(line_number)); + if events + .peek() + .is_some_and(|(event_line, _)| *event_line == line_number) + { + let (_, (starts, ends)) = events.next().unwrap_or_default(); + comments_covering_line = comments_covering_line.saturating_sub(ends); + comments_covering_line = comments_covering_line.saturating_add(starts); + } + rows.push(DocumentRow::Source { + line_number, + commented: comments_covering_line > 0, + active: active_range + .is_some_and(|(start, end)| start <= line_number && line_number <= end), + }); if let Some(comments) = comments_by_end.get(&line_number) { for comment in comments { for (index, body_line) in comment.body.lines().enumerate() { @@ -182,15 +232,24 @@ fn update_scroll(app: &mut App, rows: &[DocumentRow], viewport_height: usize) { let source_row = rows .iter() - .position(|row| matches!(row, DocumentRow::Source(line) if *line == app.cursor_line)) + .position(|row| { + matches!( + row, + DocumentRow::Source { line_number, .. } if *line_number == app.cursor_line + ) + }) .unwrap_or(0); let target_row = if app.editor.is_some() { rows.iter() .enumerate() .skip(source_row) - .take_while( - |(_, row)| !matches!(row, DocumentRow::Source(line) if *line > app.cursor_line), - ) + .take_while(|(_, row)| { + !matches!( + row, + DocumentRow::Source { line_number, .. } + if *line_number > app.cursor_line + ) + }) .map(|(index, _)| index) .last() .unwrap_or(source_row) @@ -213,13 +272,15 @@ fn render_source_row( area: Rect, line_number: usize, line_digits: usize, + commented: bool, + active: bool, ) { let selected = app .selection .is_some_and(|selection| selection.contains(line_number)); let cursor = app.cursor_line == line_number && app.editor.is_none(); let marker = if cursor { "▶" } else { " " }; - let number = format!("{marker}{line_number:>line_digits$} │ "); + let number = format!("{marker}{line_number:>line_digits$} "); let text = horizontally_scrolled( app.source.line(line_number).unwrap_or_default(), app.horizontal_scroll, @@ -234,15 +295,41 @@ fn render_source_row( } else { Style::default().fg(Color::DarkGray) }; + let rail = if commented { "┃" } else { "│" }; + let rail_style = if active { + Style::default() + .fg(Color::Black) + .bg(Color::Green) + .add_modifier(Modifier::BOLD) + } else if commented { + Style::default() + .fg(Color::Green) + .add_modifier(Modifier::BOLD) + } else { + number_style + }; let line = Line::from(vec![ Span::styled(number, number_style), + Span::styled(rail, rail_style), + Span::styled(" ", style), Span::styled(text, style), ]); frame.render_widget(Paragraph::new(line).style(style), area); } -fn render_comment_row(frame: &mut Frame<'_>, area: Rect, text: &str, first: bool, active: bool) { - let prefix = if first { " └─ " } else { " " }; +fn render_comment_row( + frame: &mut Frame<'_>, + area: Rect, + text: &str, + first: bool, + line_digits: usize, + active: bool, +) { + let prefix = if first { + format!("{}└─ ", " ".repeat(line_digits + 2)) + } else { + " ".repeat(line_digits + 5) + }; let style = if active { Style::default().fg(Color::Black).bg(Color::Green) } else { @@ -295,16 +382,21 @@ fn expand_tabs(text: &str) -> String { fn render_footer(frame: &mut Frame<'_>, app: &App, area: Rect) { let help = if app.editor.is_some() { - " Enter save · Ctrl-O newline · Esc cancel · Ctrl-A/E line start/end " + " Enter save · Ctrl-O newline · Esc cancel " } else if let Some(selection) = app.selection { let (start, end) = selection.normalized(); + let finish = if app.keyboard_shift_anchor.is_some() { + "release Shift or Enter" + } else { + "Enter" + }; return render_footer_text( frame, area, - &format!(" Lines {start}–{end} selected · Enter comment · Esc cancel "), + &format!(" Lines {start}–{end} selected · {finish} to comment · Esc cancel "), ); } else { - " drag select · Enter comment · e edit · d delete · [/] comments · q output & quit " + " drag or Shift-↑/↓ select · Enter comment · e edit · d delete · [/] comments · q quit " }; let text = app.status.as_deref().unwrap_or(help); render_footer_text(frame, area, text); @@ -388,7 +480,11 @@ mod tests { terminal .draw(|frame| hits = render_app(frame, &mut app)) .unwrap(); - assert!(terminal.backend().to_string().contains("└─ first line")); + let rendered = terminal.backend().to_string(); + assert!(rendered.contains("1 ┃ one")); + assert!(rendered.contains("2 ┃ two")); + assert!(rendered.contains("3 │ three")); + assert!(rendered.contains("└─ first line")); assert!(hits.iter().any(|hit| hit.target == HitTarget::Comment(1))); app.begin_edit(1); @@ -398,6 +494,7 @@ mod tests { let rendered = terminal.backend().to_string(); assert!(rendered.contains("Comment on lines 1–2")); assert!(rendered.contains("Ctrl-O newline")); + assert!(!rendered.contains("Ctrl-A")); app.cancel_editor(); app.review.comments[0].body = "\tcode".into(); diff --git a/src/runner.rs b/src/runner.rs index ea51e43..744f531 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -6,8 +6,8 @@ use std::{ use anyhow::{bail, Context}; use crossterm::event::{ - self, Event, KeyCode, KeyEvent, KeyEventKind, KeyModifiers, MouseButton, MouseEvent, - MouseEventKind, + self, Event, KeyCode, KeyEvent, KeyEventKind, KeyModifiers, ModifierKeyCode, MouseButton, + MouseEvent, MouseEventKind, }; use ratatui_textarea::Scrolling; use unicode_normalization::UnicodeNormalization; @@ -238,17 +238,42 @@ fn write_output(path: Option<&Path>, output: &str) -> anyhow::Result<()> { pub fn handle_event(event: Event, app: &mut App, hit_areas: &[HitArea]) { match event { - Event::Key(key) if key.kind != KeyEventKind::Release => handle_key(key, app), + Event::Key(key) => handle_key_event(key, app), Event::Mouse(mouse) => handle_mouse(mouse, app, hit_areas), Event::Paste(text) => { if let Some(editor) = app.editor.as_mut() { editor.textarea.insert_str(text); } } - Event::FocusGained | Event::FocusLost | Event::Resize(_, _) | Event::Key(_) => {} + Event::FocusGained | Event::FocusLost | Event::Resize(_, _) => {} } } +fn handle_key_event(key: KeyEvent, app: &mut App) { + if key.kind == KeyEventKind::Release { + if is_shift_modifier(key.code) { + app.finish_shift_selection(); + } + return; + } + + if app.editor.is_none() + && app.keyboard_shift_anchor.is_some() + && !key.modifiers.contains(KeyModifiers::SHIFT) + && !is_shift_modifier(key.code) + { + app.finish_shift_selection(); + } + handle_key(key, app); +} + +fn is_shift_modifier(code: KeyCode) -> bool { + matches!( + code, + KeyCode::Modifier(ModifierKeyCode::LeftShift | ModifierKeyCode::RightShift) + ) +} + pub fn handle_key(key: KeyEvent, app: &mut App) { if app.editor.is_some() { handle_editor_key(key, app); @@ -289,6 +314,12 @@ fn handle_browse_key(key: KeyEvent, app: &mut App) { KeyCode::Char('c') if key.modifiers.contains(KeyModifiers::CONTROL) => { app.should_quit = true; } + KeyCode::Down if key.modifiers.contains(KeyModifiers::SHIFT) => { + app.extend_shift_selection(1); + } + KeyCode::Up if key.modifiers.contains(KeyModifiers::SHIFT) => { + app.extend_shift_selection(-1); + } KeyCode::Down | KeyCode::Char('j') => app.move_cursor(1), KeyCode::Up | KeyCode::Char('k') => app.move_cursor(-1), KeyCode::PageDown => app.move_cursor(10), @@ -423,6 +454,63 @@ mod tests { assert_eq!(app.review.comments[0].body, "x"); } + #[test] + fn shift_arrows_open_the_selected_range_when_shift_is_released() { + let mut app = app(); + handle_key(KeyEvent::new(KeyCode::Down, KeyModifiers::SHIFT), &mut app); + handle_key(KeyEvent::new(KeyCode::Down, KeyModifiers::SHIFT), &mut app); + assert_eq!(app.selection.unwrap().normalized(), (1, 3)); + + handle_event( + Event::Key(KeyEvent::new_with_kind( + KeyCode::Modifier(ModifierKeyCode::LeftShift), + KeyModifiers::SHIFT, + KeyEventKind::Release, + )), + &mut app, + &[], + ); + + let editor = app.editor.as_ref().unwrap(); + assert_eq!((editor.start_line, editor.end_line), (1, 3)); + } + + #[test] + fn shift_up_selects_a_reverse_range() { + let mut app = app(); + app.move_to_line(3); + handle_key(KeyEvent::new(KeyCode::Up, KeyModifiers::SHIFT), &mut app); + handle_key(KeyEvent::new(KeyCode::Up, KeyModifiers::SHIFT), &mut app); + handle_event( + Event::Key(KeyEvent::new_with_kind( + KeyCode::Modifier(ModifierKeyCode::RightShift), + KeyModifiers::SHIFT, + KeyEventKind::Release, + )), + &mut app, + &[], + ); + + let editor = app.editor.as_ref().unwrap(); + assert_eq!((editor.start_line, editor.end_line), (1, 3)); + } + + #[test] + fn first_unshifted_key_finalizes_selection_on_legacy_terminals() { + let mut app = app(); + handle_key(KeyEvent::new(KeyCode::Down, KeyModifiers::SHIFT), &mut app); + + handle_event( + Event::Key(KeyEvent::new(KeyCode::Char('x'), KeyModifiers::NONE)), + &mut app, + &[], + ); + + let editor = app.editor.as_ref().unwrap(); + assert_eq!((editor.start_line, editor.end_line), (1, 2)); + assert_eq!(editor.body(), "x"); + } + #[test] fn modified_d_does_not_delete_a_comment() { let mut app = app(); diff --git a/src/terminal.rs b/src/terminal.rs index 4654b1a..7f20f13 100644 --- a/src/terminal.rs +++ b/src/terminal.rs @@ -8,7 +8,10 @@ use std::{ }; use crossterm::{ - event::{DisableBracketedPaste, DisableMouseCapture, EnableBracketedPaste, EnableMouseCapture}, + event::{ + DisableBracketedPaste, DisableMouseCapture, EnableBracketedPaste, EnableMouseCapture, + KeyboardEnhancementFlags, PopKeyboardEnhancementFlags, PushKeyboardEnhancementFlags, + }, execute, terminal::{disable_raw_mode, enable_raw_mode, EnterAlternateScreen, LeaveAlternateScreen}, }; @@ -21,6 +24,7 @@ static RAW_ENABLED: AtomicBool = AtomicBool::new(false); static SCREEN_ENABLED: AtomicBool = AtomicBool::new(false); static MOUSE_ENABLED: AtomicBool = AtomicBool::new(false); static PASTE_ENABLED: AtomicBool = AtomicBool::new(false); +static KEYBOARD_ENHANCED: AtomicBool = AtomicBool::new(false); #[derive(Debug)] pub struct TerminalGuard; @@ -41,6 +45,15 @@ impl TerminalGuard { SCREEN_ENABLED.store(true, Ordering::SeqCst); execute!(tty, EnableBracketedPaste)?; PASTE_ENABLED.store(true, Ordering::SeqCst); + execute!( + tty, + PushKeyboardEnhancementFlags( + KeyboardEnhancementFlags::DISAMBIGUATE_ESCAPE_CODES + | KeyboardEnhancementFlags::REPORT_EVENT_TYPES + | KeyboardEnhancementFlags::REPORT_ALL_KEYS_AS_ESCAPE_CODES + ) + )?; + KEYBOARD_ENHANCED.store(true, Ordering::SeqCst); if mouse_enabled { execute!(tty, EnableMouseCapture)?; MOUSE_ENABLED.store(true, Ordering::SeqCst); @@ -77,6 +90,11 @@ fn restore_terminal() { let _ = execute!(tty, DisableMouseCapture); } } + if KEYBOARD_ENHANCED.swap(false, Ordering::SeqCst) { + if let Some(tty) = &mut tty { + let _ = execute!(tty, PopKeyboardEnhancementFlags); + } + } if PASTE_ENABLED.swap(false, Ordering::SeqCst) { if let Some(tty) = &mut tty { let _ = execute!(tty, DisableBracketedPaste); From 5998886731401234722423cab22a03d6d8634be8 Mon Sep 17 00:00:00 2001 From: Onur Solmaz <2453968+osolmaz@users.noreply.github.com> Date: Fri, 10 Jul 2026 20:40:53 +0800 Subject: [PATCH 2/2] fix: finalize legacy shift selection cleanly --- src/render.rs | 4 ++-- src/runner.rs | 22 +++++++++++++++++++++- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/render.rs b/src/render.rs index 6c6fa46..3ee93e0 100644 --- a/src/render.rs +++ b/src/render.rs @@ -382,7 +382,7 @@ fn expand_tabs(text: &str) -> String { fn render_footer(frame: &mut Frame<'_>, app: &App, area: Rect) { let help = if app.editor.is_some() { - " Enter save · Ctrl-O newline · Esc cancel " + " Enter save · Esc cancel " } else if let Some(selection) = app.selection { let (start, end) = selection.normalized(); let finish = if app.keyboard_shift_anchor.is_some() { @@ -493,7 +493,7 @@ mod tests { .unwrap(); let rendered = terminal.backend().to_string(); assert!(rendered.contains("Comment on lines 1–2")); - assert!(rendered.contains("Ctrl-O newline")); + assert!(!rendered.contains("Ctrl-O")); assert!(!rendered.contains("Ctrl-A")); app.cancel_editor(); diff --git a/src/runner.rs b/src/runner.rs index 744f531..8e79803 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -262,7 +262,10 @@ fn handle_key_event(key: KeyEvent, app: &mut App) { && !key.modifiers.contains(KeyModifiers::SHIFT) && !is_shift_modifier(key.code) { - app.finish_shift_selection(); + let finalized = app.finish_shift_selection(); + if finalized && key.code == KeyCode::Enter { + return; + } } handle_key(key, app); } @@ -511,6 +514,23 @@ mod tests { assert_eq!(editor.body(), "x"); } + #[test] + fn enter_finalizes_a_legacy_shift_selection_without_submitting_it() { + let mut app = app(); + handle_key(KeyEvent::new(KeyCode::Down, KeyModifiers::SHIFT), &mut app); + + handle_event( + Event::Key(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)), + &mut app, + &[], + ); + + let editor = app.editor.as_ref().unwrap(); + assert_eq!((editor.start_line, editor.end_line), (1, 2)); + assert_eq!(editor.body(), ""); + assert!(app.status.is_none()); + } + #[test] fn modified_d_does_not_delete_a_comment() { let mut app = app();