Repository navigation
fix(desktop): add the missing git_file_diff command; file history compare follows renames - #269
Merged
Merged
Conversation
backend.ts has invoked `git_file_diff` since the time-travel diff landed (7e8ead3), but no Rust command of that name ever existed nor was registered in generate_handler!. Under dev:web `/api/git-file-diff` answered, so the file history viewer's compare tab only failed in the desktop app. - commands/read.rs: `git_file_diff(cwd, path, from_hash, to_hash)` runs `git diff <from> <to> -- <path>` and returns `{ path, hunks }` through the shared `parse_diff_hunks`, the shape the TS wrapper reads. Revisions that are empty or start with `-` are refused, the path goes through `safe_repo_path` (a file deleted since still resolves), output is capped like `git_diff`, and an unresolvable revision is an error. - dev-server.mjs: the route now refuses `-` revisions (400), reports git failures (500) instead of an empty diff that read as "no changes", and caps output the same way, so both backends agree. - Parity: `git_file_diff` in the command-parity dispatcher and tests/parity/git-file-diff.test.mjs (edit, file added, file deleted, untouched range, option-like revision, unknown revision). - commandRegistry.test.ts: every command the frontend invokes must be registered in generate_handler! (new `findRegisteredCommands` parser, with unit tests). git_file_diff was the only one missing.
…w fixes Review follow-ups on git_file_diff, applied to both backends so they stay in parity: - Renames: the viewer's commits come from `git log --follow`, so either may predate a rename. The file's path is resolved at each commit (the path itself when it exists there, else the name `git log --follow --name-status -z` recorded, computed once and only when needed). Same path on both sides: `git diff A B -- path`; renamed in between: blob to blob `git diff A:old B:new`, so the hunks are the content change rather than "everything added". - git runs with --literal-pathspecs, --no-ext-diff, --no-color and --no-textconv: `a[1].txt` is not a glob, `:x.txt` is not magic, and user config cannot change what is parsed. - The path is normalised like git_diff (repo_relative_path) and the repo root is refused. Revisions are resolved to commits first, so an unknown one reports "unknown revision: X" (400 on the dev-server). - dev-server: empty or whitespace-only revisions get 400 with Rust's message; validation messages are now identical on both sides. - backend.ts: the Tauri branch passes truncatedFromBytes through, and the HTTP branch throws the dev-server's `error` message. FileHistoryViewer shows a truncation notice (diff.compareTruncated, 5 locales), styled like CommitDiffViewer's. - findRegisteredCommands: lexes Rust comments, strings, raw strings and char literals before looking for generate_handler!, and removes attributes by bracket matching. Tests: Rust (rename both before / one each side, literal glob and colon names, ./ path, root refused, ext-diff/color/textconv config, follow-output parser), parity cases for each, and parser unit tests.
… a re-walk Re-review follow-ups. git_file_diff no longer rebuilds the file's name at each commit; the history it compares already knows it. - git_file_log / git_file_log_pickaxe (Rust and dev-server) run with --literal-pathspecs --name-only -z and a control-character format, and each entry gains an optional `path`: the file's repo-root-relative name at that commit (the old name before a rename). Existing fields and the range log are unchanged. - FileHistoryViewer passes the two entries' names as fromPath / toPath; git_file_diff takes them as optional arguments (default: path), guarded like path (inside the repo, never its root, normalised). Equal names: `git diff A B -- name`; different names with both present: blob to blob. All git calls run at the top level, so a repository opened from a subdirectory resolves the log's root-relative names correctly. - A path that is a directory at either commit is refused. - Removed parse_follow_names / path_at / exists_at and parseFollowNames: no more full-history walk per click, no follow-walk buffer limit, no name-vs-follow ambiguity. - stripRustCommentsAndStrings / withoutAttributes scan character codes and copy code runs as slices instead of a substring + regex per character. Tests: Rust (log names across a rename, other log fields intact, literal log pathspec, compare across a rename via the log, subdirectory cwd, directory and side-path refusals, named-log parser); parity for the file log, the viewer-style compare, subdirectory, literal names, directory and side-path refusals; lexer cases for raw strings, escapes and astral chars.
Rust and dev-server in step:
- Windows: git runs from the top level exactly as `rev-parse
--show-toplevel` prints it, never a canonicalised (`\\?\`) path.
- Fewer processes: one `rev-parse --show-toplevel --show-prefix`, one
`cat-file --batch-check` that resolves both revisions (`<rev>^{commit}`)
and both names (`<rev>^{commit}:<name>`), then the diff. The unknown
revision is still named. A name holding a newline cannot travel in the
batch request and is looked up on its own; a revision with a control
character is refused.
- A file absent from both commits is an error ("<path> not found at either
commit") instead of an empty diff.
- fromPath / toPath are names inside past commits: checked lexically (not
empty, not absolute, no `..`, no NUL, not the root) instead of with
safe_repo_path against today's tree, which refused an old name under a
directory since replaced by a symlink out of the repo.
- git_file_log / pickaxe: --no-relative, so `diff.relative` cannot make the
names relative to a subdirectory cwd.
- parse_file_log_named / parseFileLogNamed: the header ends at the first
0x1e after the five fields, and only the NUL + single newline git writes
before the name is stripped: names starting with a newline or holding a
0x1e survive.
- FileHistoryViewer: nameAtCommit (utils/fileHistoryNames.ts) gives an
unnamed entry (merge, line-range result) the name of the nearest older
named entry, else the nearest newer, placing a commit outside the loaded
history by date. `git log -L` cannot list names (it only allows -p/-s).
Tests: Rust (odd names through log and diff, diff.relative + subdir, the
symlinked historical dir, lexical side-path rules, absent-from-both,
control characters, unknown revision on each side, batch-check lines,
exact named-log parsing), parity for each of those, nameAtCommit unit tests.
Fourth review pass: - Compare state moves to a composable, useFileCompare: selection, result, and an error scoped to the Compare tab (shown there, cleared when the selection changes or a new compare starts), so a failed compare no longer hides blame and the log. Replies for a stale selection are dropped. - Changing file resets the compare selection and result, pickaxe and line-range results: the previous file's names never feed the new diff. - nameAtCommit no longer places commits by author date. An unnamed entry that is in the loaded history (a merge) takes its neighbours' name by list order; a commit outside it gets no side path (the backend uses `path`), and the Compare tab says the name there is unknown, both on a "not found" error and above an all-added/all-deleted diff (diff.compareFailed, diff.compareBeyondHistory, 5 locales). - dev-server /api/git-file-log and /api/git-file-log-pickaxe return the Rust command's error on a failing git log instead of an empty list. - `diff.relative` is neutralised with `-c diff.relative=false` instead of `--no-relative`: the flag needs git 2.28, an unknown config key is ignored by older git, and GitWand documents no minimum git version. Tests: useFileCompare (names passed, scoped error cleared by reselection, no side path beyond history, reset drops an in-flight reply), fileHistoryNames (no inference outside the loaded history, isBeyondHistory), parity for a failing file log.
…n still there Final review round: - useFileHistoryLists owns the per-file lists (log, pickaxe, line-range) and a generation token. Changing file bumps it and clears every list, error and loading flag, the log included, so a failed new load cannot leave the previous file's history as the compare's name source. Pickaxe, line-range, the file load and the blame reload all check the token before writing: a reply for the previous file is dropped, and nothing stays stuck loading. - A compare reply brings the Compare tab forward only if the user is still on the tab where they picked the second commit. - isNameUnknown (was isBeyondHistory) is true whenever nameAtCommit finds no name, including an unnamed in-history commit with no named neighbour; the hint (diff.compareNameUnknown, 5 locales) shows then too. - dev-server file-log and pickaxe routes: maxBuffer 256 MB, as other large git log routes, instead of 10 MB that turned big histories into ENOBUFS 500s while Rust succeeds. - Dead API trimmed: select() returns nothing, load() and the options type are no longer exported, the clearCompare pass-through is gone. Tests: useFileHistoryLists (current reply written, stale pickaxe/range replies dropped and loading reset, log cleared on file change), isNameUnknown for an unnamed in-history commit.
devlint
added a commit
that referenced
this pull request
Oct 14, 2026
devlint
added a commit
that referenced
this pull request
Oct 14, 2026
* fix: diff parsers keep lines starting with -- or ++ inside a hunk
A removed line whose content starts with `--` (diff line `--- comment`)
and an added line starting with `++` (diff line `+++x`) were dropped,
because the `---`/`+++` file-header filter also ran inside hunks. A .sql
file losing a `-- comment` came back as a hunk with only its context
lines, and every later line number shifted.
Every parser now bounds a hunk body by the counts of its `@@` header:
while they are not used up, a line is classified by its first character
only ('+', '-', ' '; an empty string is never context). `---`/`+++` are
file headers only outside a body. A combined `@@@` hunk stays open until
the next `diff ` line, since its counts are per parent.
Fixed parsers:
- Rust: parse_diff_hunks (git_diff), git_show, extract_staged_added_lines
(secrets scan, where `+++ token` also renamed the file),
gl_diff_stats_from_files. Shared helpers parse_hunk_header / HunkBody.
- dev-server.mjs: /api/git-diff, /api/git-file-diff, /api/git-show,
/api/scan-secrets (devHunkBody helpers, kept in sync with Rust).
- @gitwand/core: new hunk-body helpers (exported), used by isPureMove.
- CLI: `gitwand scan` extractStagedAddedLines.
- Desktop: utils/unifiedDiff parseFileDiff, useAbsorb parseDiffHunks
(`--- a/x` content reassigned hunks to file `x`), usePrSummary
countChanges; PullRequestPanel drops its copy for utils/unifiedDiff.
Tests: Rust unit tests on real temp repos (git_diff staged/unstaged,
multi-file git_show, secrets scan) and on raw text (multi-file headers,
no-newline marker, empty line, combined hunk); vitest for every TS parser;
parity cases for git-diff and scan-secrets, plus a new git-show parity
test (probe gains a git-show command).
* fix: one bounded rule for blank context lines across diff parsers
Review follow-up to the `--`/`++` hunk-line fix.
- Blank context lines: with `diff.suppressBlankEmpty=true` git prints a
blank context line as "". The TS parser counted it as context, while
Rust `HunkBody` and the dev-server dropped it without consuming a count,
so later line numbers shifted and rebuilt patches failed `git apply`.
One rule everywhere now: inside a hunk, while counts remain on both the
old and new side, "" is a context line; outside a hunk or once a count
is used up, it is ignored. Implemented in core `classifyHunkLine` (so
unifiedDiff no longer maps "" to " " itself) and Rust `HunkBody::classify`
(`push_hunk_line` no longer slices an empty line).
- AGENTS.md "Diff Parsing" states the bounded rule, keeping the warning
against unbounded `!startsWith('\\')` parsers; core and desktop CLAUDE.md
point to it.
- Core API: `hunkBodyFromHeader(line)` parses the header itself (counts
default to 1) and replaces the parse-then-build pairs at every call site;
`hunkBodyActive` is no longer exported. Rust gains `HunkBody::from_header`.
- dev-server.mjs: the devHunkBody/devClassifyHunkLine copies are gone; a
single `devParseDiffHunks` built on @gitwand/core serves /api/git-diff,
/api/git-file-diff and /api/git-show, and /api/scan-secrets uses core's
helpers and `parseHunkHeader` instead of its own regex.
- git_show now splits on `diff --git` and parses each section with
`parse_diff_hunks` (new `parse_multi_file_diff`), like /api/git-show,
instead of carrying its own hunk loop.
Tests: Rust real-repo tests with diff.suppressBlankEmpty for git_diff
(staged and unstaged) and git_show, plus raw-text cases; core and desktop
vitest for the bounded "" rule and hunkBodyFromHeader; parity cases for
git-diff and git-show with diff.suppressBlankEmpty.
* fix: strict hunk counts, dev-server core loading, git_file_diff coverage
Second review follow-up.
- Strict counts: core `classifyHunkLine` and Rust `HunkBody::classify`
accept '+' only while new-side lines remain, '-' only while old-side
lines remain, and context (' ' or "") only while both do. A header that
undercounts its body now ends the hunk there, so a following `--- a/x`
is read as a header (fails closed) instead of a deletion. A combined
`@@@` hunk keeps first-character classification. Rust drops the now
unused `HunkBody::active`.
- dev:web / dev:server build @gitwand/core first (`build:core`, ~4 s):
dev-server.mjs reads the shared diff parser from packages/core/dist.
`loadDevCore()` imports core once per process, checks the helpers it
needs and fails with "run `pnpm --filter @gitwand/core build`" when dist
is missing or stale; a failed load is retried on the next request.
- `devParseDiffHunks(core, text)` is synchronous; /api/git-show loads core
once per request instead of once per file section; /api/scan-secrets
goes through `loadDevCore()` as well.
- git_file_diff (#269): Rust tests and parity cases with `-- x`/`--- x`
removed and `++x`/`+++ y` added, plain and blob-to-blob (renamed), with
and without diff.suppressBlankEmpty.
- AGENTS.md "Diff Parsing" states the strict per-side rule and lists every
user of the shared parsers; apps/desktop/CLAUDE.md documents the core
build that dev:web and dev:server now run.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
getGitFileDiff(backend.ts, used by FileHistoryViewer's Compare tab) has invoked a Tauri commandgit_file_diffsince 2026-04-09, but that command never existed in Rust and was never ingenerate_handler!. In the desktop app the compare failed on every call; it only worked underdev:webthrough/api/git-file-diff. The registry check from #268 couldn't see it: it checks dev-server routes, not Rust registration.What
git_file_diff(Rust,commands/read.rs): diffs one file between two commits with the shared hunk parser, 5 MB cap likegit_diff,repo_lock::read. 3 git processes per compare (rev-parse --show-toplevel --show-prefix, onecat-file --batch-checkfor both revisions and both names, the diff).git --literal-pathspecs diff --no-ext-diff --no-color --no-textconv, so user diff config (external diff, color, textconv) can't replace the unified diff.git_diff, repo root and directories refused, option-like/empty/control-char revisions refused, unknown revision named in the error; a file absent at both commits is an error, not "no differences".git_file_log/git_file_log_pickaxe(log --follow --name-only -z,--literal-pathspecs,-c diff.relative=false) now return each entry'spathat that commit (optional field, existing payloads unchanged). The viewer passesfromPath/toPath; different names give a blob-to-blob diff. A merge with no listed name takes its neighbours' name; a commit outside the loaded history isn't guessed — the Compare tab says the name there is unknown.useFileComparewith a compare-scoped error (the other tabs stay usable); log / pickaxe / line-range lists inuseFileHistoryListswith a per-file token, so replies for a previous file are dropped; truncation notice; strings in all 5 locales./api/git-file-diffused to answer any git failure with an empty diff, and the file-log routes with[]— both now return the error.commandRegistry.test.tsnow also asserts every invoked command is registered ingenerate_handler!(findRegisteredCommands, comment/string-aware).git_file_diffwas the only gap out of 305.Tests
file_diff_tests(real repos): renames, literal names (a[1].txt,:x.txt), subdirectory cwd withdiff.relative=true, historical dir now a symlink, user diff config, directory/root refusal, odd names (\n,\x1e), errors.git-file-diff.test.mjs: 27 cases, identical payloads and error messages.useConflictHistorytimeout under load (passes alone);vue-tsc,clippyclean.Not verified
Not clicked through in the app, and not run on Windows (git now runs from the raw
--show-toplevelpath, never a\\?\canonical one).Known divergence left as is: on CRLF files Rust strips
\rfrom line content and the dev-server keeps it — same as the existinggit_diff.