diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 00000000..5173424d --- /dev/null +++ b/.gitattributes @@ -0,0 +1,7 @@ +# The viewer's built bundle is committed so `cargo install` needs no Node +# toolchain (see src/web/viewer/assets.rs). Vite names every chunk by content +# hash, so any change under viewer-ui/src rewrites these files wholesale and +# buries the real diff. Collapse them: `-diff` prints one line instead of the +# bundle, `linguist-generated` folds them in GitHub reviews. Rebuild with +# `npm --prefix viewer-ui run build` — never hand-edit. +viewer-ui/dist/** -diff linguist-generated=true diff --git a/Cargo.toml b/Cargo.toml index fe126263..9bff4ee6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,6 +27,7 @@ exclude = [ "release.toml", "CLAUDE.md", "docs/code-review-2026-05-08.md", + "docs/repo-picker-plan.md", "docs/status-short-plan.md", "docs/web-viewer-plan.md", ] diff --git a/README.md b/README.md index 2535b16d..13b89eb5 100644 --- a/README.md +++ b/README.md @@ -123,7 +123,21 @@ The diff for a selected file shows the combined working-tree-with-index changes. **Tree view** (` b`) — a read-only directory tree of the whole working tree on the left, with the selected file's raw contents on the right. Unlike the status view (which lists only changed files), the tree lets you browse and read *any* file next to the diff without leaving nightcrow. `j`/`k` move the cursor, `→`/`Enter` expand a directory (read lazily, one level at a time), `←` collapses it or steps up to the parent, and selecting a file previews it. Press `/` while the tree is focused for a recursive filename search across the whole tree — type to filter, `Enter` reveals the selected match in place (expanding its ancestor directories), `Esc` cancels. Focus the file preview with ` 2`, then press `/` to search within the file contents — `n`/`N` jump to the next/previous match, `Esc` clears the search. `.gitignore`-matched paths (e.g. `target/`, `node_modules/`) are hidden by default — toggle with `[tree] respect_gitignore`. Expanded directories are watched for filesystem changes, so files and folders created, moved, or deleted by another process (an editor, `git`, an LLM CLI) appear without leaving the view; set `[tree] live_watch = false` to refresh only on entry instead. The tree never writes, renames, or deletes anything. Expansion state and the selected path persist across sessions. -**Notice row** — a one-row strip just above the hint bar shows the repo path (home-relative, e.g. `~/projects/myapp`), the current branch, and ahead/behind counts (`↑N ↓M`) when the branch tracks an upstream. When something fails — a git snapshot, a diff load, a terminal pane, or a repo path you typed that doesn't exist — the message takes over this row in red until the problem is resolved or you act on the app again. A rejected repo path therefore appears directly above the input you're correcting. +**Notice row** — a one-row strip just above the hint bar shows the repo path (home-relative, e.g. `~/projects/myapp`), the current branch, and ahead/behind counts (`↑N ↓M`) when the branch tracks an upstream. When something fails — a git snapshot, a diff load, a terminal pane, or a repo path you typed that doesn't exist — the message takes over this row in red until the problem is resolved or you act on the app again. A rejected repo path therefore appears directly above the input you're correcting. The repo dialog's completion candidates share this row (dimmed, and a notice outranks them), so a list too long for one line ends in `+N more`. + +**Path completion in the repo dialog** — `Tab` completes the directory you're typing, so you don't have to know the path by heart. One press extends as far as the names allow; when there's nothing left to extend it lists what's there instead. On a trailing `/` the first press shows that directory's contents, and a unique match gains a trailing `/` so you can keep pressing `Tab` to descend. Only directories are offered (a file can't be a repo), dotted directories stay hidden until you type a leading `.`, and a name that differs only in case is matched and corrected for you. The dialog is a path field, not a shell — `~`, `..` and paths relative to your working directory all work, but `cd`, `$VAR`, and globs don't, and `Enter` always means "open this path". + +**Browsing for a repo** — when you don't know the path, press `↓` in the repo dialog to browse instead of typing. (A second `Tab`, once the candidate list is up, opens the same browser: at that point the flat list has told you all it can.) The browser fills the body of the screen, rooted at whatever directory the field currently names, and the field stays visible below it with the keys spelled out. + +| Key | Action | +|-----|--------| +| `↓` / `j`, `↑` / `k` | Move the cursor | +| `→` | Expand the selected directory (read lazily, one level at a time) | +| `←` | Collapse it, or step out — to the parent row, or one level *above the root* when you're already at the top, so a sibling checkout is one press away | +| `Enter` | Take the selected path into the field and return to it — this does **not** open the repo. Press `Enter` again in the field for that, or keep refining the path with `Tab` first | +| `Esc` | Leave the browser, keeping the text it started from. A second `Esc` cancels the dialog | + +Directories only, hidden ones excluded, and nothing is ever written. Note that `Enter` means *select* here but *open* in the field — the browser's job is to fill the field, so `→` alone expands (unlike the file-tree view, where `Enter` expands too). Paths keep your own notation: browsing out of `~/coding` gives you back `~/coding/…`, not an absolute path. Mouse selection isn't supported; the browser is keyboard-only. ## Keyboard shortcuts @@ -162,7 +176,7 @@ visible from the terminal pane. | ` l` | Toggle between status view and commit log view | | ` b` | Toggle the read-only file-tree view (returns to status view) | | ` f` | Fullscreen the focused pane. For the terminal it cycles `off → grid (all panes) → zoom (active pane only) → off`; with a single pane it toggles straight off/on. File list and diff viewer toggle off/on | -| ` o` | Open a repo in a **project tab** (prefilled with the active project's path — type to replace it, or press `→`/`End` first to extend it). A leading `~` expands to your home directory. If another tab already has that repo open, nightcrow focuses that tab instead of running two copies against one worktree | +| ` o` | Open a repo in a **project tab** (prefilled with the active project's path — type to replace it, or press `→`/`End` first to extend it). `Tab` completes the path against your filesystem and `↓` opens a directory browser (see below). A leading `~` expands to your home directory. If another tab already has that repo open, nightcrow focuses that tab instead of running two copies against one worktree | | ` x` | Close the active project tab. Closing the last one leaves nightcrow with no project open, which is a normal state | | ` p` | Cycle accent color (yellow → cyan → green → magenta → blue) | | ` r` | Force a full redraw (clears stray glyphs left by terminal programs) | @@ -209,7 +223,10 @@ through to the terminal program. | `PgUp` / `PgDn` | Scroll 20 lines | | `←` / `→` | Horizontal scroll (4 columns) | | `v` | Toggle between hunk diff and full file preview | +| `w` | Toggle soft wrapping of long lines. On, the tail of a long line continues on the next row instead of needing `←`/`→`; the line number folds into the line rather than sitting in its own column, so a continuation row carries no number. Horizontal scrolling is inert while wrapping (and the offset resets when you turn it on). The split view ignores wrapping — halves folding to different heights would stop lining up | +| `Tab` | Cycle the display: unified diff → side-by-side split → file contents → unified. `v` and `s` each toggle one view against the unified default, so the third stays hidden unless you know it exists; `Tab` walks all three. Skips the file step when there is no file to open, and does nothing in tree view | | `s` | Toggle between the unified diff and a side-by-side split view (falls back to unified when the pane is too narrow) | +| — | **Line numbers** are always shown in a pinned gutter. The unified view shows both sides (old, new) — an added line leaves the old column blank, a removed line leaves the new one blank. The split view numbers each half with the side it shows, and the file view (`v`) numbers the file itself. The gutter stays in place while `←`/`→` scroll the code | | ` f` | Zoom the diff/file pane to full screen (toggle) | | `/` | Open search (works in both diff and file preview, including tree mode) | | `n` / `N` | Next / previous search match | diff --git a/docs/architecture.md b/docs/architecture.md index 033139f6..8b2e77e7 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -103,8 +103,11 @@ src/ │ ├── mod.rs # Workspace: open projects (Vec) + active index, │ │ # process-level repo dialog/notice │ ├── repo_input.rs # o repo-input modal state +│ ├── path_complete.rs # Tab 경로 완성 (read_dir 한 단계, 디렉터리만) +│ ├── path_tree.rs # ↓ 디렉터리 브라우저 상태 (평면 row 리스트) +│ ├── repo_picker.rs # 필드 ↔ 브라우저 전환 │ ├── persistence.rs # workspace + per-repo state (~/.nightcrow/workspace.json) -│ └── tests/ # workspace + repo_input tests +│ └── tests/ # workspace + repo_input + repo_picker tests ├── runtime/ │ ├── mod.rs │ ├── snapshot.rs # SnapshotChannel: background git status/log worker @@ -209,6 +212,15 @@ trait TerminalBackend { - UI 스레드 동기 로드: 파일/커밋 선택이 바뀌면 `load_*_with_repo`를 직접 호출한다. App은 `git2::Repository`를 lazy-cache하므로 매 호출마다 `Repository::discover`를 다시 실행하지 않는다. cache는 프로젝트와 수명을 같이 하므로 무효화 시점이 따로 없다 — 저장소가 바뀌는 유일한 방법이 탭을 닫고 새로 여는 것이기 때문. - 경로 검증: 워크트리 안의 파일·디렉토리를 여는 경로는 전부 `git::path::resolve_in_workdir`를 거친다(파일 미리보기와 트리 리스팅 양쪽). plain relative 컴포넌트만 허용하고 `..`·절대경로·NUL·`.git`(대소문자 무시)을 거부하며, 워크디렉토리부터 한 컴포넌트씩 내려가 **모든 깊이의 심링크**를 막고 canonicalize containment로 마무리한다. 지금 호출자는 git이 만들어 낸 경로만 넘기지만, 검증을 호출부가 아니라 파일시스템 경계에 두어야 웹 표면이 요청 문자열을 같은 로더에 태워도 안전하다. 크기 검사와 읽기는 같은 파일 핸들에서, 트리 리스팅은 검증기가 돌려준 경로로 `read_dir`을 수행해 check→use TOCTOU를 닫는다. `.git` 판정은 `is_git_dir_name` 하나로 통일한다 — 대소문자와 후행 점·공백(NTFS가 버리는 문자)까지 흡수하며, 규칙을 두 군데에 따로 적으면 그 틈이 우회로가 된다. - 렌더링: 보이는 행(`scroll_start..scroll_start+visible_height`)에 한해 `syntect`로 syntax highlighting을 수행한다. 보이지 않는 라인은 highlighter state만 진행시켜 multi-line construct(블록 주석, 문자열 리터럴)의 syntax 연속성을 유지한다. +- **줄 번호 gutter**(`ui/diff_viewer/gutter.rs`): `DiffLine`이 libgit2의 `old_lineno`/`new_lineno`를 그대로 들고 다닌다. 추가 줄은 old가, 삭제 줄은 new가 `None`이라 해당 칼럼을 비운다 — hunk 헤더에서 파생시키지 않는 이유는 kind별 카운터를 렌더 층에서 관리하게 되어 상태가 잘못된 층에 놓이기 때문이다. unified은 두 칼럼, split은 좌=old·우=new 한 칼럼씩, file view는 파일 자신의 번호를 보여준다. + - **gutter와 본문은 반드시 별개 `Paragraph`여야 한다.** diff 계열은 수평 스크롤을 `Paragraph::scroll((0, x))`로 구현하는데 이건 라인을 통째로 밀기 때문에, 같은 paragraph에 있는 gutter는 `scroll_x > 0`이면 왼쪽으로 사라진다(실제로 file view에 그 버그가 있었다). `Block`을 따로 그리고 `block.inner`를 `Layout::Horizontal`로 쪼개 gutter는 `scroll((0,0))`, 본문만 스크롤한다. 수직 스크롤은 **어느 행을 담았는지**로 표현되므로 두 vector를 같은 루프에서 lockstep으로 채우는 것이 정렬을 지키는 유일한 수단이다. + - 폭은 로드된 hunk 전체의 최대 줄 번호에서 파생하고 최소 3자리를 보장한다. 보이는 창 기준으로 계산하면 스크롤 중에 본문 좌측 경계가 흔들린다. hunk 헤더 행도 같은 폭의 빈 gutter를 받아야 `@@`가 본문보다 한 칼럼 왼쪽에서 시작하지 않는다. + - `MIN_SPLIT_WIDTH`를 80 → 90으로 올렸다. 각 half가 gutter에 5칼럼을 쓰므로, 문턱을 그대로 두면 side-by-side 진입은 되지만 half당 읽을 수 있는 코드 폭이 조용히 줄어든다. +- **자동 줄바꿈**(`DiffPane::wrap`, diff pane focus에서 `w`): ratatui `Paragraph::wrap`은 켜지면 `scroll.x`를 무시하므로(`ratatui-widgets`의 `render_paragraph`가 wrap 분기에서 `WordWrapper`만 쓰고 `LineTruncator`의 horizontal offset 경로를 타지 않는다) **줄바꿈과 수평 스크롤은 구조적으로 배타**다. 켤 때 `scroll_x`를 0으로 되돌린다 — 남겨두면 끌 때 낡은 오프셋이 되살아난다. + - 줄바꿈 모드에서는 **gutter를 본문 라인 안으로 접어 넣는다**. 본문 한 줄이 여러 화면 행을 먹는데 gutter 라인은 한 행이라, 두 paragraph를 나란히 두면 그 아래 전부가 어긋난다. gutter를 분리한 애초의 이유(수평 스크롤)가 이 모드엔 없으므로 인라인이 안전하다. 대가는 이어지는 행에 번호가 붙지 않는 것. + - **split 뷰는 줄바꿈을 무시한다.** 좌/우 half가 서로 다른 높이로 접히면 행 대응이 무너지는데, 그 대응이 이 레이아웃의 유일한 존재 이유다. + - 수직 스크롤은 여전히 **논리 줄** 단위다(렌더러가 창을 직접 슬라이스하고 ratatui의 vertical scroll을 쓰지 않는다). 따라서 줄바꿈이 켜진 채 긴 줄이 많으면 pane 높이보다 적은 논리 줄만 보이고 아래가 잘린다 — 스크롤로 전부 도달할 수 있으므로 감춰지는 내용은 없다. 검색 매치가 논리 행 인덱스라는 전제도 이 덕분에 유지된다. +- **표시 방식 전환**: `DiffPaneView`는 `Diff`/`Split`/`File` 세 값인데 `v`(File 토글)와 `s`(Split 토글)는 각각 unified를 기준으로 한 축만 오간다 — 세 번째가 있다는 걸 모르면 발견할 수 없다. `Tab`(`App::cycle_diff_view`)이 `Diff → Split → File → Diff`로 셋을 모두 순회해 집합을 드러내고, `v`/`s`는 아는 뷰로 바로 가는 용도로 남는다. File 단계는 `can_open_file_view`가 거짓이면(선택 없음 / 해석 불가한 커밋 파일) 건너뛴다 — `v`가 no-op이 되는 것과 같은 게이트이며, 순회 중 죽은 입력을 만들지 않기 위함이다. Tree 모드는 우측 pane이 항상 파일 미리보기라 순회 대상이 없어 no-op이다. ### Split-View Terminal Panel @@ -332,7 +344,7 @@ background even while scrolled out of the window. - **Lower panel focused (terminal)**: leader/예약키가 아닌 모든 키는 active backend의 stdin으로 직접 통과한다(`encode_key`가 화살표/F-key/제어문자를 VT100 시퀀스로 인코딩). 단독 `Ctrl+T/W/L/O/P/Q` 등은 앱 명령이 아니므로 control byte로 PTY에 전달된다(리더 `Ctrl+F`만 prefix를 arm하고 통과하지 않는다). bare F키는 앱이 가로채므로 pane 안 프로그램(htop, mc 등)의 F키 메뉴는 동작하지 않는다 — 수정자를 붙인 `Ctrl+F1`, `Shift+F5` 등은 통과한다. - overlay(repo input/search) active 시에는 leader dispatch가 금지되고 overlay가 키를 소유한다. armed 중 overlay가 열리는 경로면 prefix를 취소한다. repo 다이얼로그는 `Workspace` 소유라 `main::dispatch_key`가 per-project 핸들러보다 먼저 처리한다 — 프로젝트가 없을 때도 열려야 하기 때문. - **프로젝트가 없을 때**: `main::handle_empty_key`가 leader arming과 `o`/`q`만 해석하고 나머지는 버린다. ` `는 여기서도 액션 테이블로 넘어가지 않는다 — 기본 leader가 `ctrl+f`라 follow-up이 `f`에 매칭돼 fullscreen이 토글될 수 있기 때문. -- 좌측/우측 패널 타이틀에는 현재 포커스 단축키(`F1` / `F2`)가 노출돼 사용자가 즉시 jump 키를 알 수 있다. +- 좌측/우측 패널 타이틀에는 현재 포커스 단축키(` 1` / ` 2`, 기본 leader면 `^F 1` / `^F 2`)가 노출돼 사용자가 즉시 jump 키를 알 수 있다. `ui::jump_legend`가 설정된 leader label과 digit을 **공백으로** 이어 붙인다 — `^F1`로 붙여 쓰면 Ctrl+F1로 읽히고, 그 조합은 앱이 가로채지 않고 PTY로 통과시키는 별개 키라 오해를 만든다. 프로젝트 탭 행이 쓰는 `F1`…`F10` legend와는 다른 축임에 주의한다. ### Project Boundary (`Workspace` / `App`) @@ -358,6 +370,67 @@ pane을 살려두는 탓에 탭 라벨과 셸의 작업 디렉토리가 어긋 `dispatch_key`가 워크스페이스 레벨 경우(다이얼로그, 빈 화면의 두 키)를 먼저 해소하므로, 프로젝트별 입력 경로 전체가 프로젝트 하나만 아는 채로 유지된다. +**경로 완성** — 다이얼로그의 `Tab`은 `workspace/path_complete.rs`가 처리한다. +셸을 PTY로 띄우지 않는 이유와 대안 비교는 `docs/repo-picker-plan.md`에 있다 — +요약하면 Windows에 readline 대응 프리미티브가 없어서 네이티브 완성기가 어차피 +필요하다. 규칙은 무상태 하나다: **확장할 게 있으면 확장하고, 없으면 후보를 +보여준다.** 단 fragment가 비어 있으면(구분자로 끝나는 상태) 확장과 동시에 +목록도 낸다 — 그때의 `Tab`은 "여기 뭐가 있냐"는 질문이라 조용한 확장은 답이 +아니다. Tab 한 번에 `read_dir` 한 단계만 읽고 디렉터리만 후보로 삼는다. + +사용자가 입력한 텍스트는 다시 쓰지 않는다. `~`나 상대 경로는 **읽을 때만** +확장하고 버퍼에는 완성된 컴포넌트만 이어붙인다 — `~/x`를 `/Users/me/x`로 +바꿔 써넣으면 사용자가 타이핑한 적 없는 경로가 화면에 남는다. + +`git::tree::read_children`(`ViewMode::Tree`용)을 쓰지 않는다는 점에 주의한다. +그쪽은 `git2::Repository`가 필수이고 repo-relative 경로만 받으며 워크트리 밖 +경로와 심볼릭 링크를 거부하는데, 피커는 어떤 repo에도 속하지 않는 경로를 +돌아다녀야 하고 프로젝트가 0개일 때도 떠야 한다. 심볼릭 링크 정책도 반대다 — +트리는 링크를 따라가지 않지만(순환 방지) 피커는 따라간다(링크된 체크아웃이 +실제 repo다). + +후보는 notice 행에 표시한다(`ui/notice.rs`). 우선순위는 notice > 후보 > +repo 헤더다. 플로팅 팝업을 쓰지 않은 이유는 `src/ui/`에 오버레이 인프라가 +없고(모든 surface가 레이아웃 행을 차지한다) 마우스 캡처가 기본 on이라 +`hit_test.rs`에 새 히트 영역이 필요해지기 때문이다. + +**디렉터리 브라우저** — `workspace/path_tree.rs`(상태) + `ui/path_tree.rs`(렌더). +경로를 아는 경우(형제 체크아웃 — prefill이 노리는 케이스)는 타이핑이 빠르고 +모르는 경우는 브라우저가 낫다. 둘은 경쟁이 아니라 계층이다. + +- **진입은 `↓`**(또는 `↑`). printable 문자는 전부 합법 경로 문자라 쓸 수 없고, + 필드의 수평 키(`→`/`End`=prefill 수락)는 이미 "이 경로를 편집한다"는 뜻이라 + 수직 축이 비어 있다 — 브라우저 안에서 `↓`/`j`가 커서를 옮기므로 진입 키와 + 진입 후 조작이 같은 축에 놓이고, 모든 자동완성이 목록을 아래에 두는 관용과도 + 맞는다. `Ctrl+T`는 접었다: `T` 니모닉이 ` t`(새 터미널)와 겹쳐 + "충돌하지 않는다"를 설명해야 했고, 다이얼로그의 다른 키가 전부 bare인데 + Ctrl 화음만 튄다. +- **후보 목록이 떠 있을 때의 두 번째 `Tab`도 브라우저로 승격한다.** 그 상태의 + Tab은 같은 목록을 다시 그리는 죽은 키였고, 평면 목록이 실패한 지점이 정확히 + 거기다 — 배울 키 없이 도달하는 경로를 하나 남긴다. +- **`Enter`는 확정이 아니라 필드로 되돌리며 경로를 채운다.** repo를 실제로 여는 + 지점은 필드의 `Enter` 한 곳뿐이다. 그래서 `Enter`의 의미가 두 surface에서 + 갈리고, 브라우저에서는 확장이 `→` 전용이다(트리 뷰는 `Enter`도 확장한다). +- **평면 row 리스트**로 들고 있다. 확장은 자식을 부모 뒤에 splice, 접기는 아래 + 깊은 row를 drain — 선택이 화면 인덱스 그대로여서 프레임마다 flatten이 없다. +- **사용자 표기를 보존한다**(완성기와 같은 이유). `root_text`(타이핑한 그대로)와 + canonical `PathBuf`를 따로 들고, 고른 경로는 `root_text` 기준으로 조립한다. + `←`가 depth 0에서 루트를 한 단계 올릴 때만 예외가 생긴다 — `~`나 Windows + 드라이브의 부모는 사용자 표기로 표현할 수 없으므로 절대 경로로 대체하되, + 텍스트 수술을 믿지 않고 `canonicalize` 결과를 실제 부모와 대조해 검증한다. +- **body 전체를 쓴다**(위의 팝업 부재와 같은 이유). 다이얼로그가 이미 모든 키를 + 소유하므로 view mode·fullscreen 분기보다 앞에서 body를 가로챈다 — 그 분기들이 + 그릴 것은 어차피 inert다. 마우스 클릭 선택은 범위 밖(`hit_test.rs`에 새 히트 + 영역이 필요하다). 세션 저장도 하지 않는다: 필드가 활성 프로젝트 경로로 + prefill되므로 "지난 위치"가 새 영속 상태 없이 따라온다. +- 브라우저를 열면 `prefilled`가 해제된다. 브라우저는 버퍼에 전체 경로를 쓰므로, + 플래그가 살아 있으면 복귀 후 첫 타이핑이 방금 고른 경로를 지운다. + +다이얼로그는 hint legend를 통째로 대체하므로(입력 줄이 그 자리를 쓴다) 키를 +알릴 다른 자리가 없다. `hint_bar::repo_input_line`이 커서 뒤에 축약 legend를 +붙이고, 폭이 모자라면 잘라내지 않고 통째로 버린다 — 커서는 반드시 보여야 하고 +반쪽 legend는 렌더 결함으로 읽힌다. + 입력 핸들러는 `&mut App` 하나만 받으므로 탭 목록에 닿을 수 없다. 대신 워크스페이스 수준 의도를 `KeyOutcome::Project(ProjectRequest)`로 반환하고 `main_loop`이 실행한다. 이 덕분에 프로젝트별 입력 경로 전체가 그대로 유지된다. diff --git a/docs/repo-picker-plan.md b/docs/repo-picker-plan.md new file mode 100644 index 00000000..5ce2a93c --- /dev/null +++ b/docs/repo-picker-plan.md @@ -0,0 +1,173 @@ +# repo 다이얼로그 경로 탐색 — 구현 계획 + +> **상태: 구현 완료 (계획 이력).** 1단계(Tab 완성)와 2단계(디렉터리 브라우저) 모두 +> 들어갔다. 확정된 설계는 `docs/architecture.md`로 이관했고, 사용법은 `README.md`에 +> 있다. 이 문서는 **왜 그렇게 갔는지**의 이력으로 남긴다. +> +> 구현이 5절의 계획과 갈린 지점 두 개: +> - **진입 키는 `Ctrl+T`가 아니라 `↓`다.** `T` 니모닉이 ` t`(새 터미널)와 겹쳐 +> "충돌하지 않는다"를 설명해야 했는데, 설명이 필요한 키는 이미 진 것이다. 다이얼로그의 +> 다른 키가 전부 bare인 것과도 맞고, 필드의 수평 키가 이미 "이 경로를 편집한다"는 +> 뜻이라 수직 축이 비어 있었다. 후보 목록이 떠 있을 때의 두 번째 `Tab`도 같은 곳으로 +> 승격한다 — 배울 키 없이 도달하는 경로. +> - **hint 행에 키 legend를 붙였다.** 계획에 없던 항목인데, 다이얼로그가 hint legend를 +> 통째로 입력 줄로 대체해서 `Tab` 완성조차 화면에 안 나오고 있었다. 진입 키를 아무리 +> 잘 골라도 광고할 자리가 없으면 못 찾는다. +> +> - **상태는 `BTreeSet` + children 캐시가 아니라 평면 row 리스트다.** 확장이 자식을 부모 +> 뒤에 splice하고 접기가 아래 깊은 row를 drain하면, 선택이 화면 인덱스 그대로여서 +> visible_rows 계산도 캐시 무효화도 필요 없다. 계획이 트리 뷰의 구조를 따라가려 했지만 +> 그쪽 복잡도는 repo-relative 검색 인덱스에서 온 것이고 브라우저에는 없다. +> +> 계획대로 범위 밖으로 남긴 것: 마우스 클릭 선택. + +## 1. 문제 + +` o` repo 다이얼로그(`src/workspace/repo_input.rs`)는 append-only `String` +버퍼다. `Tab`은 `text_input_char`가 `None`을 돌려주므로 **완전히 무시**된다. 경로를 +전부 손으로 쳐야 하고, 어떤 하위 폴더가 있는지 볼 방법이 없다. + +## 2. 셸을 띄우지 않는 이유 + +`bash --norc -c 'read -e -p ...'`(readline) 또는 zsh `vared`를 PTY로 띄우면 완성이 +공짜로 따라온다. 검토 후 접었다: + +- **Windows에 대응 프리미티브가 없다.** PowerShell `Read-Host`는 완성이 없고 + (PSReadLine은 대화형 호스트 루프 전용), cmd `set /p`도 없다. Windows를 목표로 두는 + 순간 네이티브 완성기를 어차피 써야 하므로 셸은 *대체*가 아니라 *추가* 경로가 된다. +- 결과 회수가 PTY 스트림 하나뿐이라 sentinel/임시 파일 프로토콜이 필요하다. +- readline 후보 목록은 여러 줄 + "Display all N possibilities?"를 뿜어 hint bar 1줄로 + 안 되고, PTY 그리드 렌더 영역을 새로 만들어야 한다. +- `$SHELL` 그대로 쓰면 rc 오염·시작 지연·rc가 입력 대기 시 먹통 리스크가 붙는다. + +`std::fs::read_dir` 기반 네이티브 구현은 새 의존성 0, `cfg(windows)` 분기 0으로 +같은 체감을 준다. + +## 3. 기존 트리 인프라를 재사용하지 않는 이유 + +2단계(트리 피커)에서 `ViewMode::Tree` 자산을 쓰고 싶었지만 대부분 못 쓴다: + +- `git::tree::read_children`은 `git2::Repository`가 필수이고 **repo-relative** 경로만 + 받으며, `resolve_in_workdir`이 워크트리 밖 경로와 심볼릭 링크를 거부한다. 피커는 + *어떤 repo에도 속하지 않는* 경로를 돌아다녀야 하고 **프로젝트 0개 상태**에서도 떠야 + 한다 — 이 함수가 막으려고 만들어진 것이 정확히 피커의 일이다. +- `TreeView`(`src/ui/tree_view/mod.rs`)는 `App` 소유(프로젝트별)이고 search index / + show_set / row_width_cache가 repo-relative 트리 전용이다. 다이얼로그는 `Workspace` + 레벨이라 타입 공유가 아니라 expand/visible_rows **패턴만** 참고한다. +- `tree_list::render`는 `&App`, `app.focus`, `jump_legend(app, '1')`에 의존해 빈 화면에서 + 호출 불가. + +실제 재사용: `render_selectable_list`(`src/ui/helpers.rs`) 하나. 그리고 1단계의 +디렉터리 목록 함수는 2단계가 그대로 쓴다 — 두 단계의 공통 기반이다. + +## 4. 1단계 — Tab 완성 + +### 후보 표시 위치: notice 행 재사용 + +`chrome_rows`가 이미 두 화면 모두에 notice 행을 할당한다. 새 팝업 영역은 레이아웃과 +hit-test로 번지므로 기존 행을 쓴다. 우선순위: + +``` +에러 notice (빨강) > 후보 목록 (DarkGray) > repo 헤더 (경로/브랜치) +``` + +프로젝트 화면에서 그 행은 repo 헤더(`render_notice_row` → `render_repo_header`)라, +완성 중에는 후보가 헤더를 덮는다. 에러 notice가 이미 같은 방식으로 덮으므로 일관된다. +폭 초과 시 `+N more`로 자른다. + +### Tab 규칙: "진행이 있으면 진행, 없으면 보여준다" + +카운터 없는 무상태 규칙 하나로 readline 체감이 나온다. + +| 매칭 수 | 동작 | +| --- | --- | +| 0개 | 무변경. 에러 notice 띄우지 않음 — 타이핑 중 정상 상태다 | +| 1개 | 이름 + 구분자 삽입 → Tab 연타로 하위 탐색 | +| N개, 공통 접두사가 fragment와 다름 | 공통 접두사까지 확장(대소문자 교정 포함), 목록 없음 | +| N개, 더 확장 불가 | 후보 목록 표시 | + +`/`로 끝나는 상태(fragment 빈 상태)에서는 첫 Tab에 그 폴더 내용이 바로 뜬다 — 원래 +불편의 핵심 케이스. bash 기본값은 벨만 울리고 두 번째 Tab에 보여주지만, readline의 +`show-all-if-ambiguous on` 쪽을 택했다. + +### 매칭 규칙 + +- **디렉터리만.** repo 피커이므로. `file_type()`로 판정하고 심볼릭 링크일 때만 + `path().is_dir()`로 추가 stat — 심볼릭 링크된 repo를 살리면서 큰 디렉터리에서 + entry당 stat을 피한다. +- **구분자**: `/`는 항상, `\`는 `cfg!(windows)`일 때만. Unix에서 `\`는 합법 파일명 + 문자다. `cfg!` 상수 분기라 `#[cfg]` 블록이 필요 없다. +- **삽입할 구분자**: 버퍼에 이미 있는 마지막 구분자를 재사용, 없으면 + `MAIN_SEPARATOR`. Windows에서 `/`로 쳤는데 `\`가 섞이는 것을 막는다. +- **대소문자**: 정확한 접두사를 먼저 시도하고 0개일 때만 무시하고 재시도. Linux에서 + 예상 밖 매칭을 만들지 않고 macOS/Windows에선 편하다. +- **숨김 폴더**: fragment가 `.`로 시작할 때만 포함(셸 관례). +- **구분자 없는 버퍼**: 프로세스 cwd 기준 — `confirm_repo_input`이 상대 경로를 해석하는 + 기준과 같다. +- **비-UTF8 파일명 제외**: 버퍼가 `String`이라 `to_string_lossy`로 넣으면 다시 열 수 + 없는 경로가 된다. +- 접두사·공통접두사 계산은 char 단위(UTF-8 경계 안전). +- `read_dir` 실패(권한 없음, 디렉터리 아님)는 조용한 no-op. + +### 셸이 아니라는 것의 의미 + +`cd`/`ls` 등 커맨드, `$VAR`, 글롭, 커맨드 치환은 없다. Enter는 항상 "이 경로 열기"다. +동작하는 것: `~`·`~/rest`(`expand_tilde`), `..`(OS가 해석하고 확정 시 +`resolve_repo_path`가 canonicalize한다), cwd 기준 상대 경로. + +### 사용자가 입력한 텍스트는 다시 쓰지 않는다 + +`~`나 상대 경로는 **읽을 때만** 확장하고, 버퍼에는 완성된 컴포넌트만 이어붙인다. +`~/x`를 `/Users/me/x`로 바꿔 써넣지 않는다. + +### 작업 순서 + +1. `src/workspace/path_complete.rs` 신규 — 순수 함수 + 단위 테스트. + `complete_dir_path(buf) -> PathCompletion { buf, candidates }`. `Workspace`에 + 의존하지 않아 `tempfile`로 실제 트리를 만들어 검증한다. +2. 다이얼로그 배선 — `RepoInput.candidates`, `repo_input_complete()`, + `push`/`pop`/`accept_prefill`/`start`/`cancel`에서 후보 무효화, + `handlers.rs`에 `KeyCode::Tab` arm. `REPO_INPUT_MAX_BYTES` 상한은 호출자가 검사한다. +3. 후보 렌더 — `render_notice_row`가 후보를 받도록 확장, `draw`/`draw_empty` 갱신, + 폭 초과 시 `+N more`, 우선순위 테스트. +4. 문서 — `README.md` 키 표 + notice 행 설명, `docs/architecture.md`. + +각 단계 후 `cargo build && cargo test && cargo clippy --all-targets --all-features -- -D warnings` +통과 상태를 유지한다. 2단계까지만 머지된 중간 상태도 동작한다(완성은 되고 후보만 안 보임). + +## 5. 2단계 — 디렉터리 브라우저 (확정 당시의 설계) + +- **`Enter`는 확정이 아니라 텍스트 필드로 되돌리며 그 경로를 채운다.** 트리는 필드를 + 채우는 피커고, repo를 실제로 여는 지점은 여전히 필드의 `Enter` 한 곳이다. 선택 후에도 + `Tab`으로 더 파고들거나 손으로 고칠 수 있다. +- **여는 키는 `Ctrl+T`.** printable 문자는 전부 합법 경로 문자라 쓸 수 없고, 다이얼로그가 + 키를 독점하므로 앱의 다른 `Ctrl+T`(새 터미널)와 충돌하지 않는다. +- **세션 저장은 하지 않는다.** 트리를 텍스트 필드가 현재 가리키는 디렉터리에서 열면 + 된다. 필드는 이미 활성 프로젝트 경로로 prefill되므로 "지난 위치"가 새 영속 상태 없이 + 따라온다. +- 트리 키: `j`/`k` 이동, `→` 확장, `←` 접기/부모로, `Enter` 선택 후 필드 복귀, + `Esc` 트리만 닫기(한 번 더 누르면 다이얼로그 취소). 기존 트리 뷰(` b`)는 `→`와 + `Enter` 둘 다 확장이지만 여기서는 `Enter`가 선택이라 확장은 `→` 전용이다 — 같은 앱에서 + `Enter` 의미가 갈리는 지점이라 README에 명시한다. +- 디렉터리만 표시한다(1단계와 동일 — 파일은 repo가 될 수 없다). +- 루트 위로 올라갈 수 있어야 한다. depth 0에서 `←`는 부모로 re-root한다. 버퍼의 literal + 텍스트(`root_text`)와 확장된 `PathBuf`를 따로 들고, 고른 경로는 `root_text` 기준으로 + 조립한다 — 1단계와 같은 이유로 사용자가 타이핑한 `~`를 절대 경로로 바꿔 쓰지 않는다. +- **마우스 클릭 선택은 범위 밖.** `hit_test.rs`에 새 히트 영역이 필요해 별도 작업으로 + 둔다. 키보드로 완결된다. +- **플로팅이 아니라 body 영역 전체를 쓰는 리스트.** `src/ui/`에 팝업/오버레이 인프라가 + 전혀 없다(`Clear` 위젯도 centered-rect 헬퍼도 없고, 모든 surface가 레이아웃 영역을 + 차지한다). 떠 있는 박스는 이 프로젝트 최초의 플로팅 UI가 되고 마우스 캡처가 기본 on이라 + `hit_test.rs`에 새 히트 영역을 끼워야 한다. body 전체를 쓰면 둘 다 크게 줄어든다. +- 1단계의 `split_dir` / `read_dir_names`를 그대로 쓴다 — 이미 추출해 뒀다. +- 텍스트 필드와 병행한다: 필드에서 `Tab`은 완성, `Ctrl+T`로 트리를 연다. 경로를 아는 + 경우(형제 체크아웃 — prefill이 노리는 케이스)는 타이핑이 빠르고, 모르는 경우는 트리가 + 낫다. 경쟁 관계가 아니다. + +### 작업 순서 + +1. `workspace/path_tree.rs` — 트리 상태(선택 / 확장 `BTreeSet` / lazy children 캐시)와 + 탐색. `Workspace` 소유. +2. 필드 ↔ 트리 전환 배선 — `Ctrl+T` 열기, `Enter` 선택 시 필드 버퍼에 경로 + 구분자 주입. +3. `ui/path_tree.rs` — body 렌더. `render_selectable_list` 재사용. +4. README 키 표 + `architecture.md` 갱신. diff --git a/src/app/file_view_load.rs b/src/app/file_view_load.rs index 76123380..2532518e 100644 --- a/src/app/file_view_load.rs +++ b/src/app/file_view_load.rs @@ -112,6 +112,47 @@ impl App { }; } + /// Soft-wrap long lines instead of scrolling sideways to reach them. + /// + /// Turning wrapping on resets the horizontal offset. ratatui ignores it + /// while wrapping, so leaving it set would strand a stale offset that + /// silently reappears the moment wrapping is turned back off. + pub fn toggle_diff_wrap(&mut self) { + self.diff.wrap = !self.diff.wrap; + if self.diff.wrap { + self.diff.scroll_x = 0; + self.diff.file_view.scroll_x = 0; + } + } + + /// Step to the next display: unified → split → file → unified. + /// + /// `v` and `s` each toggle one view against the unified default, which + /// leaves the third one undiscoverable unless you already know it exists. + /// One key that walks all three makes the set visible; the direct toggles + /// stay for jumping straight to a known view. + /// + /// The file step is skipped when there is nothing to open (no selection, or + /// a commit whose file cannot be resolved) rather than being a dead press — + /// the same gate `can_open_file_view` puts on `v`. + pub fn cycle_diff_view(&mut self) { + // Tree mode's right pane is always the raw file preview, so there is + // no cycle to walk — matching `v`/`s`. + if self.mode == ViewMode::Tree { + return; + } + match self.diff.view { + DiffPaneView::Diff => self.toggle_diff_split_view(), + DiffPaneView::Split => { + self.diff.view = DiffPaneView::Diff; + if self.can_open_file_view() { + self.toggle_diff_file_view(); + } + } + DiffPaneView::File => self.toggle_diff_file_view(), + } + } + pub(crate) fn load_commit_diff_for_selected(&mut self) { let (oid, title) = match self.log_view.commits.get(self.log_view.selected) { Some(entry) => (entry.oid, entry.to_string()), diff --git a/src/app/tests/diff_file_view.rs b/src/app/tests/diff_file_view.rs index d08b1154..1e7bacb3 100644 --- a/src/app/tests/diff_file_view.rs +++ b/src/app/tests/diff_file_view.rs @@ -36,6 +36,56 @@ fn toggle_diff_file_view_ignores_selection_outside_filter() { assert!(app.diff.file_view.key.is_none()); } +#[test] +fn cycling_the_diff_view_walks_all_three_and_returns_to_the_start() { + let mut app = app_with_files(vec!["a.rs"]); + assert_eq!(app.diff.view, DiffPaneView::Diff); + + app.cycle_diff_view(); + assert_eq!(app.diff.view, DiffPaneView::Split); + app.cycle_diff_view(); + assert_eq!(app.diff.view, DiffPaneView::File); + app.cycle_diff_view(); + assert_eq!( + app.diff.view, + DiffPaneView::Diff, + "the cycle must close so one key can reach every view" + ); +} + +#[test] +fn cycling_skips_the_file_view_when_there_is_nothing_to_open() { + // Selection outside the filter leaves no resolvable file, the same gate + // that makes `v` a no-op. Skipping keeps the press from doing nothing. + let mut app = app_with_files(vec!["alpha.rs", "bravo.rs"]); + app.status_view.search_query.set("alpha"); + app.status_view.recompute_filter(); + app.status_view.selected = 1; + assert!(!app.can_open_file_view()); + + app.cycle_diff_view(); + assert_eq!(app.diff.view, DiffPaneView::Split); + app.cycle_diff_view(); + assert_eq!( + app.diff.view, + DiffPaneView::Diff, + "with no file to show the cycle is unified <-> split" + ); +} + +#[test] +fn cycling_the_diff_view_does_nothing_in_tree_mode() { + // Tree mode's right pane is always the raw file preview, so there is no + // cycle to walk — matching `v`/`s`. + let mut app = app_with_files(vec!["a.rs"]); + app.mode = ViewMode::Tree; + app.diff.view = DiffPaneView::File; + + app.cycle_diff_view(); + + assert_eq!(app.diff.view, DiffPaneView::File); +} + #[test] fn toggle_diff_split_view_round_trips_and_overrides_file_view() { let mut app = app_with_files(vec!["a.rs"]); @@ -129,3 +179,31 @@ fn snapshot_refresh_with_no_filter_matches_clears_file_view() { assert_eq!(app.diff.view, DiffPaneView::Diff); assert!(app.diff.file_view.key.is_none()); } + +#[test] +fn enabling_wrap_drops_the_stale_horizontal_offset() { + // ratatui ignores scroll.x while wrapping, so an offset left behind would + // reappear the moment wrap is switched back off. + let mut app = app_with_files(vec!["a.rs"]); + app.diff.scroll_x = 12; + app.diff.file_view.scroll_x = 9; + + app.toggle_diff_wrap(); + + assert!(app.diff.wrap); + assert_eq!(app.diff.scroll_x, 0); + assert_eq!(app.diff.file_view.scroll_x, 0); +} + +#[test] +fn disabling_wrap_leaves_the_offset_where_it_was_reset() { + let mut app = app_with_files(vec!["a.rs"]); + app.toggle_diff_wrap(); + app.toggle_diff_wrap(); + + assert!(!app.diff.wrap); + assert_eq!( + app.diff.scroll_x, 0, + "turning wrap off must not resurrect a pre-wrap offset" + ); +} diff --git a/src/app/tests/helpers.rs b/src/app/tests/helpers.rs index 587d9ce6..2b2fce9f 100644 --- a/src/app/tests/helpers.rs +++ b/src/app/tests/helpers.rs @@ -90,6 +90,8 @@ pub(crate) fn context_hunk(lines: &[&str]) -> DiffHunk { .map(|content| DiffLine { kind: LineKind::Context, content: (*content).to_string(), + old_lineno: None, + new_lineno: None, }) .collect(), file_path: None, diff --git a/src/application/input/dispatch.rs b/src/application/input/dispatch.rs index 43cec91e..94b246ee 100644 --- a/src/application/input/dispatch.rs +++ b/src/application/input/dispatch.rs @@ -1,7 +1,8 @@ use crate::app::{App, Focus}; use crate::application::input::handlers::{ - handle_empty_key, handle_repo_input_key, handle_terminal_key, handle_upper_key, + handle_empty_key, handle_terminal_key, handle_upper_key, }; +use crate::application::input::repo_dialog::handle_repo_input_key; use crate::input::{Action, encode_key, map_key, prefix_action, prefix_action_fullscreen}; use crate::workspace::Workspace; use crossterm::event::{KeyCode, KeyEvent, KeyEventKind, KeyModifiers}; diff --git a/src/application/input/handlers.rs b/src/application/input/handlers.rs index b7419258..558273cf 100644 --- a/src/application/input/handlers.rs +++ b/src/application/input/handlers.rs @@ -33,33 +33,6 @@ pub(crate) fn handle_empty_key(ws: &mut Workspace, key: KeyEvent) -> KeyOutcome KeyOutcome::Continue } -pub(crate) fn handle_repo_input_key(ws: &mut Workspace, key: KeyEvent) -> KeyOutcome { - match key.code { - KeyCode::Esc => ws.cancel_repo_input(), - KeyCode::Enter => { - if let crate::workspace::RepoInputResult::Open(path) = ws.confirm_repo_input() { - return KeyOutcome::Project(ProjectRequest::Open(path)); - } - } - KeyCode::Backspace => { - if ws.repo_input.buf.is_empty() { - ws.cancel_repo_input(); - } else { - ws.repo_input_pop(); - } - } - // The caret is always at the end of the buffer, so these can't move - // it; they mean "keep this path and let me extend it". - KeyCode::Right | KeyCode::End => ws.repo_input_accept_prefill(), - _ => { - if let Some(c) = text_input_char(key) { - ws.repo_input_push(c); - } - } - } - KeyOutcome::Continue -} - pub(crate) fn handle_terminal_key(app: &mut App, key: KeyEvent, action: Action) { match action { Action::TermScrollUp => { @@ -260,6 +233,9 @@ fn handle_unmapped_upper_key(app: &mut App, key: KeyEvent) { Focus::DiffViewer => match key.code { _ if matches_text_command(key, 'v') => app.toggle_diff_file_view(), _ if matches_text_command(key, 's') => app.toggle_diff_split_view(), + _ if matches_text_command(key, 'w') => app.toggle_diff_wrap(), + // Walks all three views; `v`/`s` still jump straight to one. + KeyCode::Tab => app.cycle_diff_view(), _ if matches_text_command(key, '/') => { exit_split_for_search(app); app.diff.start_search(); diff --git a/src/application/input/mod.rs b/src/application/input/mod.rs index 63cf73cc..0721b817 100644 --- a/src/application/input/mod.rs +++ b/src/application/input/mod.rs @@ -4,3 +4,4 @@ pub(crate) mod dispatch; mod handlers; pub(crate) mod mouse; pub(crate) mod paste; +mod repo_dialog; diff --git a/src/application/input/repo_dialog.rs b/src/application/input/repo_dialog.rs new file mode 100644 index 00000000..b6077f88 --- /dev/null +++ b/src/application/input/repo_dialog.rs @@ -0,0 +1,76 @@ +//! Keys for the open-repo dialog: the path field, and the directory browser it +//! can open. The dialog owns every key while it is up, so these are all bare +//! keys — no leader, and no chord to keep clear of the app's own bindings. + +use crate::application::input::dispatch::{KeyOutcome, ProjectRequest, text_input_char}; +use crate::workspace::Workspace; +use crossterm::event::{KeyCode, KeyEvent}; + +pub(crate) fn handle_repo_input_key(ws: &mut Workspace, key: KeyEvent) -> KeyOutcome { + // The browser takes the keys while it is open; the field is still on screen + // below it, but its text cannot change until the browser hands a path back. + if ws.repo_input.picker.is_some() { + handle_picker_key(ws, key); + return KeyOutcome::Continue; + } + match key.code { + KeyCode::Esc => ws.cancel_repo_input(), + KeyCode::Enter => { + if let crate::workspace::RepoInputResult::Open(path) = ws.confirm_repo_input() { + return KeyOutcome::Project(ProjectRequest::Open(path)); + } + } + KeyCode::Backspace => { + if ws.repo_input.buf.is_empty() { + ws.cancel_repo_input(); + } else { + ws.repo_input_pop(); + } + } + // The caret is always at the end of the buffer, so these can't move + // it; they mean "keep this path and let me extend it". + KeyCode::Right | KeyCode::End => ws.repo_input_accept_prefill(), + // Down opens the browser, matching where every autocomplete puts its + // list. Up too: reaching for either vertical key means "the list", + // and neither can mean anything else in a single-line field. + KeyCode::Down | KeyCode::Up => ws.repo_input_browse(), + // `BackTab` is deliberately unhandled: completion here never cycles, so + // there is nothing for a reverse Tab to step back through. + KeyCode::Tab => { + // A Tab that can no longer extend the path has already shown the + // candidate list, so a second one has nothing left to do — that + // dead press is exactly the moment the flat list proved too little, + // so it escalates to the browser instead. + if ws.repo_input.candidates.is_empty() { + ws.repo_input_complete(); + } else { + ws.repo_input_browse(); + } + } + _ => { + if let Some(c) = text_input_char(key) { + ws.repo_input_push(c); + } + } + } + KeyOutcome::Continue +} + +/// `Enter` selects here rather than opening — the browser fills the field, and +/// the field's Enter remains the single place a repo is opened. That splits the +/// meaning of Enter between the two surfaces, which is why `→` alone expands +/// (unlike the in-repo tree view, where Enter expands too). +fn handle_picker_key(ws: &mut Workspace, key: KeyEvent) { + match key.code { + // One Esc leaves the browser, a second cancels the dialog: the field's + // text survives the first, so a browse can be abandoned without + // retyping the path it started from. + KeyCode::Esc => ws.repo_input_close_browser(), + KeyCode::Enter => ws.repo_input_pick(), + KeyCode::Down | KeyCode::Char('j') => ws.repo_picker_move(true), + KeyCode::Up | KeyCode::Char('k') => ws.repo_picker_move(false), + KeyCode::Right => ws.repo_picker_expand(), + KeyCode::Left => ws.repo_picker_collapse(), + _ => {} + } +} diff --git a/src/application/tests/mod.rs b/src/application/tests/mod.rs index 0308837c..28dd549d 100644 --- a/src/application/tests/mod.rs +++ b/src/application/tests/mod.rs @@ -6,6 +6,7 @@ mod mouse_release; mod paste; mod prefix; mod prefix_digits; +mod repo_dialog; mod search; mod swap; mod terminal; diff --git a/src/application/tests/repo_dialog.rs b/src/application/tests/repo_dialog.rs new file mode 100644 index 00000000..5f92f5a0 --- /dev/null +++ b/src/application/tests/repo_dialog.rs @@ -0,0 +1,134 @@ +//! Key routing for the repo dialog's directory browser. Pins which surface owns +//! a key — the browser's own behaviour lives in `workspace::path_tree`, and the +//! field/browser contract in `workspace::tests::repo_picker_tests`. + +use super::helpers::*; +use crate::application::input::dispatch::{KeyOutcome, dispatch_key}; +use crate::workspace::Workspace; +use crossterm::event::{KeyCode, KeyModifiers}; +use tempfile::TempDir; + +/// The dialog open on a real temp directory holding `dirs`, plus its canonical +/// path as the field's text. +fn dialog_on(dirs: &[&str]) -> (TempDir, Workspace, String) { + let root = TempDir::new().expect("a temp dir"); + for d in dirs { + let mut p = root.path().to_path_buf(); + for part in d.split('/') { + p.push(part); + if !p.is_dir() { + std::fs::create_dir(&p).expect("create dir"); + } + } + } + let text = std::fs::canonicalize(root.path()) + .expect("canonical temp path") + .to_str() + .expect("a UTF-8 temp path") + .to_string(); + let mut ws = workspace_on(&["/a"]); + ws.start_repo_input(); + ws.repo_input.buf = text.clone(); + (root, ws, text) +} + +fn send(ws: &mut Workspace, code: KeyCode) { + assert_eq!( + dispatch_key(ws, press(code, KeyModifiers::NONE)), + KeyOutcome::Continue + ); +} + +#[test] +fn down_in_the_field_opens_the_browser() { + let (_guard, mut ws, _) = dialog_on(&["alpha"]); + + send(&mut ws, KeyCode::Down); + + assert!(ws.repo_input.picker.is_some()); +} + +#[test] +fn a_second_tab_escalates_from_the_candidate_list_to_the_browser() { + // The first Tab cannot extend past the shared prefix, so it lists; the + // second would repeat that list, which is the moment the list proved too + // little. + let (_guard, mut ws, _) = dialog_on(&["alpha", "another"]); + ws.repo_input.buf.push('/'); + + send(&mut ws, KeyCode::Tab); + assert!( + !ws.repo_input.candidates.is_empty() && ws.repo_input.picker.is_none(), + "the first Tab lists without leaving the field" + ); + + send(&mut ws, KeyCode::Tab); + assert!(ws.repo_input.picker.is_some(), "the second Tab escalates"); +} + +#[test] +fn tab_still_completes_when_no_list_is_up() { + let (_guard, mut ws, text) = dialog_on(&["alpha"]); + ws.repo_input.buf = format!("{text}/al"); + + send(&mut ws, KeyCode::Tab); + + assert_eq!(ws.repo_input.buf, format!("{text}/alpha/")); + assert!(ws.repo_input.picker.is_none()); +} + +#[test] +fn the_browser_takes_the_keys_the_field_would_have_had() { + let (_guard, mut ws, text) = dialog_on(&["alpha/inner"]); + send(&mut ws, KeyCode::Down); + + // In the field these would edit the buffer; here they drive the tree. + send(&mut ws, KeyCode::Right); + send(&mut ws, KeyCode::Down); + send(&mut ws, KeyCode::Enter); + + assert!(ws.repo_input.picker.is_none(), "Enter selects and returns"); + assert_eq!(ws.repo_input.buf, format!("{text}/alpha/inner/")); + assert!(ws.repo_input.active, "selecting must not open the repo"); +} + +#[test] +fn typing_cannot_change_the_field_while_the_browser_is_up() { + let (_guard, mut ws, text) = dialog_on(&["alpha"]); + send(&mut ws, KeyCode::Down); + + send(&mut ws, KeyCode::Char('x')); + send(&mut ws, KeyCode::Backspace); + + assert_eq!(ws.repo_input.buf, text); +} + +#[test] +fn the_first_esc_leaves_the_browser_and_the_second_cancels_the_dialog() { + let (_guard, mut ws, _) = dialog_on(&["alpha"]); + send(&mut ws, KeyCode::Down); + + send(&mut ws, KeyCode::Esc); + assert!(ws.repo_input.picker.is_none()); + assert!(ws.repo_input.active, "the field survives the first Esc"); + + send(&mut ws, KeyCode::Esc); + assert!(!ws.repo_input.active); +} + +#[test] +fn j_and_k_move_the_browser_without_reaching_the_field() { + let (_guard, mut ws, text) = dialog_on(&["alpha", "zeta"]); + send(&mut ws, KeyCode::Down); + + send(&mut ws, KeyCode::Char('j')); + assert_eq!( + ws.repo_input.picker.as_ref().expect("open").selected(), + 1, + "`j` moves the cursor rather than typing a `j`" + ); + send(&mut ws, KeyCode::Char('k')); + send(&mut ws, KeyCode::Enter); + + assert_eq!(ws.repo_input.buf, format!("{text}/alpha/")); +} diff --git a/src/application/tests/search.rs b/src/application/tests/search.rs index 7c3e0c8e..bd6dfe72 100644 --- a/src/application/tests/search.rs +++ b/src/application/tests/search.rs @@ -94,3 +94,28 @@ fn handle_key_diff_next_match_from_split_returns_to_unified_when_query_exists() assert_eq!(app.diff.view, DiffPaneView::Diff); } + +#[test] +fn tab_in_the_diff_viewer_cycles_the_view() { + // Tab reaching the diff viewer at all is the point: it is not a text + // command, so it needs its own arm in the focus handler. + let mut app = app_with_files(vec!["a.rs"]); + app.focus = Focus::DiffViewer; + assert_eq!(app.diff.view, DiffPaneView::Diff); + + let _ = handle_key(&mut app, press(KeyCode::Tab, KeyModifiers::NONE)); + + assert_eq!(app.diff.view, DiffPaneView::Split); +} + +#[test] +fn tab_outside_the_diff_viewer_leaves_the_view_alone() { + // The file list owns Tab-less navigation and the terminal forwards Tab to + // its PTY; neither may reach the diff pane's cycle. + let mut app = app_with_files(vec!["a.rs"]); + app.focus = Focus::FileList; + + let _ = handle_key(&mut app, press(KeyCode::Tab, KeyModifiers::NONE)); + + assert_eq!(app.diff.view, DiffPaneView::Diff); +} diff --git a/src/application/tests/workspace.rs b/src/application/tests/workspace.rs index 948f4021..04e51887 100644 --- a/src/application/tests/workspace.rs +++ b/src/application/tests/workspace.rs @@ -58,6 +58,26 @@ fn confirming_the_dialog_asks_the_workspace_to_open_that_path() { assert!(!ws.repo_input.active, "dialog must close on success"); } +#[test] +fn the_dialog_completes_the_path_on_tab() { + // Tab used to fall through to `text_input_char`, which rejects it, so the + // key was silently swallowed. This pins the routing, not the completion + // rules — those are covered in `workspace::path_complete`. + let dir = tempfile::TempDir::new().expect("a temp dir"); + std::fs::create_dir(dir.path().join("nightcrow")).expect("create dir"); + let base = format!("{}/", dir.path().to_str().expect("a UTF-8 temp path")); + let mut ws = workspace_on(&["/a"]); + ws.start_repo_input(); + for c in format!("{base}night").chars() { + ws.repo_input_push(c); + } + + let outcome = dispatch_key(&mut ws, press(KeyCode::Tab, KeyModifiers::NONE)); + + assert_eq!(outcome, KeyOutcome::Continue); + assert_eq!(ws.repo_input.buf, format!("{base}nightcrow/")); +} + #[test] fn confirming_the_dialog_on_a_bad_path_keeps_it_open() { let mut ws = workspace_on(&["/a"]); diff --git a/src/git/diff/diff_load.rs b/src/git/diff/diff_load.rs index 99f1b755..270e34dd 100644 --- a/src/git/diff/diff_load.rs +++ b/src/git/diff/diff_load.rs @@ -238,7 +238,12 @@ fn collect_hunks( _ => LineKind::Context, }; if let Some(h) = hunks.borrow_mut().last_mut() { - h.lines.push(DiffLine { kind, content }); + h.lines.push(DiffLine { + kind, + content, + old_lineno: line.old_lineno(), + new_lineno: line.new_lineno(), + }); } true }), diff --git a/src/git/diff/snapshot.rs b/src/git/diff/snapshot.rs index e9d39500..334671e3 100644 --- a/src/git/diff/snapshot.rs +++ b/src/git/diff/snapshot.rs @@ -189,6 +189,9 @@ pub(super) fn binary_diff_hunk(file_path: &str) -> DiffHunk { lines: vec![DiffLine { kind: LineKind::Context, content: "Binary files differ".to_string(), + // Synthetic placeholder: a binary file has no line numbering. + old_lineno: None, + new_lineno: None, }], file_path: Some(file_path.to_string()), } diff --git a/src/git/diff/tests/diff_lineno.rs b/src/git/diff/tests/diff_lineno.rs new file mode 100644 index 00000000..4761af8c --- /dev/null +++ b/src/git/diff/tests/diff_lineno.rs @@ -0,0 +1,110 @@ +use crate::git::diff::{LineKind, load_commit_file_diff, load_commit_log, load_file_diff}; +use crate::test_util::{make_repo, open_repo, run_git}; +use std::path::Path; + +/// `(kind, old_lineno, new_lineno)` for every line of a hunk — the shape the +/// gutter renderer will consume. +fn gutter(hunk: &crate::git::diff::DiffHunk) -> Vec<(LineKind, Option, Option)> { + hunk.lines + .iter() + .map(|l| (l.kind, l.old_lineno, l.new_lineno)) + .collect() +} + +#[test] +fn mixed_hunk_lines_carry_the_line_numbers_of_the_side_they_exist_on() { + let (dir, path) = make_repo(); + let fp = Path::new(&path).join("m.txt"); + std::fs::write(&fp, "one\ntwo\nthree\n").unwrap(); + run_git(&path, &["add", "."]); + run_git(&path, &["commit", "-m", "init"]); + std::fs::write(&fp, "one\nTWO\nthree\n").unwrap(); + + let hunks = load_file_diff(&open_repo(&path), "m.txt").unwrap(); + + assert_eq!(hunks.len(), 1); + assert_eq!( + gutter(&hunks[0]), + vec![ + (LineKind::Context, Some(1), Some(1)), + // Removed exists only on the old side, added only on the new side. + (LineKind::Removed, Some(2), None), + (LineKind::Added, None, Some(2)), + (LineKind::Context, Some(3), Some(3)), + ] + ); + drop(dir); +} + +#[test] +fn second_hunk_resumes_from_its_own_offsets_after_an_earlier_insertion() { + let (dir, path) = make_repo(); + let fp = Path::new(&path).join("long.txt"); + let before: String = (1..=20).map(|n| format!("l{n:02}\n")).collect(); + std::fs::write(&fp, &before).unwrap(); + run_git(&path, &["add", "."]); + run_git(&path, &["commit", "-m", "init"]); + // Insert one extra line near the top and edit near the bottom, so the two + // changes land in separate hunks and the new side of the second hunk is + // offset by one from the old side. + let after: String = (1..=20) + .map(|n| match n { + 2 => "l02a\nl02b\n".to_string(), + 18 => "l18x\n".to_string(), + _ => format!("l{n:02}\n"), + }) + .collect(); + std::fs::write(&fp, &after).unwrap(); + + let hunks = load_file_diff(&open_repo(&path), "long.txt").unwrap(); + + assert_eq!(hunks.len(), 2, "expected two separate hunks"); + let second = gutter(&hunks[1]); + // First context line of the second hunk: old side 15, new side 16 because + // the earlier hunk added a line. + assert_eq!(second[0], (LineKind::Context, Some(15), Some(16))); + assert_eq!( + second, + vec![ + (LineKind::Context, Some(15), Some(16)), + (LineKind::Context, Some(16), Some(17)), + (LineKind::Context, Some(17), Some(18)), + (LineKind::Removed, Some(18), None), + (LineKind::Added, None, Some(19)), + (LineKind::Context, Some(19), Some(20)), + (LineKind::Context, Some(20), Some(21)), + ] + ); + drop(dir); +} + +#[test] +fn commit_diff_lines_carry_line_numbers_too() { + let (dir, path) = make_repo(); + let fp = Path::new(&path).join("c.txt"); + std::fs::write(&fp, "one\ntwo\n").unwrap(); + run_git(&path, &["add", "."]); + run_git(&path, &["commit", "-m", "init"]); + std::fs::write(&fp, "one\nTWO\n").unwrap(); + run_git(&path, &["add", "."]); + run_git(&path, &["commit", "-m", "edit"]); + + let commits = load_commit_log(&open_repo(&path), 1).unwrap(); + let hunks = load_commit_file_diff(&open_repo(&path), commits[0].oid, "c.txt").unwrap(); + + // The commit collector prepends a synthetic per-file header hunk; the real + // hunk follows it and must still carry libgit2's numbers. + let real = hunks + .iter() + .find(|h| h.header.starts_with("@@")) + .expect("commit diff should contain a unified hunk"); + assert_eq!( + gutter(real), + vec![ + (LineKind::Context, Some(1), Some(1)), + (LineKind::Removed, Some(2), None), + (LineKind::Added, None, Some(2)), + ] + ); + drop(dir); +} diff --git a/src/git/diff/tests/mod.rs b/src/git/diff/tests/mod.rs index d8cac34d..68cf0e7f 100644 --- a/src/git/diff/tests/mod.rs +++ b/src/git/diff/tests/mod.rs @@ -1,3 +1,4 @@ mod commit_log; +mod diff_lineno; mod diff_load; mod snapshot; diff --git a/src/git/diff/types.rs b/src/git/diff/types.rs index d236a4d3..f54df6de 100644 --- a/src/git/diff/types.rs +++ b/src/git/diff/types.rs @@ -152,6 +152,15 @@ pub enum LineKind { pub struct DiffLine { pub kind: LineKind, pub content: String, + /// Line number on the pre-image side, as reported by libgit2. `None` for an + /// added line, which exists only on the new side — so the gutter can leave + /// that column blank instead of inventing a number. Also `None` on + /// hand-built fixtures and the synthetic binary-file hunk, where no real + /// line numbering exists. + pub old_lineno: Option, + /// Line number on the post-image side. `None` for a removed line, which is + /// absent from the new side. Same `None` cases as `old_lineno` otherwise. + pub new_lineno: Option, } #[derive(Debug, Clone)] diff --git a/src/input/encode.rs b/src/input/encode.rs index 6753b27d..f2a20067 100644 --- a/src/input/encode.rs +++ b/src/input/encode.rs @@ -36,7 +36,11 @@ pub fn encode_key(key: KeyEvent) -> Option> { let mut enc = [0u8; 4]; Some(c.encode_utf8(&mut enc).as_bytes().to_vec()) } - KeyCode::Enter => Some(vec![b'\r']), + // Alt+Enter carries the Meta prefix like Alt+Char does. Terminal UIs + // read ESC+CR as "insert a newline, don't submit" — it is what Claude + // Code binds its newline to — so dropping the modifier here made the + // two indistinguishable and every Alt+Enter submitted instead. + KeyCode::Enter => Some(if alt { vec![0x1b, b'\r'] } else { vec![b'\r'] }), KeyCode::Backspace => Some(vec![0x7f]), KeyCode::Delete => Some(csi_tilde(3, key.modifiers)), KeyCode::Esc => Some(vec![0x1b]), diff --git a/src/input/tests/encode_tests.rs b/src/input/tests/encode_tests.rs index f3a8d591..3b344b0e 100644 --- a/src/input/tests/encode_tests.rs +++ b/src/input/tests/encode_tests.rs @@ -114,6 +114,16 @@ fn encode_enter_as_cr() { assert_eq!(encode_key(key(KeyCode::Enter)), Some(vec![b'\r'])); } +#[test] +fn encode_alt_enter_as_esc_cr() { + // A pane program cannot tell "newline" from "submit" if the modifier is + // dropped; ESC+CR is the Meta-prefixed form TUIs read as newline. + assert_eq!( + encode_key(KeyEvent::new(KeyCode::Enter, KeyModifiers::ALT)), + Some(vec![0x1b, b'\r']) + ); +} + #[test] fn encode_ctrl_space_as_nul() { // xterm convention: Ctrl+Space → NUL. The generic `c - '@'` formula diff --git a/src/ui/diff_pane/mod.rs b/src/ui/diff_pane/mod.rs index c5f3a3ba..022b1ecf 100644 --- a/src/ui/diff_pane/mod.rs +++ b/src/ui/diff_pane/mod.rs @@ -59,6 +59,13 @@ pub struct DiffPane { pub(crate) cached_content_bytes: usize, pub scroll: usize, pub scroll_x: usize, + /// Soft-wrap long lines instead of letting them run off the right edge. + /// + /// Mutually exclusive with horizontal scrolling by construction, not by + /// choice: ratatui's `Paragraph` ignores its `scroll.x` once wrapping is on. + /// The split view ignores this entirely — halves that wrap to different + /// heights would stop lining up, which is the whole point of that layout. + pub wrap: bool, pub search: DiffSearch, pub view: DiffPaneView, pub file_view: FileViewState, diff --git a/src/ui/diff_pane/tests/mod.rs b/src/ui/diff_pane/tests/mod.rs index 50220d01..dcc7a040 100644 --- a/src/ui/diff_pane/tests/mod.rs +++ b/src/ui/diff_pane/tests/mod.rs @@ -22,6 +22,8 @@ fn match_hunk(lines: &[&str]) -> DiffHunk { .map(|s| DiffLine { kind: LineKind::Context, content: (*s).to_string(), + old_lineno: None, + new_lineno: None, }) .collect(), file_path: None, @@ -77,6 +79,8 @@ fn kinded_hunk(lines: &[(LineKind, &str)]) -> DiffHunk { .map(|(kind, s)| DiffLine { kind: *kind, content: (*s).to_string(), + old_lineno: None, + new_lineno: None, }) .collect(), file_path: None, diff --git a/src/ui/diff_viewer/file_view.rs b/src/ui/diff_viewer/file_view.rs index ac493282..f5aa5ff7 100644 --- a/src/ui/diff_viewer/file_view.rs +++ b/src/ui/diff_viewer/file_view.rs @@ -1,10 +1,11 @@ use crate::app::{App, Focus}; +use crate::ui::jump_legend; use ratatui::{ Frame, layout::{Constraint, Direction, Layout, Rect}, style::{Color, Style}, text::{Line, Span}, - widgets::{Block, Borders, Paragraph}, + widgets::{Block, Borders}, }; use syntect::highlighting::ThemeSet; use syntect::parsing::SyntaxSet; @@ -45,7 +46,7 @@ pub(crate) fn render_file_view( (area, None) }; - let jump = super::jump_legend(app, '2'); + let jump = jump_legend(app, '2'); let title = if has_search { let count = app.diff.search.matches.len(); if count == 0 { @@ -63,6 +64,10 @@ pub(crate) fn render_file_view( let visible_height = (content_area.height as usize).saturating_sub(2); let current_match = app.diff.search.current_match(); + // An error or an empty file has no lines to number, so the gutter column is + // not reserved at all — otherwise the message would sit indented under it. + let mut gutter_lines: Vec = Vec::new(); + let mut gutter_width = 0u16; let lines: Vec = if let Some(err) = &app.diff.file_view.error { vec![Line::from(Span::styled( err.as_str(), @@ -77,7 +82,10 @@ pub(crate) fn render_file_view( app.diff.file_view.ensure_highlight_cache(ss, ts, syntax); let fv = &app.diff.file_view; let total = fv.line_count(); - let width = total.to_string().len(); + // Same floor as the diff gutters, so switching between `v` and the diff + // view does not shift the body's left edge. + let digits = super::gutter::digits_for(total); + gutter_width = super::gutter::side_gutter_width(digits); // Belt-and-braces: ensure_highlight_cache keeps line_highlights // aligned with content.lines().count(), but if that invariant ever // slips the slice below would panic. Clamp against the cache length. @@ -107,30 +115,42 @@ pub(crate) fn render_file_view( } else { Color::Reset }; - let mut spans = vec![Span::styled( - format!(" {:>width$} ", line_no, width = width), + // The number lives in its own paragraph so horizontal scrolling + // cannot slide it off the left edge, which is what used to + // happen while it shared the body's paragraph. + gutter_lines.push(Line::from(Span::styled( + super::gutter::side_gutter_text(Some(line_no as u32), digits), Style::default().fg(Color::DarkGray).bg(bg), - )]; - for seg in segs { - spans.push(Span::styled( - seg.text.as_str(), - Style::default().fg(super::rgb_to_color(seg.rgb)).bg(bg), - )); - } + ))); + let spans: Vec = segs + .iter() + .map(|seg| { + Span::styled( + seg.text.as_str(), + Style::default().fg(super::rgb_to_color(seg.rgb)).bg(bg), + ) + }) + .collect(); Line::from(spans) }) .collect() }; - let para = Paragraph::new(lines) - .block( - Block::default() - .borders(Borders::ALL) - .title(title) - .border_style(border_style), - ) - .scroll((0, app.diff.file_view.scroll_x.min(u16::MAX as usize) as u16)); - frame.render_widget(para, content_area); + let block = Block::default() + .borders(Borders::ALL) + .title(title) + .border_style(border_style); + let inner = block.inner(content_area); + frame.render_widget(block, content_area); + super::gutter::render_gutter_and_body( + frame, + inner, + gutter_width, + gutter_lines, + lines, + app.diff.file_view.scroll_x.min(u16::MAX as usize) as u16, + app.diff.wrap, + ); if let Some(sa) = search_area { super::render_search_bar( diff --git a/src/ui/diff_viewer/gutter.rs b/src/ui/diff_viewer/gutter.rs new file mode 100644 index 00000000..5eb4dbc1 --- /dev/null +++ b/src/ui/diff_viewer/gutter.rs @@ -0,0 +1,136 @@ +use crate::git::diff::DiffHunk; +use ratatui::{ + Frame, + layout::{Constraint, Direction, Layout, Rect}, + text::Line, + widgets::{Paragraph, Wrap}, +}; + +/// Minimum digits reserved for one line-number column. Keeps the gutter — and +/// with it the body's left edge — from twitching between a 1-digit and a +/// 2-digit file, the way `delta`/`diff-so-fancy` reserve a fixed column. +const MIN_LINENO_DIGITS: usize = 3; + +/// One padding space on each side of a number column: it lifts the digits off +/// the pane border and off the `+`/`-` marker that follows. +const LINENO_PAD: usize = 2; + +/// Single space separating the old and new columns of the unified gutter. +const LINENO_GAP: usize = 1; + +/// Digits needed to print `max_lineno`, floored at `MIN_LINENO_DIGITS`. +pub(crate) fn digits_for(max_lineno: usize) -> usize { + let digits = if max_lineno == 0 { + 1 + } else { + max_lineno.ilog10() as usize + 1 + }; + digits.max(MIN_LINENO_DIGITS) +} + +/// Gutter digit count for a whole loaded diff: the widest line number that +/// appears on either side of any hunk. Derived from the loaded hunks, never +/// from the visible window, so scrolling cannot change the gutter width and +/// shift the body sideways mid-scroll. +/// +/// Recomputed per frame instead of cached: it is one allocation-free pass over +/// the same lines `ensure_highlight_cache` already walks for its fingerprint, +/// and a cache would need invalidating at every `DiffPane::hunks` mutation — +/// a missed one renders numbers against the wrong column width. +pub(crate) fn lineno_digits(hunks: &[DiffHunk]) -> usize { + let max = hunks + .iter() + .flat_map(|h| h.lines.iter()) + // `Option::max` picks the larger `Some`; both `None` only on fixtures + // and the synthetic binary hunk, which then fall back to the minimum. + .filter_map(|l| l.old_lineno.max(l.new_lineno)) + .max() + .unwrap_or(0); + digits_for(max as usize) +} + +/// Width of the unified gutter, which shows the old and new columns together. +pub(crate) fn unified_gutter_width(digits: usize) -> u16 { + (2 * digits + LINENO_GAP + LINENO_PAD) as u16 +} + +/// Width of a one-column gutter (each split half, and the file view). +pub(crate) fn side_gutter_width(digits: usize) -> u16 { + (digits + LINENO_PAD) as u16 +} + +/// `" old new "`, with either column blank when the line is absent on that +/// side (added lines have no old number, removed lines have no new one). +pub(crate) fn unified_gutter_text(old: Option, new: Option, digits: usize) -> String { + format!( + " {:>digits$} {:>digits$} ", + lineno_text(old), + lineno_text(new) + ) +} + +/// `" n "` for a single-column gutter; all spaces when `no` is `None`. +pub(crate) fn side_gutter_text(no: Option, digits: usize) -> String { + format!(" {:>digits$} ", lineno_text(no)) +} + +fn lineno_text(no: Option) -> String { + no.map(|v| v.to_string()).unwrap_or_default() +} + +/// Render a pinned gutter column and a horizontally scrollable body inside +/// `inner` (a `Block`'s inner area — draw the block yourself first). +/// +/// The two are separate `Paragraph`s: `Paragraph::scroll` shifts the whole +/// line, so a gutter span living in the body's paragraph would slide off the +/// left edge as soon as `scroll_x > 0`. Vertical scroll is instead expressed +/// by *which* lines the caller collected, so passing windows built from the +/// same rows is what keeps numbers aligned with their content. +/// +/// With `wrap` set that split is abandoned for the opposite reason: a wrapped +/// body line occupies several screen rows while its gutter line still occupies +/// one, which would desynchronise every row below it. The number is folded into +/// the body line instead, where wrapping carries it along. That is safe only +/// because wrapping and horizontal scrolling cannot both be active. +pub(crate) fn render_gutter_and_body( + frame: &mut Frame, + inner: Rect, + gutter_width: u16, + gutter: Vec>, + body: Vec>, + scroll_x: u16, + wrap: bool, +) { + if wrap { + frame.render_widget( + // `trim: false` keeps a continuation row's leading whitespace, which + // in source code is the indentation. + Paragraph::new(merge_gutter_into_body(gutter, body)).wrap(Wrap { trim: false }), + inner, + ); + return; + } + let cols = Layout::default() + .direction(Direction::Horizontal) + .constraints([Constraint::Length(gutter_width), Constraint::Min(0)]) + .split(inner); + frame.render_widget(Paragraph::new(gutter), cols[0]); + frame.render_widget(Paragraph::new(body).scroll((0, scroll_x)), cols[1]); +} + +/// Prepend each gutter line's spans to the body line it belongs to. The two +/// vectors are built in lockstep by the callers, so index `i` pairs row `i`; a +/// body row with no gutter entry simply keeps its own spans. +fn merge_gutter_into_body<'a>(gutter: Vec>, body: Vec>) -> Vec> { + let mut gutter = gutter.into_iter(); + body.into_iter() + .map(|line| match gutter.next() { + Some(g) => { + let mut spans = g.spans; + spans.extend(line.spans); + Line::from(spans) + } + None => line, + }) + .collect() +} diff --git a/src/ui/diff_viewer/mod.rs b/src/ui/diff_viewer/mod.rs index f55fa575..72f9ffef 100644 --- a/src/ui/diff_viewer/mod.rs +++ b/src/ui/diff_viewer/mod.rs @@ -1,26 +1,38 @@ +#[cfg(test)] +mod tests; + mod file_view; +mod gutter; mod split_view; +mod title; pub(crate) use file_view::render_file_view; pub(crate) use split_view::render_split_view; use crate::app::{App, DiffPaneView, Focus, ViewMode}; use crate::git::diff::LineKind; -use crate::ui::{focused_border_style, jump_legend, path_extension, render_search_bar}; +use crate::ui::{focused_border_style, path_extension, render_search_bar}; +use gutter::{lineno_digits, render_gutter_and_body, unified_gutter_text, unified_gutter_width}; use ratatui::{ Frame, layout::{Constraint, Direction, Layout, Rect}, style::{Color, Style}, text::{Line, Span}, - widgets::{Block, Borders, Paragraph}, + widgets::{Block, Borders}, }; use syntect::highlighting::ThemeSet; use syntect::parsing::SyntaxSet; +use title::unified_title; /// Minimum pane width (columns) for the side-by-side split layout. Below this /// each half is too narrow to read, so `Split` view falls back to the unified /// diff renderer. -const MIN_SPLIT_WIDTH: u16 = 80; +/// +/// Derived: 80 columns used to leave each half ~38 columns of code, and each +/// half now spends `side_gutter_width(MIN_LINENO_DIGITS)` = 5 of them on its +/// line-number gutter. Raising the threshold by both gutters keeps the same +/// readable code width per side rather than silently shrinking it. +const MIN_SPLIT_WIDTH: u16 = 90; pub(crate) fn rgb_to_color(rgb: (u8, u8, u8)) -> Color { Color::Rgb(rgb.0, rgb.1, rgb.2) @@ -81,7 +93,21 @@ pub fn render( app.diff.scroll = scroll_start; let visible_end = scroll_start.saturating_add(visible_height); + // Gutter width is a property of the whole loaded diff, not of the visible + // window, so the body's left edge stays put while scrolling. With no diff + // loaded the pane holds only a placeholder message, which has no line to + // number — reserving the column there would just indent the message. + let digits = lineno_digits(&app.diff.hunks); + let gutter_width = if total_lines == 0 { + 0 + } else { + unified_gutter_width(digits) + }; + let mut lines: Vec = Vec::with_capacity(visible_height); + // Collected in lockstep with `lines`: same rows, same order, so the two + // paragraphs share one vertical window. + let mut gutter_lines: Vec = Vec::with_capacity(visible_height); let mut flat_idx: usize = 0; 'outer: for (hi, hunk) in app.diff.hunks.iter().enumerate() { @@ -95,6 +121,9 @@ pub fn render( hunk.header.as_str(), Style::default().fg(Color::Cyan), ))); + // Blank, but full width: a header row with no gutter cell would + // start its `@@` one column left of the body's left edge. + gutter_lines.push(Line::from("")); } flat_idx += 1; @@ -151,6 +180,12 @@ pub fn render( } lines.push(Line::from(spans)); + // Same background as the row so the number reads as part of it + // rather than as a column floating beside the highlight. + gutter_lines.push(Line::from(Span::styled( + unified_gutter_text(diff_line.old_lineno, diff_line.new_lineno, digits), + Style::default().fg(Color::DarkGray).bg(bg), + ))); flat_idx += 1; } } @@ -182,75 +217,23 @@ pub fn render( ))); } - let jump = jump_legend(app, '2'); - let title = match app.mode { - ViewMode::Log => { - let label = if app.log_view.diff_title.is_empty() { - "Diff" - } else { - app.log_view.diff_title.as_str() - }; - if has_search { - let count = app.diff.search.matches.len(); - if count == 0 { - format!(" {jump} {label} [no matches] ") - } else { - format!( - " {jump} {label} [{}/{}] ", - app.diff.search.cursor + 1, - count - ) - } - } else { - format!(" {jump} {label} ") - } - } - ViewMode::Status => { - let selected = app.selected_filtered_status_file(); - if has_search { - let count = app.diff.search.matches.len(); - let file = selected.map(|f| f.path.as_str()).unwrap_or("Diff"); - if count == 0 { - format!(" {jump} {file} [no matches] ") - } else { - format!(" {jump} {file} [{}/{}] ", app.diff.search.cursor + 1, count) - } - } else if let Some(f) = selected { - format!(" {jump} {} ", f.path) - } else { - format!(" {jump} Diff ") - } - } - ViewMode::Tree => { - let path = app.tree_view.selected_path(); - let label = path.as_deref().unwrap_or("File"); - if has_search { - let count = app.diff.search.matches.len(); - if count == 0 { - format!(" {jump} {label} [no matches] ") - } else { - format!( - " {jump} {label} [{}/{}] ", - app.diff.search.cursor + 1, - count - ) - } - } else { - format!(" {jump} {label} ") - } - } - }; - - let para = Paragraph::new(lines) - .block( - Block::default() - .borders(Borders::ALL) - .title(title) - .border_style(border_style), - ) - .scroll((0, app.diff.scroll_x.min(u16::MAX as usize) as u16)); - - frame.render_widget(para, diff_area); + // The block is rendered on its own rather than attached to a paragraph: + // the gutter and the body are two paragraphs sharing one bordered area. + let block = Block::default() + .borders(Borders::ALL) + .title(unified_title(app)) + .border_style(border_style); + let inner = block.inner(diff_area); + frame.render_widget(block, diff_area); + render_gutter_and_body( + frame, + inner, + gutter_width, + gutter_lines, + lines, + app.diff.scroll_x.min(u16::MAX as usize) as u16, + app.diff.wrap, + ); if let Some(sa) = search_area { render_search_bar( diff --git a/src/ui/diff_viewer/split_view.rs b/src/ui/diff_viewer/split_view.rs index b122cfeb..ac3bc863 100644 --- a/src/ui/diff_viewer/split_view.rs +++ b/src/ui/diff_viewer/split_view.rs @@ -1,4 +1,4 @@ -use crate::app::{App, Focus, ViewMode}; +use crate::app::{App, Focus}; use crate::git::diff::LineKind; use crate::ui::diff_pane::SplitRow; use ratatui::{ @@ -6,7 +6,7 @@ use ratatui::{ layout::{Constraint, Direction, Layout, Rect}, style::{Color, Style}, text::{Line, Span}, - widgets::{Block, Borders, Paragraph}, + widgets::{Block, Borders}, }; use syntect::highlighting::ThemeSet; use syntect::parsing::SyntaxSet; @@ -35,8 +35,16 @@ pub(crate) fn render_split_view( app.diff.scroll = scroll_start; let scroll_end = scroll_start.saturating_add(visible_height).min(rows.len()); + // Each half carries the number of the side it shows: old on the left, new + // on the right. Collected in lockstep with the body lines so the two + // paragraphs of a half share one vertical window. + let digits = super::gutter::lineno_digits(&app.diff.hunks); + let gutter_width = super::gutter::side_gutter_width(digits); + let mut left_lines: Vec = Vec::with_capacity(visible_height); let mut right_lines: Vec = Vec::with_capacity(visible_height); + let mut left_gutter: Vec = Vec::with_capacity(visible_height); + let mut right_gutter: Vec = Vec::with_capacity(visible_height); for row in &rows[scroll_start..scroll_end] { match row { SplitRow::Header(hi) => { @@ -51,15 +59,23 @@ pub(crate) fn render_split_view( Style::default().fg(Color::Cyan), ))); right_lines.push(Line::from("")); + // Blank but present: a header with no gutter cell would start + // one column left of the body beneath it. + left_gutter.push(Line::from("")); + right_gutter.push(Line::from("")); } SplitRow::Body { left, right } => { - left_lines.push(split_side_line(app, *left)); - right_lines.push(split_side_line(app, *right)); + let (lg, lb) = split_side_lines(app, *left, digits, Side::Old); + let (rg, rb) = split_side_lines(app, *right, digits, Side::New); + left_lines.push(lb); + right_lines.push(rb); + left_gutter.push(lg); + right_gutter.push(rg); } } } - let title = split_title(app); + let title = super::title::split_title(app); let block = Block::default() .borders(Borders::ALL) .title(title) @@ -73,31 +89,62 @@ pub(crate) fn render_split_view( .split(inner); let scroll_x = app.diff.scroll_x.min(u16::MAX as usize) as u16; - let left_para = Paragraph::new(left_lines).scroll((0, scroll_x)); - // A left border on the right column draws the vertical divider between - // the two halves and indents the new-side content by one cell. - let right_para = Paragraph::new(right_lines) - .block( - Block::default() - .borders(Borders::LEFT) - .border_style(border_style), - ) - .scroll((0, scroll_x)); + super::gutter::render_gutter_and_body( + frame, + halves[0], + gutter_width, + left_gutter, + left_lines, + scroll_x, + // Wrapping is deliberately ignored here: halves that fold to different + // heights stop lining up, and lining up is what this layout is for. + false, + ); + + // A left border on the right column draws the vertical divider between the + // two halves and indents the new-side content by one cell. It is rendered + // on its own so the gutter and body can split the area inside it. + let right_block = Block::default() + .borders(Borders::LEFT) + .border_style(border_style); + let right_inner = right_block.inner(halves[1]); + frame.render_widget(right_block, halves[1]); + super::gutter::render_gutter_and_body( + frame, + right_inner, + gutter_width, + right_gutter, + right_lines, + scroll_x, + false, + ); +} - frame.render_widget(left_para, halves[0]); - frame.render_widget(right_para, halves[1]); +/// Which side's line number a half shows. +enum Side { + Old, + New, } -/// Build one side's `Line` for a split body row. `None` (no counterpart line -/// on this side) renders as a blank line; otherwise the cell is styled by line -/// kind and reuses the prebuilt highlight cache, mirroring the unified -/// renderer's per-line treatment. -fn split_side_line<'a>(app: &'a App, cell: Option<(usize, usize)>) -> Line<'a> { +/// Build one side's gutter and body `Line` for a split body row, as +/// `(gutter, body)`. `None` (no counterpart line on this side) renders both as +/// blank; otherwise the cell is styled by line kind and reuses the prebuilt +/// highlight cache, mirroring the unified renderer's per-line treatment. +/// +/// Both lines come from one lookup so they cannot disagree about which +/// `DiffLine` the row is showing. +fn split_side_lines<'a>( + app: &'a App, + cell: Option<(usize, usize)>, + digits: usize, + side: Side, +) -> (Line<'a>, Line<'a>) { + let blank = || (Line::from(""), Line::from("")); let Some((hi, li)) = cell else { - return Line::from(""); + return blank(); }; let Some(diff_line) = app.diff.hunks.get(hi).and_then(|h| h.lines.get(li)) else { - return Line::from(""); + return blank(); }; let bg = match diff_line.kind { @@ -128,29 +175,14 @@ fn split_side_line<'a>(app: &'a App, cell: Option<(usize, usize)>) -> Line<'a> { Style::default().bg(bg), )); } - Line::from(spans) -} -/// Title for the split pane: the same file/commit label the unified view uses, -/// tagged `[split]`. Search match counts are omitted because the split view -/// does not render search highlights. -fn split_title(app: &App) -> String { - let label = match app.mode { - ViewMode::Log => { - if app.log_view.diff_title.is_empty() { - "Diff".to_string() - } else { - app.log_view.diff_title.clone() - } - } - ViewMode::Status => app - .selected_filtered_status_file() - .map(|f| f.path.clone()) - .unwrap_or_else(|| "Diff".to_string()), - ViewMode::Tree => app - .tree_view - .selected_path() - .unwrap_or_else(|| "File".to_string()), + let lineno = match side { + Side::Old => diff_line.old_lineno, + Side::New => diff_line.new_lineno, }; - format!(" {} {label} [split] ", super::jump_legend(app, '2')) + let gutter = Line::from(Span::styled( + super::gutter::side_gutter_text(lineno, digits), + Style::default().fg(Color::DarkGray).bg(bg), + )); + (gutter, Line::from(spans)) } diff --git a/src/ui/diff_viewer/tests/file_view.rs b/src/ui/diff_viewer/tests/file_view.rs new file mode 100644 index 00000000..0ef425a7 --- /dev/null +++ b/src/ui/diff_viewer/tests/file_view.rs @@ -0,0 +1,44 @@ +use super::*; + +/// Regression: the file view already had a gutter, but it shared the body's +/// paragraph, so scrolling sideways carried the numbers off the left edge. +#[test] +fn file_view_line_numbers_stay_put_when_the_body_scrolls_sideways() { + let mut app = app_with_files(vec!["src/lib.rs"]); + app.mode = ViewMode::Status; + app.diff.view = DiffPaneView::File; + app.diff.file_view.key = Some(crate::app::FileViewKey::Status("src/lib.rs".to_string())); + app.diff.file_view.content = + "fn first() { let a_long_identifier = 1; }\nfn second() {}\n".to_string(); + + let unscrolled = drawn_file_view(&mut app, 60, 8, 0); + let scrolled = drawn_file_view(&mut app, 60, 8, 12); + + for screen in [&unscrolled, &scrolled] { + assert!( + screen.iter().any(|l| l.contains('1')) && screen.iter().any(|l| l.contains('2')), + "both line numbers must be on screen, got:\n{screen:#?}" + ); + } + assert_ne!( + unscrolled, scrolled, + "the body should actually have scrolled" + ); + assert_eq!( + unscrolled + .iter() + .filter_map(|l| l.find(" 1 ").map(|_| ())) + .count(), + scrolled + .iter() + .filter_map(|l| l.find(" 1 ").map(|_| ())) + .count(), + "the numbered gutter column must survive the scroll:\n{scrolled:#?}" + ); +} + +/// The file view reads its own horizontal offset, not `diff.scroll_x`. +fn drawn_file_view(app: &mut App, width: u16, height: u16, scroll_x: usize) -> Vec { + app.diff.file_view.scroll_x = scroll_x; + drawn(app, width, height, 0) +} diff --git a/src/ui/diff_viewer/tests/mod.rs b/src/ui/diff_viewer/tests/mod.rs new file mode 100644 index 00000000..c89ca3dc --- /dev/null +++ b/src/ui/diff_viewer/tests/mod.rs @@ -0,0 +1,124 @@ +//! Line-number gutter rendering. +//! +//! The gutter and the diff body are deliberately two `Paragraph`s: a single +//! paragraph would slide the numbers off the left edge as soon as the body is +//! scrolled horizontally. Several of these tests exist only to keep that from +//! regressing, so they assert on a rendered screen rather than on a helper. + +use crate::app::tests::app_with_files; +use crate::app::{App, DiffPaneView, ViewMode}; +use crate::git::diff::{DiffHunk, DiffLine, LineKind}; +use ratatui::{Terminal, backend::TestBackend, layout::Rect, style::Color}; +use syntect::highlighting::ThemeSet; + +/// A context / removed / added trio, which is the shape that exercises every +/// gutter column state: both numbers, old only, new only. +fn trio_hunk() -> DiffHunk { + DiffHunk { + header: "@@ -41,3 +41,3 @@".to_string(), + lines: vec![ + DiffLine { + kind: LineKind::Context, + content: "keep_me();".to_string(), + old_lineno: Some(41), + new_lineno: Some(41), + }, + DiffLine { + kind: LineKind::Removed, + content: "gone();".to_string(), + old_lineno: Some(42), + new_lineno: None, + }, + DiffLine { + kind: LineKind::Added, + content: "fresh();".to_string(), + old_lineno: None, + new_lineno: Some(42), + }, + ], + file_path: Some("src/lib.rs".to_string()), + } +} + +fn app_showing(hunk: DiffHunk, view: DiffPaneView) -> App { + let mut app = app_with_files(vec!["src/lib.rs"]); + app.mode = ViewMode::Status; + app.diff.hunks = vec![hunk]; + app.diff.view = view; + app +} + +/// Screen column of `needle`. `str::find` yields a byte offset, and the pane +/// border is a 3-byte `│`, so byte offsets are not columns here. +fn col_of(line: &str, needle: &str) -> usize { + let byte = line + .find(needle) + .unwrap_or_else(|| panic!("{needle:?} not in {line:?}")); + line[..byte].chars().count() +} + +/// The `n` leftmost columns of a rendered row, counted in characters. +fn left_columns(line: &str, n: usize) -> String { + line.chars().take(n).collect() +} + +/// Everything from column `n` rightwards, counted in characters. +fn right_columns(line: &str, n: usize) -> String { + line.chars().skip(n).collect() +} + +/// Render the diff pane on its own and return the screen as lines of text. +fn drawn(app: &mut App, width: u16, height: u16, scroll_x: usize) -> Vec { + app.diff.scroll_x = scroll_x; + let mut terminal = Terminal::new(TestBackend::new(width, height)).expect("a terminal"); + let ss = two_face::syntax::extra_newlines(); + let ts = ThemeSet::load_defaults(); + terminal + .draw(|frame| { + super::render( + frame, + app, + Rect::new(0, 0, width, height), + &ss, + &ts, + Color::Yellow, + ); + }) + .expect("draw"); + let buf = terminal.backend().buffer(); + (0..buf.area.height) + .map(|y| { + (0..buf.area.width) + .map(|x| buf[(x, y)].symbol()) + .collect::() + }) + .collect() +} +/// The split view pairs a removed line with its added counterpart on one row, +/// so this fixture gives the two sides *different* numbers — with both at 42 +/// the test could not tell an old column from a new one. +fn skewed_pair_hunk() -> DiffHunk { + DiffHunk { + header: "@@ -42 +77 @@".to_string(), + lines: vec![ + DiffLine { + kind: LineKind::Removed, + content: "gone();".to_string(), + old_lineno: Some(42), + new_lineno: None, + }, + DiffLine { + kind: LineKind::Added, + content: "fresh();".to_string(), + old_lineno: None, + new_lineno: Some(77), + }, + ], + file_path: Some("src/lib.rs".to_string()), + } +} + +mod file_view; +mod split; +mod unified; +mod wrap; diff --git a/src/ui/diff_viewer/tests/split.rs b/src/ui/diff_viewer/tests/split.rs new file mode 100644 index 00000000..ddaa7e19 --- /dev/null +++ b/src/ui/diff_viewer/tests/split.rs @@ -0,0 +1,37 @@ +use super::*; + +#[test] +fn split_halves_number_the_side_each_one_shows() { + let mut app = app_showing(skewed_pair_hunk(), DiffPaneView::Split); + + // Wide enough to clear MIN_SPLIT_WIDTH, or the renderer falls back to + // unified and this would silently test the wrong layout. + let screen = drawn(&mut app, 120, 10, 0); + let joined = screen.join("\n"); + assert!( + joined.contains("[split]"), + "the split layout must actually be in use, got:\n{joined}" + ); + + // One row, both sides: the removed line on the left, its replacement on the + // right. That pairing is what the split view is for. + let row = screen + .iter() + .find(|l| l.contains("gone")) + .expect("the paired change row"); + assert!( + row.contains("fresh"), + "the split view pairs the removal with its replacement on one row: {row:?}" + ); + + let mid = 120 / 2; + let (left, right) = (left_columns(row, mid), right_columns(row, mid)); + assert!( + left.contains("42") && !left.contains("77"), + "the left half carries only the old-side number: {left:?}" + ); + assert!( + right.contains("77") && !right.contains("42"), + "the right half carries only the new-side number: {right:?}" + ); +} diff --git a/src/ui/diff_viewer/tests/unified.rs b/src/ui/diff_viewer/tests/unified.rs new file mode 100644 index 00000000..3ba41e93 --- /dev/null +++ b/src/ui/diff_viewer/tests/unified.rs @@ -0,0 +1,108 @@ +use super::*; + +#[test] +fn unified_gutter_numbers_each_line_on_the_side_it_exists_on() { + let mut app = app_showing(trio_hunk(), DiffPaneView::Diff); + + let screen = drawn(&mut app, 60, 10, 0); + let body: Vec<&String> = screen.iter().filter(|l| l.contains("();")).collect(); + + let context = body + .iter() + .find(|l| l.contains("keep_me")) + .expect("the context row"); + let removed = body + .iter() + .find(|l| l.contains("gone")) + .expect("the removed row"); + let added = body + .iter() + .find(|l| l.contains("fresh")) + .expect("the added row"); + + assert!( + context.contains("41") && context.matches("41").count() == 2, + "a context line exists on both sides, so both columns carry 41: {context:?}" + ); + assert!( + removed.contains("42"), + "a removed line keeps its old number: {removed:?}" + ); + assert!( + removed.matches("42").count() == 1, + "a removed line has no new-side number: {removed:?}" + ); + assert!( + added.matches("42").count() == 1, + "an added line has only a new-side number: {added:?}" + ); +} + +/// The reason the gutter is a separate paragraph at all. +#[test] +fn unified_gutter_stays_put_when_the_body_scrolls_sideways() { + let mut app = app_showing(trio_hunk(), DiffPaneView::Diff); + + let unscrolled = drawn(&mut app, 60, 10, 0); + let scrolled = drawn(&mut app, 60, 10, 6); + + let numbers_before = unscrolled.iter().filter(|l| l.contains("41")).count(); + let numbers_after = scrolled.iter().filter(|l| l.contains("41")).count(); + assert_eq!( + numbers_before, numbers_after, + "the gutter must survive horizontal scroll, got:\n{scrolled:#?}" + ); + assert!( + scrolled.iter().any(|l| !l.contains("keep_me()")), + "the body should actually have scrolled, got:\n{scrolled:#?}" + ); +} + +#[test] +fn a_hunk_header_reserves_the_same_gutter_width_as_the_body() { + let mut app = app_showing(trio_hunk(), DiffPaneView::Diff); + + let screen = drawn(&mut app, 60, 10, 0); + let header = screen + .iter() + .find(|l| l.contains("@@")) + .expect("the hunk header row"); + let body = screen + .iter() + .find(|l| l.contains("keep_me")) + .expect("the context row"); + + // The body carries a `+`/`-`/space kind marker that the header does not, so + // aligned means "one column apart", not "equal". Asserting the relationship + // rather than a literal column keeps this from pinning the gutter's width. + assert_eq!( + col_of(header, "@@") + 1, + col_of(body, "keep_me"), + "header and body must start from the same gutter edge:\nheader {header:?}\nbody {body:?}" + ); + assert!( + col_of(header, "@@") > 1, + "the header must clear the reserved gutter rather than starting against \ + the border, got column {} in {header:?}", + col_of(header, "@@") + ); +} + +#[test] +fn an_empty_diff_reserves_no_gutter_for_its_placeholder() { + let mut app = app_with_files(vec![]); + app.mode = ViewMode::Status; + + let screen = drawn(&mut app, 60, 10, 0); + let msg = screen + .iter() + .find(|l| l.contains("No changes")) + .expect("the placeholder row"); + + assert_eq!( + col_of(msg, "No changes"), + 1, + "with nothing to number the message sits against the border, not \ + indented under an empty gutter: {msg:?}" + ); +} diff --git a/src/ui/diff_viewer/tests/wrap.rs b/src/ui/diff_viewer/tests/wrap.rs new file mode 100644 index 00000000..a1b3f5f3 --- /dev/null +++ b/src/ui/diff_viewer/tests/wrap.rs @@ -0,0 +1,94 @@ +use super::*; + +/// One context line far wider than any pane this test renders into. +fn long_line_hunk() -> DiffHunk { + DiffHunk { + header: "@@ -7,1 +7,1 @@".to_string(), + lines: vec![DiffLine { + kind: LineKind::Context, + content: "alpha bravo charlie delta echo foxtrot golf hotel india juliett".to_string(), + old_lineno: Some(7), + new_lineno: Some(7), + }], + file_path: Some("src/lib.rs".to_string()), + } +} + +/// Rows that carry any of the line's words, i.e. how many screen rows the one +/// logical line ended up occupying. +fn rows_with_content(screen: &[String]) -> usize { + screen + .iter() + .filter(|l| { + ["alpha", "charlie", "foxtrot", "india", "juliett"] + .iter() + .any(|w| l.contains(w)) + }) + .count() +} + +#[test] +fn wrapping_folds_a_long_line_onto_several_rows() { + let mut app = app_showing(long_line_hunk(), DiffPaneView::Diff); + + let truncated = drawn(&mut app, 40, 10, 0); + app.diff.wrap = true; + let wrapped = drawn(&mut app, 40, 10, 0); + + assert_eq!( + rows_with_content(&truncated), + 1, + "unwrapped, the line is clipped to one row:\n{truncated:#?}" + ); + assert!( + rows_with_content(&wrapped) > 1, + "wrapped, it must continue onto further rows:\n{wrapped:#?}" + ); + assert!( + wrapped.iter().any(|l| l.contains("juliett")), + "the tail must become reachable without scrolling:\n{wrapped:#?}" + ); +} + +/// With wrapping on the gutter is folded into the body line, because a separate +/// gutter paragraph would desynchronise the moment a line spans two rows. +#[test] +fn wrapping_keeps_the_line_number_on_the_row_the_line_starts_on() { + let mut app = app_showing(long_line_hunk(), DiffPaneView::Diff); + app.diff.wrap = true; + + let screen = drawn(&mut app, 40, 10, 0); + let first = screen + .iter() + .find(|l| l.contains("alpha")) + .expect("the row the line starts on"); + + assert!( + first.contains('7'), + "the number travels with its own line: {first:?}" + ); + let continuation = screen + .iter() + .find(|l| l.contains("juliett") && !l.contains("alpha")) + .expect("a continuation row"); + assert!( + !continuation.contains('7'), + "a continuation row must not be numbered again: {continuation:?}" + ); +} + +#[test] +fn the_split_view_ignores_wrapping() { + // Halves that fold to different heights would stop lining up, and lining up + // is the only reason to be in this layout. + let mut app = app_showing(long_line_hunk(), DiffPaneView::Split); + app.diff.wrap = true; + + let screen = drawn(&mut app, 120, 10, 0); + + assert_eq!( + rows_with_content(&screen), + 1, + "the split row stays clipped to one row:\n{screen:#?}" + ); +} diff --git a/src/ui/diff_viewer/title.rs b/src/ui/diff_viewer/title.rs new file mode 100644 index 00000000..a5dbe553 --- /dev/null +++ b/src/ui/diff_viewer/title.rs @@ -0,0 +1,52 @@ +use crate::app::{App, ViewMode}; +use crate::ui::jump_legend; + +/// What the diff pane is showing right now: the commit's diff title in log +/// mode, the selected path in status/tree mode, with per-mode fallbacks for +/// "nothing selected". +fn diff_label(app: &App) -> String { + match app.mode { + ViewMode::Log => { + if app.log_view.diff_title.is_empty() { + "Diff".to_string() + } else { + app.log_view.diff_title.clone() + } + } + ViewMode::Status => app + .selected_filtered_status_file() + .map(|f| f.path.clone()) + .unwrap_or_else(|| "Diff".to_string()), + ViewMode::Tree => app + .tree_view + .selected_path() + .unwrap_or_else(|| "File".to_string()), + } +} + +/// Title for the unified diff pane: jump legend, label, and the search match +/// counter while a query is active. +pub(crate) fn unified_title(app: &App) -> String { + let jump = jump_legend(app, '2'); + let label = diff_label(app); + if !app.diff.search.has_query() { + return format!(" {jump} {label} "); + } + let count = app.diff.search.matches.len(); + if count == 0 { + format!(" {jump} {label} [no matches] ") + } else { + format!( + " {jump} {label} [{}/{}] ", + app.diff.search.cursor + 1, + count + ) + } +} + +/// Title for the split pane: the same label, tagged `[split]`. Search match +/// counts are omitted because the split view does not render search +/// highlights. +pub(crate) fn split_title(app: &App) -> String { + format!(" {} {} [split] ", jump_legend(app, '2'), diff_label(app)) +} diff --git a/src/ui/hint_bar.rs b/src/ui/hint_bar.rs index a700e1f2..e2ee5362 100644 --- a/src/ui/hint_bar.rs +++ b/src/ui/hint_bar.rs @@ -3,6 +3,7 @@ use crate::ui::chrome::{Chrome, chrome_rows}; use crate::ui::hint_text::{ EMPTY_HINT, EMPTY_HINT_ARMED, PREFIX_CHIP, normal_hint_literal, prefix_armed_hint_text, }; +use crate::ui::status_view::RepoInput; use ratatui::{ layout::{Position, Rect}, style::{Color, Modifier, Style}, @@ -83,19 +84,47 @@ pub(crate) fn hint_spans(text: &str, leader: &str, mark_clickable: bool) -> Vec< spans } +/// The dialog's input line, with its keys spelled out after the caret. Nothing +/// else advertises them: the dialog replaces the hint legend entirely, so a key +/// missing from this line cannot be found anywhere on screen. +/// +/// The legend is dropped whole when the path leaves no room for it, rather than +/// clipped — the caret has to stay visible, and half a legend reads as a glitch. +/// `width` is the hint row's; 0 means "unknown", which keeps the legend. +pub(crate) fn repo_input_line<'a>( + repo_input: &'a RepoInput, + accent: Color, + width: u16, +) -> Line<'a> { + const PROMPT: &str = "repo: "; + let legend = if repo_input.picker.is_some() { + " ↓↑/jk: move | →: open | ←: up | enter: select | esc: back" + } else { + " ↓: browse | tab: complete | enter: open | esc: cancel" + }; + let mut spans = vec![ + Span::styled(PROMPT, Style::default().fg(accent)), + Span::raw(repo_input.buf.as_str()), + Span::styled("█", Style::default().fg(accent)), + ]; + // Display columns, not bytes: a path can hold wide or combining characters. + let used: usize = spans.iter().map(Span::width).sum(); + if width == 0 || used + Span::raw(legend).width() <= usize::from(width) { + spans.push(Span::styled(legend, Style::default().fg(Color::DarkGray))); + } + Line::from(spans) +} + pub(crate) fn render_hint_bar<'a>( app: &'a App, chrome: Chrome<'a>, accent: Color, + width: u16, ) -> Paragraph<'a> { if chrome.repo_input.active { // A rejected path is reported on the notice row above, so this row - // stays a plain input line. - return Paragraph::new(Line::from(vec![ - Span::styled("repo: ", Style::default().fg(accent)), - Span::raw(chrome.repo_input.buf.as_str()), - Span::styled("█", Style::default().fg(accent)), - ])); + // stays the input line plus its own legend. + return Paragraph::new(repo_input_line(chrome.repo_input, accent, width)); } if app.prefix_armed() { let mut spans = vec![Span::styled( diff --git a/src/ui/hint_text.rs b/src/ui/hint_text.rs index 365d753d..c0d90e80 100644 --- a/src/ui/hint_text.rs +++ b/src/ui/hint_text.rs @@ -64,21 +64,24 @@ pub(crate) fn normal_hint_literal(app: &App) -> &'static str { // Tree mode's right pane is permanently the file view — `v` // can't leave it, so don't advertise a no-op. if app.mode == ViewMode::Tree { - " f: exit zoom | j/k: scroll | pgup/pgdn: page | q: quit" + " f: exit zoom | j/k: scroll | pgup/pgdn: page | w: wrap | q: quit" } else { - " f: exit zoom | v: back to diff | j/k: scroll | pgup/pgdn: page | q: quit" + " f: exit zoom | v: back to diff | j/k: scroll | pgup/pgdn: page | w: wrap | q: quit" } } else if app.diff.view == DiffPaneView::Split { + // No `w: wrap` here or in the unzoomed split arm: the split view + // ignores wrapping (halves folding to different heights would stop + // lining up), and a hint for a no-op key would lie. " f: exit zoom | s: unified diff | j/k: scroll | pgup/pgdn: page | q: quit" } else if app.diff.search.active { " type to search | enter: confirm | esc: cancel" } else if !app.diff.search.query.is_empty() { " f: exit zoom | n: next match | shift+n: prev match | /: new search | esc: clear" } else if app.can_open_file_view() { - " f: exit zoom | j/k: scroll | v: view file | s: split | /: search | pgup/pgdn: page | q: quit" + " f: exit zoom | j/k: scroll | tab: view | w: wrap | v: view file | s: split | /: search | pgup/pgdn: page | q: quit" } else { // No file target for `v` — a hint for a no-op key would lie. - " f: exit zoom | j/k: scroll | s: split | /: search | pgup/pgdn: page | q: quit" + " f: exit zoom | j/k: scroll | tab: view | w: wrap | s: split | /: search | pgup/pgdn: page | q: quit" }; return hint; } @@ -134,9 +137,9 @@ pub(crate) fn normal_hint_literal(app: &App) -> &'static str { // Tree mode's right pane is permanently the file view — `v` // can't leave it, so don't advertise a no-op. if app.mode == ViewMode::Tree { - " j/k: scroll | pgup/pgdn: page | /: search | shift+←/→: cycle | q: quit" + " j/k: scroll | pgup/pgdn: page | w: wrap | /: search | shift+←/→: cycle | q: quit" } else { - " v: back to diff | j/k: scroll | pgup/pgdn: page | /: search | shift+←/→: cycle | q: quit" + " v: back to diff | j/k: scroll | pgup/pgdn: page | w: wrap | /: search | shift+←/→: cycle | q: quit" } } else if app.diff.view == DiffPaneView::Split { " s: unified diff | j/k: scroll | pgup/pgdn: page | shift+←/→: cycle | f: zoom | q: quit" @@ -148,16 +151,16 @@ pub(crate) fn normal_hint_literal(app: &App) -> &'static str { // The `l` toggle names its destination (Tree mode never reaches // these arms — its right pane is always the file view). if app.mode == ViewMode::Log { - " shift+←/→: cycle | j/k: scroll | pgup/pgdn: scroll | v: view file | s: split | /: search | t: new pane | f: zoom | l: status view | b: tree view | o: open project | q: quit" + " shift+←/→: cycle | j/k: scroll | pgup/pgdn: scroll | tab: view | w: wrap | v: view file | s: split | /: search | t: new pane | f: zoom | l: status view | b: tree view | o: open project | q: quit" } else { - " shift+←/→: cycle | j/k: scroll | pgup/pgdn: scroll | v: view file | s: split | /: search | t: new pane | f: zoom | l: log view | b: tree view | o: open project | q: quit" + " shift+←/→: cycle | j/k: scroll | pgup/pgdn: scroll | tab: view | w: wrap | v: view file | s: split | /: search | t: new pane | f: zoom | l: log view | b: tree view | o: open project | q: quit" } } else { // No file target for `v` — a hint for a no-op key would lie. if app.mode == ViewMode::Log { - " shift+←/→: cycle | j/k: scroll | pgup/pgdn: scroll | s: split | /: search | t: new pane | f: zoom | l: status view | b: tree view | o: open project | q: quit" + " shift+←/→: cycle | j/k: scroll | pgup/pgdn: scroll | tab: view | w: wrap | s: split | /: search | t: new pane | f: zoom | l: status view | b: tree view | o: open project | q: quit" } else { - " shift+←/→: cycle | j/k: scroll | pgup/pgdn: scroll | s: split | /: search | t: new pane | f: zoom | l: log view | b: tree view | o: open project | q: quit" + " shift+←/→: cycle | j/k: scroll | pgup/pgdn: scroll | tab: view | w: wrap | s: split | /: search | t: new pane | f: zoom | l: log view | b: tree view | o: open project | q: quit" } } } diff --git a/src/ui/mod.rs b/src/ui/mod.rs index f0d1d530..7ad61ead 100644 --- a/src/ui/mod.rs +++ b/src/ui/mod.rs @@ -4,6 +4,7 @@ pub mod diff_viewer; pub mod file_list; pub mod file_view; pub mod log_view; +pub mod path_tree; pub mod project_tab; pub mod search; pub mod splash; @@ -67,34 +68,30 @@ pub fn draw_empty( ); let leader_label = crate::app::leader_label_of(leader); - frame.render_widget( - Paragraph::new(Line::from(vec![Span::styled( - format!(" no project open — {leader_label} o to open a repo"), - Style::default().fg(Color::DarkGray), - )])) - .block(Block::default().borders(Borders::ALL)), - rows.body, - ); + match chrome.repo_input.picker.as_ref() { + Some(tree) => path_tree::render(frame, tree, rows.body, accent), + None => frame.render_widget( + Paragraph::new(Line::from(vec![Span::styled( + format!(" no project open — {leader_label} o to open a repo"), + Style::default().fg(Color::DarkGray), + )])) + .block(Block::default().borders(Borders::ALL)), + rows.body, + ), + } - // Matches `render_notice_row`: a notice is the same red wherever it lands. - let notice_line = match notice { - Some(n) => Line::from(Span::styled( - format!(" {}", n.line()), - Style::default().fg(Color::Red).add_modifier(Modifier::BOLD), - )), - None => Line::default(), - }; + // Shares `render_notice_row`'s priority order so a notice looks the same + // wherever it lands; with no project there is no repo header to fall back + // to, so the row just goes empty. + let notice_line = notice::notice_or_candidates(notice, chrome.repo_input, rows.notice.width) + .unwrap_or_default(); frame.render_widget(Paragraph::new(notice_line), rows.notice); // The armed prefix shows the same chip as the project screen: pressing // the leader here has to look like it did something, or it reads as a // dead key. let hint = if chrome.repo_input.active { - Line::from(vec![ - Span::styled("repo: ", Style::default().fg(accent)), - Span::raw(chrome.repo_input.buf.clone()), - Span::styled("█", Style::default().fg(accent)), - ]) + hint_bar::repo_input_line(chrome.repo_input, accent, rows.hint.width) } else if prefix_armed { let mut spans = vec![Span::styled( PREFIX_CHIP, @@ -132,17 +129,39 @@ pub fn draw( project_tab::render(tabs.repo_paths, tabs.active, rows.tabs, accent), rows.tabs, ); - frame.render_widget(render_notice_row(app, accent), notice_area); + frame.render_widget( + render_notice_row(app, tabs.repo_input, accent, notice_area.width), + notice_area, + ); + + // The browser owns the body while it is open, ahead of every view-mode and + // fullscreen branch: the dialog already holds all the keys, so whatever + // those branches would draw is inert and would only hide the browse. + if let Some(tree) = tabs.repo_input.picker.as_ref() { + path_tree::render(frame, tree, body_area, accent); + frame.render_widget( + render_hint_bar(app, tabs, accent, hint_area.width), + hint_area, + ); + // No PTY cursor: the pane underneath cannot be typed into from here. + return None; + } if app.terminal.fullscreen.fills_body() { let cursor = terminal_tab::render(frame, app, body_area, accent); - frame.render_widget(render_hint_bar(app, tabs, accent), hint_area); + frame.render_widget( + render_hint_bar(app, tabs, accent, hint_area.width), + hint_area, + ); return cursor; } if app.diff.fullscreen { diff_viewer::render(frame, app, body_area, ss, ts, accent); - frame.render_widget(render_hint_bar(app, tabs, accent), hint_area); + frame.render_widget( + render_hint_bar(app, tabs, accent, hint_area.width), + hint_area, + ); return None; } @@ -152,7 +171,10 @@ pub fn draw( ViewMode::Log => commit_list::render(frame, app, body_area, accent), ViewMode::Tree => tree_list::render(frame, app, body_area, accent), } - frame.render_widget(render_hint_bar(app, tabs, accent), hint_area); + frame.render_widget( + render_hint_bar(app, tabs, accent, hint_area.width), + hint_area, + ); return None; } @@ -178,6 +200,9 @@ pub fn draw( } diff_viewer::render(frame, app, upper[1], ss, ts, accent); let cursor = terminal_tab::render(frame, app, main[1], accent); - frame.render_widget(render_hint_bar(app, tabs, accent), hint_area); + frame.render_widget( + render_hint_bar(app, tabs, accent, hint_area.width), + hint_area, + ); cursor } diff --git a/src/ui/notice.rs b/src/ui/notice.rs index 292a1f2d..2f985876 100644 --- a/src/ui/notice.rs +++ b/src/ui/notice.rs @@ -1,18 +1,84 @@ -use crate::app::App; +use crate::app::{App, Notice}; +use crate::ui::status_view::RepoInput; use ratatui::{ style::{Color, Modifier, Style}, text::{Line, Span}, widgets::Paragraph, }; -pub(crate) fn render_notice_row<'a>(app: &'a App, accent: Color) -> Paragraph<'a> { - if let Some(notice) = app.notice.as_ref() { - return Paragraph::new(Line::from(Span::styled( +/// Separates candidate names, and sits before the overflow count. +const CANDIDATE_GAP: &str = " "; + +pub(crate) fn render_notice_row<'a>( + app: &'a App, + repo_input: &RepoInput, + accent: Color, + width: u16, +) -> Paragraph<'a> { + match notice_or_candidates(app.notice.as_ref(), repo_input, width) { + Some(line) => Paragraph::new(line), + None => render_repo_header(app, accent), + } +} + +/// The notice row's content when something wants to claim it: a notice first, +/// then the repo dialog's completion candidates. `None` leaves the row to the +/// caller's own fallback — the repo header on the project screen, nothing on the +/// empty one. +/// +/// A notice outranks the candidates because it explains a rejected action, and +/// any edit (Tab included) clears it, so the two rarely compete for long. +pub(crate) fn notice_or_candidates<'a>( + notice: Option<&'a Notice>, + repo_input: &RepoInput, + width: u16, +) -> Option> { + if let Some(notice) = notice { + return Some(Line::from(Span::styled( format!(" {}", notice.line()), Style::default().fg(Color::Red).add_modifier(Modifier::BOLD), ))); } - render_repo_header(app, accent) + if repo_input.candidates.is_empty() { + return None; + } + // Dim, not red: these are an answer to Tab, and reading them as an error + // would undo the point of showing them. + Some(Line::from(Span::styled( + candidate_line(&repo_input.candidates, width), + Style::default().fg(Color::DarkGray), + ))) +} + +/// Fit as many candidate names as the row holds, reporting the rest as +/// `+N more`. The row is one line, so a long list has to be cut somewhere and +/// dropping the tail silently would read as "that is all there is". +fn candidate_line(candidates: &[String], width: u16) -> String { + let width = width as usize; + let mut line = String::new(); + let mut shown = 0; + for name in candidates { + let next = format!("{}{name}", if shown == 0 { " " } else { CANDIDATE_GAP }); + // Reserve room for the count this name would push into the overflow, so + // the last name placed can never crowd out its own `+N more`. + let overflow = overflow_label(candidates.len() - shown - 1); + if Span::raw(&line).width() + Span::raw(&next).width() + Span::raw(&overflow).width() + > width + { + break; + } + line.push_str(&next); + shown += 1; + } + line.push_str(&overflow_label(candidates.len() - shown)); + line +} + +fn overflow_label(remaining: usize) -> String { + if remaining == 0 { + return String::new(); + } + format!("{CANDIDATE_GAP}+{remaining} more") } pub(crate) fn render_repo_header<'a>(app: &'a App, accent: Color) -> Paragraph<'a> { diff --git a/src/ui/path_tree.rs b/src/ui/path_tree.rs new file mode 100644 index 00000000..6d480a0a --- /dev/null +++ b/src/ui/path_tree.rs @@ -0,0 +1,58 @@ +//! The repo dialog's directory browser, drawn over the whole body. +//! +//! Not a floating box: nothing in this crate floats — every surface takes a +//! layout area — and mouse capture is on by default, so an overlay would be the +//! first thing needing a hit region of its own. Taking the body avoids both, and +//! the path field stays visible on the hint row underneath. + +use crate::ui::render_selectable_list; +use crate::workspace::PathTree; +use ratatui::{ + Frame, + layout::Rect, + style::{Color, Style}, + text::{Line, Span}, + widgets::ListItem, +}; + +/// Two columns per level: enough to read the nesting without pushing long names +/// off the right edge of a deep tree. +const INDENT: usize = 2; + +pub(crate) fn render(frame: &mut Frame, tree: &PathTree, area: Rect, accent: Color) { + let dim = Style::default().fg(Color::DarkGray); + let (items, selected) = if tree.rows().is_empty() { + // Nothing selectable, but the box must say why it is blank — an empty + // frame reads as a failure to load. Enter still picks the root itself. + ( + vec![ListItem::new(Line::from(Span::styled( + " (no sub-directories)", + dim, + )))], + None, + ) + } else { + let items = tree + .rows() + .iter() + .map(|row| { + let marker = if row.expanded { "▾" } else { "▸" }; + ListItem::new(Line::from(vec![ + Span::raw(" ".repeat(row.depth * INDENT)), + Span::styled(format!("{marker} "), dim), + Span::raw(row.name.clone()), + ])) + }) + .collect(); + (items, Some(tree.selected())) + }; + + render_selectable_list( + frame, + area, + format!(" browse {} ", tree.root_label()), + items, + selected, + Style::default().fg(accent), + ); +} diff --git a/src/ui/status_view.rs b/src/ui/status_view.rs index 2e361124..e80bec6e 100644 --- a/src/ui/status_view.rs +++ b/src/ui/status_view.rs @@ -103,4 +103,12 @@ pub struct RepoInput { /// whole prefill; Backspace clears the flag instead, keeping the text /// and entering ordinary editing (the sub-directory case). pub prefilled: bool, + /// Directory names offered by the last Tab press, shown on the notice row. + /// Any edit clears them: the list describes a fragment that no longer + /// matches what is in the buffer. + pub candidates: Vec, + /// The directory browser, when open. `Some` takes over the body and the + /// dialog's keys; the field stays on screen below it and keeps the text, so + /// closing the browser returns to exactly what was being typed. + pub picker: Option, } diff --git a/src/ui/tests/chrome_tests.rs b/src/ui/tests/chrome_tests.rs index dbf8450c..a9f85de6 100644 --- a/src/ui/tests/chrome_tests.rs +++ b/src/ui/tests/chrome_tests.rs @@ -42,6 +42,8 @@ fn the_empty_screen_shows_the_dialog_and_its_rejection() { active: true, buf: "/definitely/not/here".to_string(), prefilled: false, + candidates: Vec::new(), + picker: None, }; let notice = crate::app::Notice::new(NoticeKind::RepoInput, "no such directory"); diff --git a/src/ui/tests/common.rs b/src/ui/tests/common.rs index 3a0646c7..6467ee5d 100644 --- a/src/ui/tests/common.rs +++ b/src/ui/tests/common.rs @@ -15,9 +15,20 @@ use ratatui::{ use syntect::highlighting::ThemeSet; pub(super) fn notice_text(app: &App) -> String { + notice_text_with(app, &RepoInput::default()) +} + +/// The notice row with a dialog state that can be claiming it — completion +/// candidates share the row with notices and the repo header. +pub(super) fn notice_text_with(app: &App, repo_input: &RepoInput) -> String { let mut terminal = Terminal::new(TestBackend::new(200, 1)).unwrap(); terminal - .draw(|frame| frame.render_widget(render_notice_row(app, Color::Yellow), frame.area())) + .draw(|frame| { + frame.render_widget( + render_notice_row(app, repo_input, Color::Yellow, frame.area().width), + frame.area(), + ) + }) .unwrap(); let buf = terminal.backend().buffer(); (0..buf.area.width) @@ -51,7 +62,10 @@ pub(super) fn hint_text_with(app: &App, chrome: Chrome<'_>) -> String { let mut terminal = Terminal::new(TestBackend::new(200, 1)).unwrap(); terminal .draw(|frame| { - frame.render_widget(render_hint_bar(app, chrome, Color::Yellow), frame.area()) + frame.render_widget( + render_hint_bar(app, chrome, Color::Yellow, frame.area().width), + frame.area(), + ) }) .unwrap(); let buf = terminal.backend().buffer(); @@ -71,7 +85,10 @@ pub(super) fn assert_inverted_cells_are_clickable(app: &App) { let mut terminal = Terminal::new(TestBackend::new(200, 1)).unwrap(); terminal .draw(|frame| { - frame.render_widget(render_hint_bar(app, chrome, Color::Yellow), frame.area()) + frame.render_widget( + render_hint_bar(app, chrome, Color::Yellow, frame.area().width), + frame.area(), + ) }) .unwrap(); let buf = terminal.backend().buffer(); diff --git a/src/ui/tests/hint_click_tests.rs b/src/ui/tests/hint_click_tests.rs index b891c37b..2e65c8a4 100644 --- a/src/ui/tests/hint_click_tests.rs +++ b/src/ui/tests/hint_click_tests.rs @@ -88,7 +88,12 @@ fn hint_click_agrees_with_the_rendered_buffer_not_just_the_builder() { terminal .draw(|frame| { frame.render_widget( - render_hint_bar(&app, plain_chrome(&RepoInput::default()), Color::Yellow), + render_hint_bar( + &app, + plain_chrome(&RepoInput::default()), + Color::Yellow, + frame.area().width, + ), frame.area(), ) }) diff --git a/src/ui/tests/hint_diff_tests.rs b/src/ui/tests/hint_diff_tests.rs index 39cc8557..1b4e3204 100644 --- a/src/ui/tests/hint_diff_tests.rs +++ b/src/ui/tests/hint_diff_tests.rs @@ -77,6 +77,42 @@ fn diff_hint_advertises_view_file_only_with_a_file_target() { ); } +/// `w` is handled for the whole diff focus, so every view it acts in has to +/// advertise it — the file view most of all, since a long unwrapped line is +/// what sends you looking for the key. The split view is the one exception: +/// wrapping is ignored there, and a hint for a no-op key would lie. +#[test] +fn every_view_that_wraps_advertises_the_key() { + let mut app = app_with_fake_backend(); + app.focus = Focus::DiffViewer; + for view in [DiffPaneView::Diff, DiffPaneView::File] { + app.diff.view = view; + for zoomed in [false, true] { + app.diff.fullscreen = zoomed; + let text = hint_text(&app); + assert!( + text.contains("w: wrap"), + "{view:?} legend (zoomed={zoomed}) must offer wrap, got: {text}" + ); + } + } + + // Tree mode's right pane is permanently the file view, and wraps the same. + let mut tree = app_with_fake_backend(); + tree.mode = ViewMode::Tree; + tree.focus = Focus::DiffViewer; + tree.diff.view = DiffPaneView::File; + assert!(hint_text(&tree).contains("w: wrap"), "tree file view wraps"); + + app.diff.view = DiffPaneView::Split; + app.diff.fullscreen = false; + let text = hint_text(&app); + assert!( + !text.contains("w: wrap"), + "the split view ignores wrapping, so it must not offer it, got: {text}" + ); +} + /// Tree mode's right pane is permanently the file view — `v` never /// toggles there, so the file-view legend must not offer `back to diff`. #[test] diff --git a/src/ui/tests/hint_legend_tests.rs b/src/ui/tests/hint_legend_tests.rs index 84002025..b62d633f 100644 --- a/src/ui/tests/hint_legend_tests.rs +++ b/src/ui/tests/hint_legend_tests.rs @@ -57,7 +57,12 @@ fn hint_bar_inverts_nothing_when_mouse_capture_is_disabled() { terminal .draw(|frame| { frame.render_widget( - render_hint_bar(&app, plain_chrome(&RepoInput::default()), Color::Yellow), + render_hint_bar( + &app, + plain_chrome(&RepoInput::default()), + Color::Yellow, + frame.area().width, + ), frame.area(), ) }) diff --git a/src/ui/tests/mod.rs b/src/ui/tests/mod.rs index a965700d..b7b53fc2 100644 --- a/src/ui/tests/mod.rs +++ b/src/ui/tests/mod.rs @@ -6,3 +6,4 @@ mod hint_diff_tests; mod hint_legend_tests; mod hit_test_tests; mod notice_tests; +mod repo_picker_tests; diff --git a/src/ui/tests/notice_tests.rs b/src/ui/tests/notice_tests.rs index 3bf606c0..566ac59c 100644 --- a/src/ui/tests/notice_tests.rs +++ b/src/ui/tests/notice_tests.rs @@ -64,6 +64,88 @@ fn notice_row_shows_notices_through_every_overlay() { } } +fn dialog_offering(candidates: &[&str]) -> crate::ui::status_view::RepoInput { + crate::ui::status_view::RepoInput { + active: true, + buf: "/repos/".to_string(), + prefilled: false, + candidates: candidates.iter().map(|c| c.to_string()).collect(), + picker: None, + } +} + +#[test] +fn completion_candidates_take_the_notice_row_over_repo_identity() { + let mut app = app_with_files(vec![]); + app.repo_path = "/tmp/somewhere".to_string(); + + let text = notice_text_with(&app, &dialog_offering(&["nightcrow", "nightowl"])); + + assert!(text.contains("nightcrow"), "got: {text}"); + assert!(text.contains("nightowl"), "got: {text}"); + assert!( + !text.contains("/tmp/somewhere"), + "the candidates answer the Tab that is on screen, got: {text}" + ); +} + +/// A notice explains a rejected action, so it outranks a candidate list — and +/// because any edit clears it, the two cannot both be stale for long. +#[test] +fn a_notice_outranks_the_completion_candidates() { + let mut app = app_with_files(vec![]); + app.raise_notice(NoticeKind::RepoInput, "no such directory"); + + let text = notice_text_with(&app, &dialog_offering(&["nightcrow"])); + + assert!(text.contains("no such directory"), "got: {text}"); + assert!(!text.contains("nightcrow"), "got: {text}"); +} + +#[test] +fn a_candidate_list_too_wide_for_the_row_reports_what_it_dropped() { + let mut terminal = + ratatui::Terminal::new(ratatui::backend::TestBackend::new(24, 1)).expect("a terminal"); + let app = app_with_files(vec![]); + let dialog = dialog_offering(&["alpha", "bravo", "charlie", "delta"]); + + terminal + .draw(|frame| { + frame.render_widget( + crate::ui::notice::render_notice_row( + &app, + &dialog, + ratatui::style::Color::Yellow, + frame.area().width, + ), + frame.area(), + ) + }) + .expect("draw"); + let buf = terminal.backend().buffer(); + let text: String = (0..buf.area.width).map(|x| buf[(x, 0)].symbol()).collect(); + + assert!(text.contains("alpha"), "got: {text}"); + assert!( + text.contains("more"), + "a truncated list must say the tail exists, got: {text}" + ); + assert!( + !text.contains("delta"), + "the row is 24 columns wide, got: {text}" + ); +} + +#[test] +fn the_empty_screen_shows_completion_candidates_too() { + // With no project there is no repo header to fall back to, so the row is + // free — but it still has to render the list. + let text = drawn_empty(&dialog_offering(&["nightcrow", "nightowl"]), None, false); + + assert!(text.contains("nightcrow"), "got: {text}"); + assert!(text.contains("nightowl"), "got: {text}"); +} + /// With nothing raised the row is the repo/branch line, and it comes back /// intact after a notice is cleared. #[test] diff --git a/src/ui/tests/repo_picker_tests.rs b/src/ui/tests/repo_picker_tests.rs new file mode 100644 index 00000000..2c74ceea --- /dev/null +++ b/src/ui/tests/repo_picker_tests.rs @@ -0,0 +1,105 @@ +use super::common::*; +use crate::ui::hint_bar::repo_input_line; +use crate::ui::status_view::RepoInput; +use crate::workspace::PathTree; +use ratatui::style::Color; +use tempfile::TempDir; + +/// The dialog's state with the browser open on a temp directory holding `dirs`. +fn browsing(dirs: &[&str]) -> (TempDir, RepoInput) { + let root = TempDir::new().expect("a temp dir"); + for d in dirs { + std::fs::create_dir(root.path().join(d)).expect("create dir"); + } + let buf = std::fs::canonicalize(root.path()) + .expect("canonical temp path") + .to_str() + .expect("a UTF-8 temp path") + .to_string(); + let picker = PathTree::open(&buf).expect("a readable root"); + ( + root, + RepoInput { + active: true, + buf, + prefilled: false, + candidates: Vec::new(), + picker: Some(picker), + }, + ) +} + +fn field_only() -> RepoInput { + RepoInput { + active: true, + buf: "/repos/current".to_string(), + prefilled: true, + candidates: Vec::new(), + picker: None, + } +} + +#[test] +fn the_browser_fills_the_body_with_the_directories_it_read() { + let (_guard, repo_input) = browsing(&["alpha", "zeta"]); + + let screen = drawn_empty(&repo_input, None, false); + + assert!( + screen.contains("browse"), + "the box names where it is:\n{screen}" + ); + assert!( + screen.contains("alpha") && screen.contains("zeta"), + "{screen}" + ); +} + +#[test] +fn an_empty_directory_says_so_rather_than_drawing_a_blank_box() { + let (_guard, repo_input) = browsing(&[]); + + let screen = drawn_empty(&repo_input, None, false); + + assert!(screen.contains("no sub-directories"), "{screen}"); +} + +#[test] +fn the_dialog_advertises_its_keys_on_the_input_row() { + // Nothing else can: the dialog replaces the hint legend entirely, so an + // unadvertised key is unfindable. + let field = field_only(); + let line = repo_input_line(&field, Color::Yellow, 90).to_string(); + + assert!(line.contains("/repos/current"), "the path itself: {line}"); + assert!( + line.contains("↓: browse"), + "the way into the browser: {line}" + ); + assert!(line.contains("tab: complete"), "{line}"); +} + +#[test] +fn the_browsers_own_keys_replace_the_fields_on_the_input_row() { + let (_guard, repo_input) = browsing(&["alpha"]); + + let line = repo_input_line(&repo_input, Color::Yellow, 200).to_string(); + + assert!(line.contains("enter: select"), "not `enter: open`: {line}"); + assert!(line.contains("←: up"), "{line}"); + assert!(!line.contains("↓: browse"), "already browsing: {line}"); +} + +#[test] +fn a_path_too_long_for_the_legend_drops_it_whole_and_keeps_the_caret() { + let mut field = field_only(); + field.buf = "/a".repeat(30); + + let line = repo_input_line(&field, Color::Yellow, 40).to_string(); + + assert!( + !line.contains("browse"), + "a half legend reads as a glitch: {line}" + ); + assert!(line.ends_with('█'), "the caret has to survive: {line}"); +} diff --git a/src/web/viewer/dto/tests/mod.rs b/src/web/viewer/dto/tests/mod.rs index 70262595..8ee543a2 100644 --- a/src/web/viewer/dto/tests/mod.rs +++ b/src/web/viewer/dto/tests/mod.rs @@ -173,6 +173,8 @@ fn hunk(header: &str, lines: usize, width: usize) -> DiffHunk { .map(|_| crate::git::diff::DiffLine { kind: LineKind::Context, content: "x".repeat(width), + old_lineno: None, + new_lineno: None, }) .collect(), } @@ -187,14 +189,20 @@ fn diff_dto_maps_line_kinds_to_wire_codes() { crate::git::diff::DiffLine { kind: LineKind::Added, content: "new".into(), + old_lineno: None, + new_lineno: Some(1), }, crate::git::diff::DiffLine { kind: LineKind::Removed, content: "old".into(), + old_lineno: Some(1), + new_lineno: None, }, crate::git::diff::DiffLine { kind: LineKind::Context, content: "same".into(), + old_lineno: Some(2), + new_lineno: Some(2), }, ], }]; diff --git a/src/workspace/mod.rs b/src/workspace/mod.rs index 8cc1a22b..69cdef6e 100644 --- a/src/workspace/mod.rs +++ b/src/workspace/mod.rs @@ -5,8 +5,12 @@ //! project. Every project drains its queues each tick whether or not it is on //! screen; snapshots apply to the active one only. +mod path_complete; +mod path_tree; mod repo_input; +mod repo_picker; +pub use path_tree::PathTree; pub use repo_input::RepoInputResult; pub(crate) mod persistence; diff --git a/src/workspace/path_complete.rs b/src/workspace/path_complete.rs new file mode 100644 index 00000000..f39b7599 --- /dev/null +++ b/src/workspace/path_complete.rs @@ -0,0 +1,168 @@ +//! Tab completion for the repo dialog's path field. +//! +//! One `read_dir` per Tab press, against the single directory the buffer names +//! — a deep tree is never walked. Directories only: the dialog opens a repo and +//! a file can never be one. +//! +//! The dialog is not a shell, so only what `confirm_repo_input` itself accepts +//! is understood here: `~`, `..`, and cwd-relative paths. No `$VAR`, no globs. + +use std::path::{MAIN_SEPARATOR, Path}; + +/// What one Tab press does to the buffer. +pub(crate) struct PathCompletion { + /// The buffer after completion — unchanged when nothing matched. + pub buf: String, + /// Directory names to offer. Empty when the completion was unambiguous, + /// when nothing matched, or when the buffer grew: a list is only worth + /// showing once typing can no longer narrow things down. + pub candidates: Vec, +} + +/// Whether `c` ends a path component. `\` counts on Windows only — on Unix it +/// is a legal filename character, so treating it as a separator there would +/// split paths that contain one. +pub(crate) fn is_sep(c: char) -> bool { + c == '/' || (cfg!(windows) && c == '\\') +} + +/// Split a dialog buffer into the directory text (up to and including the last +/// separator) and the trailing component being completed. With no separator the +/// whole buffer is the component and the directory is empty, meaning the process +/// cwd — the same reading `confirm_repo_input` gives a bare relative path. +pub(crate) fn split_dir(buf: &str) -> (&str, &str) { + match buf.char_indices().rfind(|(_, c)| is_sep(*c)) { + Some((i, c)) => (&buf[..i + c.len_utf8()], &buf[i + c.len_utf8()..]), + None => ("", buf), + } +} + +/// Immediate sub-directory names of `dir`, sorted. A directory that cannot be +/// read yields nothing: mid-typing that is the normal state for the completer, +/// and for the browser an unreadable directory is simply one with nothing to +/// show. Directories only — the dialog opens a repo, and a file cannot be one. +pub(crate) fn read_dir_names(dir: &Path, show_hidden: bool) -> Vec { + let Ok(read) = std::fs::read_dir(dir) else { + return Vec::new(); + }; + let mut names: Vec = read + .flatten() + .filter(is_dir_entry) + // A non-UTF-8 name cannot round-trip through the `String` buffer, so + // completing to it would produce a path that no longer opens. + .filter_map(|e| e.file_name().into_string().ok()) + .filter(|n| show_hidden || !n.starts_with('.')) + .collect(); + names.sort_unstable(); + names +} + +/// Complete the last component of `buf` against the directory the rest of it +/// names. See the module docs for the rules; the caller owns the length cap and +/// decides whether to apply the result. +/// +/// The user's own text is never rewritten: a leading `~` or a relative path is +/// expanded for reading only, and the returned buffer keeps the typed prefix +/// with just the completed component appended. +pub(crate) fn complete_dir_path(buf: &str) -> PathCompletion { + let unchanged = || PathCompletion { + buf: buf.to_string(), + candidates: Vec::new(), + }; + + let (dir_text, frag) = split_dir(buf); + // Reuse whatever separator is already in the buffer so a path typed with + // `/` on Windows does not come back with a `\` spliced into it. + let sep = dir_text.chars().next_back().unwrap_or(MAIN_SEPARATOR); + + let dir = + crate::platform::paths::expand_tilde(if dir_text.is_empty() { "." } else { dir_text }); + let names = read_dir_names(&dir, frag.starts_with('.')); + + let mut matches: Vec<&str> = names + .iter() + .map(String::as_str) + .filter(|n| n.starts_with(frag)) + .collect(); + if matches.is_empty() { + // Retry ignoring case only once the exact prefix has found nothing: on + // case-insensitive filesystems (macOS, Windows) the typed casing rarely + // matches the disk, and on Linux this can never shadow an exact match. + let lower = frag.to_lowercase(); + matches = names + .iter() + .map(String::as_str) + .filter(|n| n.to_lowercase().starts_with(&lower)) + .collect(); + } + // `read_dir_names` already sorted, and filtering preserves order. + + match matches.len() { + 0 => unchanged(), + // Append the separator too, so Tab can be pressed again to descend. + 1 => PathCompletion { + buf: format!("{dir_text}{}{sep}", matches[0]), + candidates: Vec::new(), + }, + _ => { + let common = longest_common_prefix(&matches); + // Extending also corrects casing, so this fires whenever the shared + // prefix reads differently from what was typed, not only when it is + // longer. + let extended = common != frag; + // Listing and extending are independent. While typing can still be + // narrowed by an extension the list would be noise — except on a + // directory boundary, where an empty fragment means "what is in + // here?" and a silent extension answers nothing. + let candidates = if extended && !frag.is_empty() { + Vec::new() + } else { + matches.iter().map(|n| n.to_string()).collect() + }; + PathCompletion { + buf: if extended { + format!("{dir_text}{common}") + } else { + buf.to_string() + }, + candidates, + } + } + } +} + +/// `file_type` comes free with the directory read on most platforms; only a +/// symlink costs the extra stat to see what it points at. Symlinked checkouts +/// are common enough that reporting them as non-directories would hide real +/// repos, so unlike the in-repo tree navigator this one follows them. +fn is_dir_entry(entry: &std::fs::DirEntry) -> bool { + match entry.file_type() { + Ok(t) if t.is_symlink() => entry.path().is_dir(), + Ok(t) => t.is_dir(), + // An entry that vanished or cannot be stat'd is dropped rather than + // failing the whole listing. + Err(_) => false, + } +} + +/// Longest prefix shared by every name, truncated on a char boundary so a +/// multi-byte name never splits mid-codepoint. +fn longest_common_prefix(names: &[&str]) -> String { + let Some((first, rest)) = names.split_first() else { + return String::new(); + }; + let mut end = first.len(); + for name in rest { + let shared = first + .char_indices() + .zip(name.chars()) + .take_while(|((_, a), b)| a == b) + .last() + .map_or(0, |((i, a), _)| i + a.len_utf8()); + end = end.min(shared); + } + first[..end].to_string() +} + +#[cfg(test)] +mod tests; diff --git a/src/workspace/path_complete/tests.rs b/src/workspace/path_complete/tests.rs new file mode 100644 index 00000000..9324c43c --- /dev/null +++ b/src/workspace/path_complete/tests.rs @@ -0,0 +1,262 @@ +use super::*; +use std::path::Path; +use tempfile::TempDir; + +/// A directory holding `dirs` as sub-directories and `files` as plain files. +fn tree(dirs: &[&str], files: &[&str]) -> TempDir { + let root = TempDir::new().expect("a temp dir"); + for d in dirs { + std::fs::create_dir(root.path().join(d)).expect("create dir"); + } + for f in files { + std::fs::write(root.path().join(f), b"x").expect("create file"); + } + root +} + +/// `/` as the dialog buffer would hold it. `/` is a separator on +/// every supported platform, so it stands in for whatever the user typed. +fn buf_in(root: &Path, frag: &str) -> String { + format!("{}/{frag}", root.to_str().expect("a UTF-8 temp path")) +} + +#[test] +fn completing_a_unique_directory_appends_a_separator_to_descend() { + let root = tree(&["nightcrow"], &[]); + + let c = complete_dir_path(&buf_in(root.path(), "night")); + + assert_eq!(c.buf, buf_in(root.path(), "nightcrow/")); + assert!(c.candidates.is_empty(), "a unique match needs no list"); +} + +#[test] +fn completing_an_ambiguous_prefix_extends_it_without_listing() { + let root = tree(&["nightcrow", "nightowl"], &[]); + + let c = complete_dir_path(&buf_in(root.path(), "n")); + + assert_eq!(c.buf, buf_in(root.path(), "night")); + assert!( + c.candidates.is_empty(), + "while an extension still narrows the prefix, a list is noise" + ); +} + +#[test] +fn completing_a_prefix_that_cannot_grow_lists_the_candidates() { + let root = tree(&["nightcrow", "nightowl"], &[]); + let buf = buf_in(root.path(), "night"); + + let c = complete_dir_path(&buf); + + assert_eq!(c.buf, buf, "with nothing left to extend the text stands"); + assert_eq!(c.candidates, vec!["nightcrow", "nightowl"]); +} + +#[test] +fn completing_at_a_directory_boundary_lists_every_subdirectory() { + let root = tree(&["alpha", "beta"], &[]); + let buf = buf_in(root.path(), ""); + + let c = complete_dir_path(&buf); + + assert_eq!(c.buf, buf); + assert_eq!(c.candidates, vec!["alpha", "beta"]); +} + +#[test] +fn completing_at_a_directory_boundary_lists_even_while_extending() { + // An empty fragment means "what is in here?", so the shared `sr` prefix + // gets applied *and* the list shown — extending alone would answer nothing. + let root = tree(&["src", "srv"], &[]); + + let c = complete_dir_path(&buf_in(root.path(), "")); + + assert_eq!(c.buf, buf_in(root.path(), "sr")); + assert_eq!(c.candidates, vec!["src", "srv"]); +} + +#[test] +fn completing_skips_files_because_a_repo_must_be_a_directory() { + let root = tree(&["target"], &["tags", "tsconfig.json"]); + + let c = complete_dir_path(&buf_in(root.path(), "t")); + + assert_eq!( + c.buf, + buf_in(root.path(), "target/"), + "the two files must not make the directory ambiguous" + ); +} + +#[test] +fn completing_hides_dotted_directories_until_the_fragment_starts_with_a_dot() { + let root = tree(&[".config", "docs"], &[]); + + let visible = complete_dir_path(&buf_in(root.path(), "")); + assert_eq!(visible.buf, buf_in(root.path(), "docs/")); + + let hidden = complete_dir_path(&buf_in(root.path(), ".")); + assert_eq!(hidden.buf, buf_in(root.path(), ".config/")); +} + +#[test] +fn completing_an_unmatched_prefix_leaves_the_buffer_alone() { + let root = tree(&["docs"], &[]); + let buf = buf_in(root.path(), "zzz"); + + let c = complete_dir_path(&buf); + + assert_eq!(c.buf, buf); + assert!(c.candidates.is_empty()); +} + +#[test] +fn completing_an_empty_directory_leaves_the_buffer_alone() { + let root = tree(&[], &["only-a-file"]); + let buf = buf_in(root.path(), ""); + + let c = complete_dir_path(&buf); + + assert_eq!(c.buf, buf); + assert!(c.candidates.is_empty()); +} + +#[test] +fn completing_inside_an_unreadable_directory_leaves_the_buffer_alone() { + // Mid-typing the buffer routinely names a directory that does not exist. + // That is not an error worth reporting, so it must be a silent no-op. + let buf = "/nonexistent-dir-for-nightcrow-tests/x".to_string(); + + let c = complete_dir_path(&buf); + + assert_eq!(c.buf, buf); + assert!(c.candidates.is_empty()); +} + +#[test] +fn completing_a_bare_fragment_with_no_separator_does_not_panic() { + // Exercises the cwd branch without depending on the working directory's + // contents, which other tests may be changing in parallel. + let buf = "zzz-no-such-prefix-in-cwd".to_string(); + + let c = complete_dir_path(&buf); + + assert_eq!(c.buf, buf); + assert!(c.candidates.is_empty()); +} + +#[test] +fn completing_keeps_a_tilde_literal_instead_of_expanding_it_into_the_buffer() { + // `~` is expanded to read the directory, but writing the expansion back + // would replace the user's text with an absolute path they never typed. + let home = dirs::home_dir().expect("a home directory"); + let home_str = home.to_str().expect("a UTF-8 home path"); + + let c = complete_dir_path("~/"); + + assert!( + c.buf.starts_with("~/"), + "the typed prefix must survive, got: {}", + c.buf + ); + assert!( + !c.buf.contains(home_str), + "the home path must not be written into the buffer, got: {}", + c.buf + ); +} + +#[test] +fn completing_falls_back_to_ignoring_case_and_corrects_the_typed_casing() { + let root = tree(&["Documents"], &[]); + + let c = complete_dir_path(&buf_in(root.path(), "docu")); + + assert_eq!( + c.buf, + buf_in(root.path(), "Documents/"), + "a case-insensitive match must be recased to the name on disk" + ); +} + +// Only a case-sensitive filesystem can hold `Docs` and `docs` side by side; +// APFS and NTFS reject the second `create_dir` as AlreadyExists. The fallback +// this covers therefore only has anything to disambiguate on Linux. +#[cfg(target_os = "linux")] +#[test] +fn completing_prefers_an_exact_case_match_over_a_case_insensitive_one() { + let root = tree(&["Docs", "docs"], &[]); + + let c = complete_dir_path(&buf_in(root.path(), "docs")); + + assert_eq!( + c.buf, + buf_in(root.path(), "docs/"), + "the exact-case pass must resolve this before the fallback runs" + ); +} + +#[test] +fn completing_handles_multi_byte_names_on_a_char_boundary() { + let root = tree(&["한국어-프로젝트", "한국어-문서"], &[]); + + let c = complete_dir_path(&buf_in(root.path(), "한")); + + assert_eq!( + c.buf, + buf_in(root.path(), "한국어-"), + "the shared prefix must stop on a char boundary, not a byte one" + ); +} + +// APFS validates filenames as UTF-8 and rejects the create outright, so only +// Linux can stage an entry the listing has to skip. +#[cfg(target_os = "linux")] +#[test] +fn completing_skips_names_that_are_not_valid_utf8() { + use std::os::unix::ffi::OsStrExt; + + let root = TempDir::new().expect("a temp dir"); + std::fs::create_dir(root.path().join("docs")).expect("create dir"); + let invalid = std::ffi::OsStr::from_bytes(b"do\xffcs"); + std::fs::create_dir(root.path().join(invalid)).expect("create dir"); + + let c = complete_dir_path(&buf_in(root.path(), "do")); + + assert_eq!( + c.buf, + buf_in(root.path(), "docs/"), + "a non-UTF-8 name cannot round-trip the buffer, so it must not compete" + ); +} + +#[cfg(windows)] +#[test] +fn completing_reuses_the_separator_already_in_the_buffer() { + let root = tree(&["docs"], &[]); + let base = root.path().to_str().expect("a UTF-8 temp path").to_string(); + + let c = complete_dir_path(&format!("{base}/do")); + + assert!( + c.buf.ends_with("docs/"), + "a buffer typed with `/` must not gain a `\\`, got: {}", + c.buf + ); +} + +#[cfg(unix)] +#[test] +fn completing_follows_a_symlink_to_a_directory() { + // Symlinked checkouts are common, and reporting one as a non-directory + // would hide a real repo from the picker. + let root = tree(&["real"], &[]); + std::os::unix::fs::symlink(root.path().join("real"), root.path().join("linked")) + .expect("create symlink"); + + let c = complete_dir_path(&buf_in(root.path(), "link")); + + assert_eq!(c.buf, buf_in(root.path(), "linked/")); +} diff --git a/src/workspace/path_tree.rs b/src/workspace/path_tree.rs new file mode 100644 index 00000000..2d27bda9 --- /dev/null +++ b/src/workspace/path_tree.rs @@ -0,0 +1,278 @@ +//! Directory browser for the repo dialog's path field. +//! +//! A flat row list, not a nested tree: expanding splices a directory's children +//! in after it and collapsing removes the rows below it, so the selection is a +//! plain index into what is on screen and no flatten pass runs per frame. +//! +//! Directories only, and nothing here writes: the browser fills the field, and +//! the field's own Enter stays the single place a repo is actually opened. It +//! deliberately does not reuse `git::tree`, which requires a `git2::Repository` +//! and refuses paths outside a worktree — the browser has to walk directories +//! belonging to no repo, with possibly no project open at all. + +use super::path_complete::{is_sep, read_dir_names, split_dir}; +use crate::platform::paths::expand_tilde; +use std::path::{MAIN_SEPARATOR, Path, PathBuf}; + +/// One visible row: a directory name `depth` levels below the root. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct PathRow { + pub name: String, + pub depth: usize, + /// Whether this row's children are spliced in below it. Set even when the + /// directory turned out to have none, so the marker shows it was read + /// rather than leaving the user pressing `→` at an unchanging row. + pub expanded: bool, +} + +#[derive(Debug, Clone)] +pub struct PathTree { + /// The root exactly as the user typed it (`~/coding`, `..`, or `""` for the + /// cwd). Kept beside `root` so a picked path is assembled from the user's + /// own notation — the dialog never rewrites their `~` into an absolute path. + root_text: String, + /// The canonicalized root. Canonical because stepping the root up walks + /// `parent()`, which yields nothing useful for a relative path like `.`. + root: PathBuf, + /// Separator to assemble picked paths with: whatever the field already uses, + /// so a path typed with `/` on Windows doesn't come back with a `\` in it. + sep: char, + rows: Vec, + selected: usize, +} + +impl PathTree { + /// Open the browser on the directory the field currently names. `None` when + /// that cannot be read, which the caller reports on the notice row — with no + /// rows and no root there is nothing to draw. + pub(crate) fn open(buf: &str) -> Option { + let trimmed = buf.trim(); + let sep = trimmed + .chars() + .rev() + .find(|c| is_sep(*c)) + .unwrap_or(MAIN_SEPARATOR); + // Browse from the directory the field names; when it names a file or a + // half-typed component, fall back to the text up to the last separator — + // the same reading Tab completion gives it. + let text = if !trimmed.is_empty() && expand_tilde(trimmed).is_dir() { + trimmed + } else { + split_dir(trimmed).0 + }; + let root = + std::fs::canonicalize(expand_tilde(if text.is_empty() { "." } else { text })).ok()?; + if !root.is_dir() { + return None; + } + let rows = list_rows(&root, 0); + Some(Self { + root_text: text.to_string(), + root, + sep, + rows, + selected: 0, + }) + } + + /// The root as the user's own text, for the browser's title. `""` means the + /// cwd, which reads as `.` on screen. + pub fn root_label(&self) -> &str { + if self.root_text.is_empty() { + "." + } else { + &self.root_text + } + } + + pub fn rows(&self) -> &[PathRow] { + &self.rows + } + + pub fn selected(&self) -> usize { + self.selected + } + + /// Clamped rather than wrapping: the list is a path being narrowed down, and + /// wrapping from the last row back to the first loses the user's place. + pub(crate) fn move_selection(&mut self, down: bool) { + if self.rows.is_empty() { + return; + } + self.selected = if down { + (self.selected + 1).min(self.rows.len() - 1) + } else { + self.selected.saturating_sub(1) + }; + } + + /// Read the selected directory's children and splice them in below it. One + /// `read_dir` per press, against that directory only — an unexpanded subtree + /// is never walked. + pub(crate) fn expand(&mut self) { + let Some(row) = self.rows.get(self.selected) else { + return; + }; + if row.expanded { + return; + } + let depth = row.depth; + let children = list_rows(&self.abs_of(self.selected), depth + 1); + self.rows[self.selected].expanded = true; + let at = self.selected + 1; + self.rows.splice(at..at, children); + } + + /// Collapse the selected directory, or — when it is already collapsed — step + /// out of it: to its parent row, or past the root itself at depth 0. + pub(crate) fn collapse_or_up(&mut self) { + let Some(row) = self.rows.get(self.selected) else { + // Nothing to collapse in an empty directory, so `←` still means + // "get me out of here". + self.re_root(); + return; + }; + let depth = row.depth; + if row.expanded { + self.rows[self.selected].expanded = false; + let from = self.selected + 1; + let end = self.rows[from..] + .iter() + .position(|r| r.depth <= depth) + .map_or(self.rows.len(), |n| from + n); + self.rows.drain(from..end); + return; + } + if depth > 0 { + if let Some(i) = self.rows[..self.selected] + .iter() + .rposition(|r| r.depth == depth - 1) + { + self.selected = i; + } + return; + } + self.re_root(); + } + + /// The picked path in the user's own notation, with a trailing separator so + /// Tab can carry on descending from it once the field has it back. + pub(crate) fn selected_path(&self) -> String { + let mut out = self.root_text.clone(); + if !self.rows.is_empty() { + for name in self.components_of(self.selected) { + if !out.is_empty() && !out.ends_with(is_sep) { + out.push(self.sep); + } + out.push_str(name); + } + } + // An empty root text is the cwd; a bare separator would read as the + // filesystem root instead. + if out.is_empty() { + out.push('.'); + } + if !out.ends_with(is_sep) { + out.push(self.sep); + } + out + } + + /// Step the root up one level, so a browse that started deep in one checkout + /// can still reach a sibling. Expansion below is dropped — the rows are + /// rebuilt from the new root — and the directory just left is selected, so + /// the key reads as "step out" rather than "jump somewhere". + fn re_root(&mut self) { + let Some(parent) = self.root.parent().map(Path::to_path_buf) else { + return; + }; + let left = self + .root + .file_name() + .and_then(|n| n.to_str()) + .map(str::to_string); + // Verify the user-notation parent against the real one instead of + // trusting the text surgery: `~` has no expressible parent, and neither + // does a bare Windows drive. Falling back to the absolute path is the + // one place the dialog rewrites the user's text, because their notation + // cannot name where they just asked to go. + self.root_text = parent_text(&self.root_text, self.sep) + .filter(|t| canonicalizes_to(t, &parent)) + .unwrap_or_else(|| parent.to_string_lossy().to_string()); + self.root = parent; + self.rows = list_rows(&self.root, 0); + self.selected = left + .and_then(|name| self.rows.iter().position(|r| r.name == name)) + .unwrap_or(0); + } + + /// The selected row's path components, walking back up the flat list: the + /// nearest preceding row one level shallower is its parent. + fn components_of(&self, idx: usize) -> Vec<&str> { + let mut want = self.rows[idx].depth; + let mut out = Vec::with_capacity(want + 1); + for row in self.rows[..=idx].iter().rev() { + if row.depth == want { + out.push(row.name.as_str()); + if want == 0 { + break; + } + want -= 1; + } + } + out.reverse(); + out + } + + fn abs_of(&self, idx: usize) -> PathBuf { + self.components_of(idx) + .iter() + .fold(self.root.clone(), |p, c| p.join(c)) + } +} + +/// Hidden directories are left out, matching the completer's default: a home +/// directory full of dot-directories would bury the checkouts being looked for. +fn list_rows(dir: &Path, depth: usize) -> Vec { + read_dir_names(dir, false) + .into_iter() + .map(|name| PathRow { + name, + depth, + expanded: false, + }) + .collect() +} + +/// The user's root text one level up, or `None` when their notation cannot +/// express it. Purely textual — the caller checks it against the real parent. +fn parent_text(text: &str, sep: char) -> Option { + let t = text.trim_end_matches(is_sep); + if t.is_empty() { + // `""` is the cwd, whose parent is `..`. All-separators is the + // filesystem root, which has no parent for `re_root` to reach. + return text.is_empty().then(|| "..".to_string()); + } + if t == "." { + return Some("..".to_string()); + } + // `..` can only go further up by appending another one; trimming a + // component off it would walk back down. + if split_dir(t).1 == ".." { + return Some(format!("{t}{sep}..")); + } + match t.char_indices().rfind(|(_, c)| is_sep(*c)) { + // A separator at index 0 is the filesystem root itself, which stays. + Some((0, c)) => Some(c.to_string()), + Some((i, _)) => Some(t[..i].to_string()), + // A lone relative component (`nightcrow`) sits in the cwd. + None => Some(".".to_string()), + } +} + +fn canonicalizes_to(text: &str, expected: &Path) -> bool { + std::fs::canonicalize(expand_tilde(text)).is_ok_and(|p| p == expected) +} + +#[cfg(test)] +mod tests; diff --git a/src/workspace/path_tree/tests.rs b/src/workspace/path_tree/tests.rs new file mode 100644 index 00000000..4dde6c30 --- /dev/null +++ b/src/workspace/path_tree/tests.rs @@ -0,0 +1,203 @@ +use super::*; +use tempfile::TempDir; + +/// A temp directory with `dirs` created inside it (slash-separated paths are +/// created level by level), plus its canonical path — macOS hands out temp paths +/// under a symlinked `/var`, and the browser reports canonical roots. +fn tree(dirs: &[&str]) -> (TempDir, PathBuf) { + let root = TempDir::new().expect("a temp dir"); + for d in dirs { + let mut p = root.path().to_path_buf(); + for part in d.split('/') { + p.push(part); + if !p.is_dir() { + std::fs::create_dir(&p).expect("create dir"); + } + } + } + let canonical = std::fs::canonicalize(root.path()).expect("canonical temp path"); + (root, canonical) +} + +fn text(path: &Path) -> String { + path.to_str().expect("a UTF-8 temp path").to_string() +} + +fn names(tree: &PathTree) -> Vec<(usize, &str)> { + tree.rows() + .iter() + .map(|r| (r.depth, r.name.as_str())) + .collect() +} + +#[test] +fn opening_on_a_directory_lists_its_sub_directories_sorted() { + let (_guard, root) = tree(&["zeta", "alpha", ".hidden"]); + + let picker = PathTree::open(&text(&root)).expect("a readable root"); + + assert_eq!( + names(&picker), + vec![(0, "alpha"), (0, "zeta")], + "sorted, directories only, hidden left out" + ); +} + +#[test] +fn opening_on_a_half_typed_name_falls_back_to_its_directory() { + let (_guard, root) = tree(&["alpha"]); + + let picker = PathTree::open(&format!("{}/alp", text(&root))).expect("a readable root"); + + assert_eq!(picker.root_label(), format!("{}/", text(&root))); + assert_eq!(names(&picker), vec![(0, "alpha")]); +} + +#[test] +fn opening_on_an_unreadable_path_yields_nothing_to_draw() { + let (_guard, root) = tree(&[]); + + assert!(PathTree::open(&format!("{}/missing/deeper", text(&root))).is_none()); +} + +#[test] +fn expanding_splices_children_below_their_parent() { + let (_guard, root) = tree(&["alpha/inner", "zeta"]); + let mut picker = PathTree::open(&text(&root)).expect("a readable root"); + + picker.expand(); + + assert_eq!( + names(&picker), + vec![(0, "alpha"), (1, "inner"), (0, "zeta")] + ); + assert!(picker.rows()[0].expanded); +} + +#[test] +fn collapsing_removes_the_whole_subtree_below_the_row() { + let (_guard, root) = tree(&["alpha/inner/deepest", "zeta"]); + let mut picker = PathTree::open(&text(&root)).expect("a readable root"); + picker.expand(); + picker.move_selection(true); + picker.expand(); + assert_eq!(picker.rows().len(), 4, "alpha, inner, deepest, zeta"); + + picker.move_selection(false); + picker.collapse_or_up(); + + assert_eq!(names(&picker), vec![(0, "alpha"), (0, "zeta")]); + assert!(!picker.rows()[0].expanded); +} + +#[test] +fn left_on_a_collapsed_child_steps_to_its_parent_row() { + let (_guard, root) = tree(&["alpha/inner"]); + let mut picker = PathTree::open(&text(&root)).expect("a readable root"); + picker.expand(); + picker.move_selection(true); + assert_eq!(picker.selected(), 1); + + picker.collapse_or_up(); + + assert_eq!(picker.selected(), 0, "the nearest shallower row above it"); +} + +#[test] +fn left_at_the_root_re_roots_to_the_parent_and_selects_where_it_came_from() { + let (_guard, root) = tree(&["alpha"]); + let inner = root.join("alpha"); + let mut picker = PathTree::open(&text(&inner)).expect("a readable root"); + + picker.collapse_or_up(); + + assert_eq!(picker.root_label(), text(&root)); + assert_eq!( + picker.rows()[picker.selected()].name, + "alpha", + "the directory just stepped out of stays selected" + ); +} + +#[test] +fn re_rooting_keeps_the_users_own_notation() { + let (_guard, root) = tree(&["alpha/inner"]); + // A trailing separator and a `..` hop: both have to survive the step up as + // text rather than being canonicalized into an absolute path. + let typed = format!("{}/alpha/../alpha/inner/", text(&root)); + let mut picker = PathTree::open(&typed).expect("a readable root"); + + picker.collapse_or_up(); + + assert_eq!( + picker.root_label(), + format!("{}/alpha/../alpha", text(&root)) + ); +} + +#[test] +fn picking_a_row_returns_the_typed_root_plus_a_trailing_separator() { + let (_guard, root) = tree(&["alpha/inner"]); + let mut picker = PathTree::open(&format!("{}/", text(&root))).expect("a readable root"); + picker.expand(); + picker.move_selection(true); + + assert_eq!( + picker.selected_path(), + format!("{}/alpha/inner/", text(&root)), + "the separator lets Tab carry on descending in the field" + ); +} + +#[test] +fn picking_in_an_empty_directory_yields_the_root_itself() { + let (_guard, root) = tree(&[]); + let picker = PathTree::open(&text(&root)).expect("a readable root"); + + assert!(picker.rows().is_empty()); + assert_eq!(picker.selected_path(), format!("{}/", text(&root))); +} + +#[test] +fn moving_the_selection_clamps_at_both_ends() { + let (_guard, root) = tree(&["alpha", "zeta"]); + let mut picker = PathTree::open(&text(&root)).expect("a readable root"); + + picker.move_selection(false); + assert_eq!(picker.selected(), 0); + picker.move_selection(true); + picker.move_selection(true); + assert_eq!( + picker.selected(), + 1, + "clamped, not wrapped to the first row" + ); +} + +#[test] +fn a_home_relative_root_falls_back_to_an_absolute_parent() { + // `~` has no expressible parent, so the one rewrite the dialog allows. + let Some(home) = std::fs::canonicalize(expand_tilde("~")).ok() else { + return; + }; + let Some(parent) = home.parent().map(Path::to_path_buf) else { + return; + }; + let mut picker = PathTree::open("~").expect("a readable home"); + assert_eq!(picker.root_label(), "~"); + + picker.collapse_or_up(); + + assert_eq!(picker.root_label(), parent.to_string_lossy()); +} + +#[test] +fn parent_text_walks_up_without_reading_the_filesystem() { + assert_eq!(parent_text("", '/').as_deref(), Some("..")); + assert_eq!(parent_text(".", '/').as_deref(), Some("..")); + assert_eq!(parent_text("..", '/').as_deref(), Some("../..")); + assert_eq!(parent_text("nightcrow", '/').as_deref(), Some(".")); + assert_eq!(parent_text("~/coding/", '/').as_deref(), Some("~")); + assert_eq!(parent_text("/Users", '/').as_deref(), Some("/")); + assert_eq!(parent_text("/", '/'), None, "the root has no parent"); +} diff --git a/src/workspace/repo_input.rs b/src/workspace/repo_input.rs index 4699ccd2..d6167c8e 100644 --- a/src/workspace/repo_input.rs +++ b/src/workspace/repo_input.rs @@ -17,7 +17,7 @@ pub enum RepoInputResult { // Mirrors `PROMPT_BUFFER_MAX_BYTES` so a bracketed paste cannot grow this // buffer without bound; comfortably above any realistic filesystem path. -const REPO_INPUT_MAX_BYTES: usize = 4096; +pub(super) const REPO_INPUT_MAX_BYTES: usize = 4096; impl Workspace { /// Open the dialog that adds a project tab. Prefilled with the active @@ -31,6 +31,8 @@ impl Workspace { .unwrap_or_default(); self.repo_input.active = true; self.repo_input.prefilled = true; + self.repo_input.candidates.clear(); + self.repo_input.picker = None; self.clear_notice(NoticeKind::RepoInput); } @@ -38,6 +40,8 @@ impl Workspace { self.repo_input.active = false; self.repo_input.buf.clear(); self.repo_input.prefilled = false; + self.repo_input.candidates.clear(); + self.repo_input.picker = None; self.clear_notice(NoticeKind::RepoInput); } @@ -72,10 +76,32 @@ impl Workspace { self.repo_input.active = false; self.repo_input.buf.clear(); self.repo_input.prefilled = false; + self.repo_input.candidates.clear(); + self.repo_input.picker = None; self.clear_notice(NoticeKind::RepoInput); RepoInputResult::Open(resolved) } + /// Extend the typed path from disk, and offer the directories it could + /// still become. Bound to Tab, which is otherwise inert in a text field — + /// and this is the one field where a path is typed with no way to see what + /// is actually there. + pub fn repo_input_complete(&mut self) { + // Tab reads as "extend this path", so an untouched prefill survives + // rather than being replaced — the same reading Backspace and → give it. + self.repo_input.prefilled = false; + let completed = super::path_complete::complete_dir_path(&self.repo_input.buf); + // A completion that would breach the cap is dropped whole: applying a + // truncated path would silently point somewhere else. + if completed.buf.len() > REPO_INPUT_MAX_BYTES { + return; + } + // Any edit invalidates the verdict on the old text, completion included. + self.clear_notice(NoticeKind::RepoInput); + self.repo_input.buf = completed.buf; + self.repo_input.candidates = completed.candidates; + } + pub fn repo_input_push(&mut self, ch: char) { // Typing over an untouched prefill replaces it: the dialog opens on // the current repo path, and a user heading somewhere unrelated would @@ -90,6 +116,7 @@ impl Workspace { } // Any edit invalidates the verdict on the old text. self.clear_notice(NoticeKind::RepoInput); + self.repo_input.candidates.clear(); self.repo_input.buf.push(ch); } @@ -99,6 +126,7 @@ impl Workspace { /// this" without Backspace eating the separator first. pub fn repo_input_accept_prefill(&mut self) { self.repo_input.prefilled = false; + self.repo_input.candidates.clear(); } pub fn repo_input_pop(&mut self) { @@ -106,6 +134,7 @@ impl Workspace { // and just leave prefill mode. self.repo_input.prefilled = false; self.clear_notice(NoticeKind::RepoInput); + self.repo_input.candidates.clear(); self.repo_input.buf.pop(); } } diff --git a/src/workspace/repo_picker.rs b/src/workspace/repo_picker.rs new file mode 100644 index 00000000..bd6d0972 --- /dev/null +++ b/src/workspace/repo_picker.rs @@ -0,0 +1,79 @@ +//! The repo dialog's directory browser, opened from the path field. +//! +//! The browser only ever fills the field; opening a repo stays the field's own +//! Enter, so there is one place that decides what gets opened no matter how the +//! path was arrived at. + +use super::Workspace; +use super::path_tree::PathTree; +use super::repo_input::REPO_INPUT_MAX_BYTES; +use crate::app::NoticeKind; + +impl Workspace { + /// Open the browser on whatever directory the field currently names. Bound + /// to `↓`: the field's horizontal keys already mean "edit this path", so the + /// vertical axis is free for the list, which is also where every other + /// autocomplete puts it. + pub fn repo_input_browse(&mut self) { + // Browsing is an edit intent: the browser writes a whole path into the + // buffer, so an untouched prefill has to stop being replaceable or the + // first key typed after returning would wipe what was just picked. + self.repo_input.prefilled = false; + // The candidate row and the browser answer the same question; leaving + // the list up would describe a fragment the browser is replacing. + self.repo_input.candidates.clear(); + match PathTree::open(&self.repo_input.buf) { + Some(tree) => { + self.clear_notice(NoticeKind::RepoInput); + self.repo_input.picker = Some(tree); + } + // The path is on screen in the field itself, so name the problem + // only — and stay in the field, where it can be corrected. + None => self.raise_notice(NoticeKind::RepoInput, "cannot browse that directory"), + } + } + + /// Close the browser and return to the field with the text untouched. + pub fn repo_input_close_browser(&mut self) { + self.repo_input.picker = None; + } + + /// Take the browser's selection into the field and return to it, so the path + /// can still be extended with Tab or corrected by hand before opening. + pub fn repo_input_pick(&mut self) { + let Some(tree) = self.repo_input.picker.take() else { + return; + }; + let picked = tree.selected_path(); + if picked.len() > REPO_INPUT_MAX_BYTES { + // Refuse whole rather than truncate: a cut path silently points + // somewhere else. Says what it means, since nothing is on screen + // to explain the field not changing. + self.raise_notice(NoticeKind::RepoInput, "path too long"); + return; + } + self.clear_notice(NoticeKind::RepoInput); + self.repo_input.buf = picked; + self.repo_input.candidates.clear(); + } + + /// Move the browser's cursor. Inert with the browser closed, so the caller + /// need not re-check which surface has the keys. + pub fn repo_picker_move(&mut self, down: bool) { + if let Some(tree) = self.repo_input.picker.as_mut() { + tree.move_selection(down); + } + } + + pub fn repo_picker_expand(&mut self) { + if let Some(tree) = self.repo_input.picker.as_mut() { + tree.expand(); + } + } + + pub fn repo_picker_collapse(&mut self) { + if let Some(tree) = self.repo_input.picker.as_mut() { + tree.collapse_or_up(); + } + } +} diff --git a/src/workspace/tests/mod.rs b/src/workspace/tests/mod.rs index 5cd1456c..44bcfdda 100644 --- a/src/workspace/tests/mod.rs +++ b/src/workspace/tests/mod.rs @@ -2,4 +2,5 @@ use super::*; mod common; mod repo_input_tests; +mod repo_picker_tests; mod workspace_tests; diff --git a/src/workspace/tests/repo_input_tests.rs b/src/workspace/tests/repo_input_tests.rs index 9f341e1a..dc40ec20 100644 --- a/src/workspace/tests/repo_input_tests.rs +++ b/src/workspace/tests/repo_input_tests.rs @@ -68,6 +68,76 @@ fn confirming_a_tilde_path_opens_the_home_relative_directory() { ); } +/// A workspace whose prefill is `/`, so Tab has a real +/// directory to complete against. +fn workspace_completing_in(root: &tempfile::TempDir, frag: &str) -> (Workspace, String) { + let base = format!("{}/", root.path().to_str().expect("a UTF-8 temp path")); + let mut ws = workspace_on(&[&format!("{base}{frag}")]); + ws.start_repo_input(); + (ws, base) +} + +#[test] +fn completing_extends_an_untouched_prefill_instead_of_replacing_it() { + // Typing over a prefill replaces it, but Tab means "extend this path" — + // the same reading Backspace and → already give it. + let root = tempfile::TempDir::new().expect("a temp dir"); + std::fs::create_dir(root.path().join("nightcrow")).expect("create dir"); + let (mut ws, base) = workspace_completing_in(&root, "night"); + + ws.repo_input_complete(); + + assert_eq!(ws.repo_input.buf, format!("{base}nightcrow/")); + assert!( + !ws.repo_input.prefilled, + "the prefill is spent, so the next keystroke must append" + ); +} + +#[test] +fn completing_at_a_directory_boundary_offers_the_subdirectories() { + let root = tempfile::TempDir::new().expect("a temp dir"); + std::fs::create_dir(root.path().join("alpha")).expect("create dir"); + std::fs::create_dir(root.path().join("beta")).expect("create dir"); + let (mut ws, base) = workspace_completing_in(&root, ""); + + ws.repo_input_complete(); + + assert_eq!(ws.repo_input.buf, base, "nothing shared to extend"); + assert_eq!(ws.repo_input.candidates, vec!["alpha", "beta"]); +} + +#[test] +fn editing_after_a_completion_drops_the_stale_candidate_list() { + let root = tempfile::TempDir::new().expect("a temp dir"); + std::fs::create_dir(root.path().join("alpha")).expect("create dir"); + std::fs::create_dir(root.path().join("beta")).expect("create dir"); + let (mut ws, _) = workspace_completing_in(&root, ""); + ws.repo_input_complete(); + assert_eq!(ws.repo_input.candidates.len(), 2); + + ws.repo_input_push('a'); + + assert!( + ws.repo_input.candidates.is_empty(), + "the list described a fragment the buffer no longer holds" + ); +} + +#[test] +fn completing_a_path_that_matches_nothing_raises_no_notice() { + // Mid-typing, a path that does not exist yet is the normal state — only + // confirming one is an error. + let root = tempfile::TempDir::new().expect("a temp dir"); + let (mut ws, base) = workspace_completing_in(&root, "zzz"); + + ws.repo_input_complete(); + + assert_eq!(ws.repo_input.buf, format!("{base}zzz")); + assert!(ws.repo_input.candidates.is_empty()); + assert!(ws.empty_notice().is_none()); +} + #[test] fn reopening_the_dialog_re_arms_the_prefill() { let mut ws = workspace_on(&["/repos/current"]); diff --git a/src/workspace/tests/repo_picker_tests.rs b/src/workspace/tests/repo_picker_tests.rs new file mode 100644 index 00000000..b1f439bc --- /dev/null +++ b/src/workspace/tests/repo_picker_tests.rs @@ -0,0 +1,131 @@ +//! The contract between the path field and the directory browser: which one +//! holds the state, and what crossing between them does to the buffer. + +use super::common::*; +use crate::app::NoticeKind; +use tempfile::TempDir; + +/// A workspace whose dialog is open on a real temp directory, since the browser +/// reads the filesystem. +fn dialog_on(dirs: &[&str]) -> (TempDir, crate::workspace::Workspace) { + let root = TempDir::new().expect("a temp dir"); + for d in dirs { + std::fs::create_dir(root.path().join(d)).expect("create dir"); + } + let canonical = std::fs::canonicalize(root.path()).expect("canonical temp path"); + let mut ws = workspace_on(&[canonical.to_str().expect("a UTF-8 temp path")]); + ws.start_repo_input(); + (root, ws) +} + +#[test] +fn browsing_opens_on_the_prefilled_path() { + let (_guard, mut ws) = dialog_on(&["alpha"]); + + ws.repo_input_browse(); + + let picker = ws.repo_input.picker.as_ref().expect("the browser is open"); + assert_eq!(picker.rows().len(), 1); + assert_eq!(picker.rows()[0].name, "alpha"); +} + +#[test] +fn browsing_leaves_prefill_mode_so_the_picked_path_survives_typing() { + let (_guard, mut ws) = dialog_on(&["alpha"]); + + ws.repo_input_browse(); + ws.repo_input_pick(); + ws.repo_input_push('x'); + + assert!( + ws.repo_input.buf.ends_with("alpha/x"), + "typing must extend the picked path, not replace it: {}", + ws.repo_input.buf + ); +} + +#[test] +fn picking_a_row_closes_the_browser_and_fills_the_field() { + let (_guard, mut ws) = dialog_on(&["alpha"]); + let before = ws.repo_input.buf.clone(); + + ws.repo_input_browse(); + ws.repo_input_pick(); + + assert!(ws.repo_input.picker.is_none(), "back to the field"); + assert_eq!(ws.repo_input.buf, format!("{before}/alpha/")); +} + +#[test] +fn closing_the_browser_keeps_the_text_it_started_from() { + let (_guard, mut ws) = dialog_on(&["alpha"]); + let before = ws.repo_input.buf.clone(); + + ws.repo_input_browse(); + ws.repo_picker_move(true); + ws.repo_input_close_browser(); + + assert!(ws.repo_input.picker.is_none()); + assert_eq!( + ws.repo_input.buf, before, + "an abandoned browse must not have to be retyped" + ); + assert!(ws.repo_input.active, "the dialog itself stays open"); +} + +#[test] +fn browsing_an_unreadable_path_reports_it_and_stays_in_the_field() { + let (_guard, mut ws) = dialog_on(&[]); + for c in "/missing/deeper".chars() { + ws.repo_input_push(c); + } + + ws.repo_input_browse(); + + assert!(ws.repo_input.picker.is_none()); + assert_eq!( + ws.active().and_then(|p| p.notice.as_ref()).map(|n| n.kind), + Some(NoticeKind::RepoInput), + "the refusal has to be visible somewhere" + ); +} + +#[test] +fn browsing_drops_the_candidate_list_it_replaces() { + let (_guard, mut ws) = dialog_on(&["alpha", "another"]); + ws.repo_input_push('/'); + ws.repo_input_complete(); + assert!(!ws.repo_input.candidates.is_empty(), "two names to offer"); + + ws.repo_input_browse(); + + assert!( + ws.repo_input.candidates.is_empty(), + "the browser answers the same question the list did" + ); +} + +#[test] +fn cancelling_the_dialog_takes_the_browser_with_it() { + let (_guard, mut ws) = dialog_on(&["alpha"]); + ws.repo_input_browse(); + + ws.cancel_repo_input(); + + assert!(ws.repo_input.picker.is_none()); + assert!(!ws.repo_input.active); +} + +#[test] +fn navigation_is_inert_with_the_browser_closed() { + let (_guard, mut ws) = dialog_on(&["alpha"]); + let before = ws.repo_input.buf.clone(); + + ws.repo_picker_move(true); + ws.repo_picker_expand(); + ws.repo_picker_collapse(); + ws.repo_input_pick(); + + assert_eq!(ws.repo_input.buf, before); + assert!(ws.repo_input.picker.is_none()); +}