Skip to content

Fix diff EOF rendering, commit draft loss, LFS preview; polish DAG#17

Merged
hewigovens merged 1 commit intomainfrom
polish/diff-dag-commit
Apr 23, 2026
Merged

Fix diff EOF rendering, commit draft loss, LFS preview; polish DAG#17
hewigovens merged 1 commit intomainfrom
polish/diff-dag-commit

Conversation

@hewigovens
Copy link
Copy Markdown
Owner

Summary

Five fixes + polish bundled together from one debugging session:

  • Diff engine (jj-diff) — Rust's .lines() silently drops the trailing newline, so an EOF-newline-only change produced an all-Equal diff and rendered as plain file content. Added no_eof_newline on DiffLine + whitespace_only_hidden on FileDiff. Post-process synthesizes a visible Remove+Add pair (with spans kept as Context so there's no spurious word-level highlight) and marks the last affected line per side when EOF differs alongside real changes.
  • Diff renderer (NativeDiffView) — render ⊘↵ no newline at EOF in dim gutter color on lines carrying the flag. DiffSection shows a small orange banner when whitespaceOnlyHidden suppressed real changes.
  • Commit box draft lossCommitBox lives inside if shouldShowCommitBox, so switching to any other change unmounted it and destroyed @State draft. Hoisted to RepoViewModel.commitDraft via Binding. Also guarded seeding with draft.isEmpty so background workingCopyDescription refreshes no longer clobber in-progress typing.
  • LFS pointer image preview — when LFS is uninstalled and a pointer file is committed under an image extension, extract_image_preview used to cache the ~130-byte pointer text as a bogus .png, leading to an infinite AsyncImage spinner. Now sniffs LFS pointer bytes first; caller emits the same <git lfs pointer ...> placeholder the text path already knows how to render.
  • DAG node styling — new DAGNodeStyle helper. Trunk-bookmarked commits (main / master / trunk, with optional @origin / @upstream) render as diamonds; other bookmarked commits render as hollow accent-stroked circles. Rebase overlays now match the node shape.
  • Bookmark copy button — extracted CopyIconButton (the icon + flash pattern that already lived inside CopyableRow) and dropped one next to each bookmark capsule in the Detail header.

Test plan

  • `cargo test -p jj-diff` — 31 pass incl. 4 new tests: EOF-newline added/removed synthesizes a visible pair, marker on real changes, and `whitespace_only_hidden` flag for `ignore_whitespace`-suppressed diffs
  • `just test` — all Rust crates green (79 tests)
  • `just test-app` — all Swift unit tests green (22)
  • `cargo clippy --workspace` + `swiftlint` clean
  • Open a file with trailing-newline-only change: see `⊘↵ no newline at EOF` on the side that lacks it, line text is not word-highlighted red/green
  • Type in commit box, select another change, select @ again: draft is preserved
  • Open a repo with an uninstalled LFS pointer image: placeholder renders instead of infinite spinner
  • DAG view with a `main` bookmark: trunk commit renders as a diamond; side bookmarks as outlined circles
  • Detail view with bookmarks: copy icon flashes ✓ on click

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request enhances the diffing engine and UI by adding support for "no newline at EOF" markers and a banner for hidden whitespace-only changes. It also refactors image preview logic to handle Git LFS pointers and introduces a more descriptive styling system for DAG nodes, using diamonds for trunk bookmarks. Additionally, the commit message box now supports persistent drafts. Feedback was provided regarding a logic flaw in the EOF newline detection where context lines at the end of a file could cause markers to be incorrectly placed.

Comment thread crates/jj-diff/src/compute.rs Outdated
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3000f00a22

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/jj-diff/src/compute.rs Outdated
@hewigovens hewigovens force-pushed the polish/diff-dag-commit branch from 3000f00 to a846043 Compare April 23, 2026 10:45
- Diff engine: detect EOF-newline and whitespace-only changes that
  Rust's .lines() silently drops. Synthesize a visible Remove+Add pair
  with a "⊘↵ no newline at EOF" marker (GitHub Desktop style), and
  surface a banner when ignoreWhitespace suppresses a real change.
- Commit box: hoist draft out of local @State into RepoViewModel so
  typed messages survive navigating @ → another change → back.
- LFS preview: sniff for committed LFS pointer bytes before caching
  as an image; avoids the infinite spinner when LFS is uninstalled.
- DAG styling: trunk-bookmarked commits render as diamonds; other
  bookmarked commits get a hollow accent-stroked circle.
- Detail header: reusable CopyIconButton adds a copy button next to
  each bookmark capsule.
@hewigovens hewigovens force-pushed the polish/diff-dag-commit branch from a846043 to 488206c Compare April 23, 2026 10:53
@hewigovens hewigovens merged commit b9bbf49 into main Apr 23, 2026
2 checks passed
@hewigovens hewigovens deleted the polish/diff-dag-commit branch April 23, 2026 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant