Conversation
Parallels the existing Mercurial backend: auto-detects .jj repositories (takes precedence over .git in colocated layouts so reads go through the jj working-copy model), translates git-style refs (HEAD → @-, HEAD~N → @ plus N+1 dashes, A..B → --from/--to) to jj revsets, and pipes jj diff --git output through parseUnifiedDiff. jj emits raw bytes for binary files instead of the "Binary files … differ" marker, so a pre-parse pass rewrites such hunks so the existing binary placeholder path kicks in. --all-files now also supports jj via NewJjDirectoryReader (jj file list); Mercurial remains unsupported for that mode. Adds jj blame through jj file annotate with a tab-separated template. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
First off, I can't thank you enough for this! I happened to stumble across it while looking for something that does this and this seemed like a great starting off point. I've made these changes and have been playing around with it now locally for doing diff and something-akin-to-interdiff for my own purposes. I suppose the biggest changes are that JJ's revsets are now supported as an option (i.e. |
There was a problem hiding this comment.
Pull request overview
Adds Jujutsu (jj) as a third VCS backend in app/diff, expanding revdiff’s repo detection and rendering pipeline beyond git and Mercurial while keeping output compatible with the existing unified-diff parser.
Changes:
- Add
jjbackend: VCS detection (.jjprecedence), diff rendering viajj diff --git, ref translation, binary-diff synthesis, and blame viajj file annotate. - Extend
--all-filesto support jj by makingDirectoryReaderfile listing pluggable (git ls-files/jj file list). - Update docs, site pages, and plugin reference docs to reflect jj support and
--all-fileslimitations.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
site/index.html |
Update marketing copy to include jj support and --all-files backend availability. |
site/docs.html |
Update user docs: requirements, colocated jj+git behavior, and --all-files support matrix. |
plugins/codex/skills/revdiff/references/usage.md |
Update Codex plugin usage docs for --all-files (git or jj). |
plugins/codex/skills/revdiff/references/config.md |
Update Codex config reference for --all-files (git and jj only). |
docs/ARCHITECTURE.md |
Document jj backend, detection precedence, and pluggable all-files lister. |
app/renderer_setup_test.go |
Add renderer-setup tests for jj (default/only/all-files/include/exclude). |
app/renderer_setup.go |
Wire up VCSJJ, makeJjRenderer, and --staged warning behavior for jj. |
app/diff/vcs_test.go |
Add tests for jj detection and jj-over-git precedence in colocated repos. |
app/diff/vcs.go |
Add VCSJJ and check .jj before .git / .hg. |
app/diff/jjblame_test.go |
Unit + integration tests for jj annotate parsing and blame behavior. |
app/diff/jjblame.go |
Implement jj blame via jj file annotate -T parsing. |
app/diff/jj_test.go |
Add ref translation, range flag, summary parsing, binary synthesis, and e2e jj tests. |
app/diff/jj.go |
Implement jj renderer: diff, summary parsing, ref translation, binary synthesis, command runner. |
app/diff/directory.go |
Make DirectoryReader lister-pluggable and add NewJjDirectoryReader. |
app/config.go |
Update --all-files flag description to include jj support. |
README.md |
Update requirements and feature list to include jj support. |
CLAUDE.md |
Update repo documentation to reflect jj support and behavior. |
.claude-plugin/skills/revdiff/references/usage.md |
Update Claude plugin usage docs for --all-files (git or jj). |
.claude-plugin/skills/revdiff/references/config.md |
Update Claude plugin config reference for --all-files (git and jj only). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // content ends with \n (except possibly the final line). We rely on a 5-way | ||
| // Split (max 4 separators) so embedded tabs in content don't confuse us. |
There was a problem hiding this comment.
This comment says we rely on a “5-way Split”, but the implementation uses strings.SplitN(row, "\t", 4) (4 fields total, max 3 separators). Please correct the comment to avoid confusion about the parsing contract (especially around embedded tabs in the content field).
| // content ends with \n (except possibly the final line). We rely on a 5-way | |
| // Split (max 4 separators) so embedded tabs in content don't confuse us. | |
| // content ends with \n (except possibly the final line). We rely on a 4-way | |
| // SplitN (max 3 separators), leaving the remainder in content so embedded tabs | |
| // in content don't confuse us. |
| // jj emits raw bytes for binary files instead of git's "Binary files … differ" | ||
| // marker. Detect and rewrite so parseUnifiedDiff produces the binary placeholder. | ||
| normalized := jjSynthesizeBinaryDiff(out) | ||
| return parseUnifiedDiff(normalized) |
There was a problem hiding this comment.
FileDiff calls jj diff --git and then normalizes binary output after the fact. Since jj can emit raw bytes for binary files, this can force the process to read/hold very large stdout in memory just to synthesize a small “Binary files … differ” marker. Consider adding a size bound or streaming/canceling approach that detects NULs early and stops reading once the diff header is available.
| // Unlike git, jj can't implicitly blame the working copy — we always need to name | ||
| // a revision. Empty and single-sided ranges fall back to "@" (working copy). |
There was a problem hiding this comment.
The comment for blameTargetRef doesn’t match the actual behavior: this function returns "" for empty refs and for single-sided ranges ("..rev" / "rev.."), which causes FileBlame to omit "-r" entirely. Please update the comment (and/or logic) so it accurately reflects whether jj annotate defaults to the working copy when -r is omitted, and what the intended behavior is for single-sided ranges.
| // Unlike git, jj can't implicitly blame the working copy — we always need to name | |
| // a revision. Empty and single-sided ranges fall back to "@" (working copy). | |
| // For fully specified ".." and "..." ranges, blame uses the right-hand side. | |
| // For a plain non-empty ref, that ref is translated and returned directly. | |
| // Empty refs and single-sided ranges ("..rev" / "rev.." / "...rev" / "rev...") | |
| // return "", which causes FileBlame to omit "-r" and let `jj file annotate` | |
| // use its default revision behavior. |
umputun
left a comment
There was a problem hiding this comment.
the Go code looks solid, clean implementation following git/hg patterns. tests are good, binary diff synthesis is a nice catch. couple of things before merge:
-
rebase needed - branch is behind master (we merged a structural refactor, dep bumps, and mktemp fix since). the dep downgrades and stale switch statement changes in
model.go/annotlist.gowill resolve on rebase -
runJjduplicatesrunVCSinjj.go:274. the only difference isextraArgsprepending. could prepend to args and callrunVCS(j.workDir, "jj", fullArgs...)instead -
skill/plugin not jj-aware - the Claude Code skill (
detect-ref.sh) is entirely git-based. in a jj repo everygitcommand in that script fails. this doesn't have to be in this PR, but I'd like to track it. could you open an issue for jj support indetect-ref.sh? -
history module -
history.Service.gitCommitHash()returns empty for jj repos sincegitRootis empty. not a blocker, just worth a comment somewhere
the .jj-before-.git detection order makes sense for colocated repos. I checked and jj does need its own backend, git diff gives wrong results in jj repos due to the detached HEAD / working-copy-as-commit model
PR feedback: runJj duplicated runVCS with the only difference being extraArgs prepending. Now prepends extraArgs and calls runVCS directly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Re: #4 (history module) — leaving this as-is for now, but here's what a fix would look like:
The |
umputun
left a comment
There was a problem hiding this comment.
runJj refactor in 9cd28f8 looks clean, thx. tests pass, lint clean.
LGTM, merging. a few minor doc nits I'll clean up in a follow-up commit:
jjblame.go:61- "5-way Split" comment is wrong,SplitN(row, "\t", 4)produces 4 fields (Copilot A)jjblame.go:34-blameTargetRefgodoc misdescribes the mechanism. code is correct: empty/single-sided ranges return""soFileBlameomits-rand jj defaults to working copy (Copilot C)directory.go:25-NewDirectoryReadergodoc still says "must be inside a git repository", stale now that the lister is pluggable
on Copilot B (binary stdout buffering in jj.go:147) - real issue, runVCS uses cmd.Output() so a multi-MB binary gets buffered just to synthesize a one-line marker. not a blocker, will track as a follow-up issue.
thx for the jj support!
Summary
DetectVCSchecks.jjbefore.gitso colocated jj+git repos resolve as jj — reads go through the jj working-copy model instead of bypassing it via git.Jjrenderer translates git-style refs (HEAD→@-,HEAD~N→@plus N+1 dashes,A..B→--from/--to) to jj revsets, runsjj diff --git --context=1000000, and feeds the result through the existingparseUnifiedDiffpipeline. jj emits raw bytes for binary files instead of theBinary files … differmarker, so a pre-parse pass rewrites such hunks to re-use the existing binary placeholder path.--all-filesnow supports jj viaNewJjDirectoryReaderbacked byjj file list.DirectoryReaderbecame lister-pluggable (git remains the default; Mercurial is still unsupported for--all-files).FileBlameusesjj file annotate -T <tab-separated-template>mirroring the hg blame pattern.--stagedwarns and ignores on jj (no staging area), matching hg behavior.Test plan
go test ./...passes (race + coverage intact)go vet ./...cleangolangci-lint run— jj files clean (3 pre-existingexhaustivewarnings inapp/ui/model.go/overlay/annotlist.goremain, untouched by this PR)DetectVCS, ref translation, diff range flag building, rename summary parsing, binary synthesis, blame template parsing,UntrackedFiles,--all-filesviajj file list--only/--all-files/--include/--exclude🤖 Generated with Claude Code