From 0732b3ffbec419da8a892ef2fade4acf664836d5 Mon Sep 17 00:00:00 2001 From: altair823 Date: Sat, 2 May 2026 14:42:49 +0000 Subject: [PATCH] =?UTF-8?q?review(p9-2):=20=ED=9A=8C=EC=B0=A8=201=20?= =?UTF-8?q?=EC=A7=80=EC=A0=81=20=EB=B0=98=EC=98=81?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. **Citation::Page 분기 fix** — `args.push(format!(\"# page {page}\"))` 가 vim/code/cursor 에 \"두 번째 파일\" 로 해석돼 의도 외 동작 (split / new buffer). 마지막 push 제거, path 만 열고 `tracing::debug!` 한 줄. PDF 페이지 jump 는 사용자 PDF reader 책임 — `KEBAB_EDITOR_JUMP_FORMAT` env hook 은 P+ enhancement. 2. **j/k/g 의 SHIFT modifier 차단** — `is_typing_mod` 가 SHIFT 를 typing 으로 취급하던 부분이 J/K/G 를 selection 키로 흡수해 \"JSON\" / \"PostgreSQL\" / \"Go\" 같은 대문자 검색어 깨짐. arrow 키 (Down/Up) 는 modifier 무관 유지, 문자 키 (j/k/g) 는 `KeyModifiers::NONE` 만. SHIFT-J / SHIFT-G 회귀 테스트 2건 추가. 3. **`format_hit_lines` 의 unused `_width` 인자 제거** — ratatui 자동 truncate 신뢰 (Library 의 한국어 column 정렬은 별도 path). Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/kebab-tui/src/search.rs | 50 +++++++++++++++++++++++--------- crates/kebab-tui/tests/search.rs | 40 +++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 13 deletions(-) diff --git a/crates/kebab-tui/src/search.rs b/crates/kebab-tui/src/search.rs index 238c177..a78fd68 100644 --- a/crates/kebab-tui/src/search.rs +++ b/crates/kebab-tui/src/search.rs @@ -98,11 +98,10 @@ fn render_result_list(f: &mut Frame, area: Rect, s: &SearchState) { return; } - let title_w = (area.width as usize).saturating_sub(20).max(20); let items: Vec = s .hits .iter() - .map(|h| ListItem::new(format_hit_lines(h, title_w))) + .map(|h| ListItem::new(format_hit_lines(h))) .collect(); let list = List::new(items) .block(block) @@ -118,7 +117,7 @@ fn render_result_list(f: &mut Frame, area: Rect, s: &SearchState) { /// 2. ` | section_label?` /// 3. snippet line 1 /// 4. snippet line 2 (or trailing blank for layout symmetry) -fn format_hit_lines(h: &SearchHit, _width: usize) -> Vec> { +fn format_hit_lines(h: &SearchHit) -> Vec> { let header = format!( "{}. {:.4} {}", h.rank, @@ -182,9 +181,12 @@ pub fn handle_key_search(state: &mut App, key: KeyEvent) -> KeyOutcome { // workspace_root after dropping the `&mut state.search` borrow. // Handle it as a pre-pass so the rest of the function can use // `state.search.as_mut()` without scope juggling. + // `g` only fires the editor jump on plain (no-modifier) press — + // SHIFT-G in vim land is "go to bottom" (not implemented here), + // and CTRL/ALT chords stay reserved. if matches!( (key.code, key.modifiers), - (KeyCode::Char('g'), m) if !is_typing_mod(m) + (KeyCode::Char('g'), KeyModifiers::NONE) ) { let (citation, has_hits) = { let s = state.search.as_ref().unwrap(); @@ -226,13 +228,27 @@ pub fn handle_key_search(state: &mut App, key: KeyEvent) -> KeyOutcome { KeyOutcome::Refresh } } - (KeyCode::Char('j'), m) | (KeyCode::Down, m) if !is_typing_mod(m) => { + // `j` / `k` only fire as selection movers when *no* modifier is + // held. SHIFT-bearing keypresses (`J`, `K`) are typed input — + // letting them through here would corrupt every \"JSON\" / + // \"PostgreSQL\" search query. Down / Up arrows still accept + // any modifier (no typing collision). + (KeyCode::Char('j'), KeyModifiers::NONE) => { move_selection(s, 1); - // Preview will refresh on next idle tick. s.preview = None; KeyOutcome::Continue } - (KeyCode::Char('k'), m) | (KeyCode::Up, m) if !is_typing_mod(m) => { + (KeyCode::Down, m) if !is_typing_mod(m) => { + move_selection(s, 1); + s.preview = None; + KeyOutcome::Continue + } + (KeyCode::Char('k'), KeyModifiers::NONE) => { + move_selection(s, -1); + s.preview = None; + KeyOutcome::Continue + } + (KeyCode::Up, m) if !is_typing_mod(m) => { move_selection(s, -1); s.preview = None; KeyOutcome::Continue @@ -315,13 +331,21 @@ pub fn build_jump_command( } } Citation::Page { page, .. } => { - // No standard editor jump for PDFs. Emit a `+1` hint so - // the user lands at the top, plus the page number in a - // following arg most editors will treat as a no-op - // search target. Best-effort. - args.push("+1".to_string()); + // No standard editor jump for PDFs across vim / VS Code / + // emacs. Earlier versions of this branch tried to push a + // `# page N` string as a final arg, but every common + // editor treats it as a *second file to open* — opening + // a stray buffer or splitting the window. Path-only is + // the honest best-effort: the user's PDF reader (or the + // editor's PDF plugin) handles in-document navigation. + // A `KEBAB_EDITOR_JUMP_FORMAT="pdf=evince -p {page} {path}"` + // env hook stays a P+ enhancement (per spec § Risks). + tracing::debug!( + target: "kebab-tui", + page, + "PDF citation — opening file only; editor page-jump unsupported" + ); args.push(path_str); - args.push(format!("# page {page}")); } _ => { args.push(path_str); diff --git a/crates/kebab-tui/tests/search.rs b/crates/kebab-tui/tests/search.rs index 0fe2626..a24c808 100644 --- a/crates/kebab-tui/tests/search.rs +++ b/crates/kebab-tui/tests/search.rs @@ -247,6 +247,46 @@ fn empty_state_renders_without_panic() { .unwrap(); } +#[test] +fn shift_j_stays_in_input_does_not_move_selection() { + // R1 fix: SHIFT-J / SHIFT-K must reach the typing branch so + // queries like \"JSON\" / \"PostgreSQL\" don't get \"J\" eaten as + // a selection move. + let mut app = fresh_app(); + { + let s = app.search.as_mut().unwrap(); + s.hits = vec![ + make_hit(1, "a.md", "snip\nl2", line_citation("a.md", 1)), + make_hit(2, "b.md", "snip\nl2", line_citation("b.md", 1)), + ]; + s.selected_hit = 0; + } + handle_key_search( + &mut app, + KeyEvent::new(KeyCode::Char('J'), KeyModifiers::SHIFT), + ); + let s = app.search.as_ref().unwrap(); + assert_eq!(s.selected_hit, 0, "selection must NOT move on SHIFT-J"); + assert_eq!(s.input, "J", "SHIFT-J must reach the input buffer"); +} + +#[test] +fn shift_g_does_not_trigger_editor_jump() { + // R1 fix: capital G must not invoke jump_to_citation. Keep it + // as plain typing so \"Go\" / \"Greetings\" search queries work. + let mut app = fresh_app(); + { + let s = app.search.as_mut().unwrap(); + s.hits = vec![make_hit(1, "a.md", "snip\nl2", line_citation("a.md", 1))]; + } + let outcome = handle_key_search( + &mut app, + KeyEvent::new(KeyCode::Char('G'), KeyModifiers::SHIFT), + ); + assert_eq!(outcome, KeyOutcome::Continue); + assert_eq!(app.search.as_ref().unwrap().input, "G"); +} + #[test] fn no_search_state_returns_to_library() { let mut config = Config::defaults();