review(p9-2): 회차 1 지적 반영
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) <noreply@anthropic.com>
This commit is contained in:
@@ -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<ListItem> = 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. `<heading_path joined by " / "> | 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<Line<'static>> {
|
||||
fn format_hit_lines(h: &SearchHit) -> Vec<Line<'static>> {
|
||||
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);
|
||||
|
||||
@@ -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();
|
||||
|
||||
Reference in New Issue
Block a user