feat(diff): context collapse threshold + word wrap#53
Open
hewigovens wants to merge 4 commits into
Open
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the diff engine to use histogram line matching and introduces 'tiny-gap auto-expansion' to prevent collapsing small sections of unmodified lines. Feedback focused on simplifying the context collapsing logic by removing redundant checks and improving safety with saturating_sub for line count calculations.
5337a75 to
8de0f94
Compare
8de0f94 to
dc95086
Compare
4 tasks
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
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.
Summary
Three commits carved out of #46. The word-wrap commit needs manual testing — there is no automated end-to-end coverage for the GPUI/AppKit text layout.
1.
perf(jj-diff): keep short context runs uncollapsed<= COLLAPSED_CONTEXT_THRESHOLD(2) lines — render them inline instead of as an "N unmodified lines" stub.compute_file_diff_full; new threshold tests.if hidden == 0guard.2.
feat(diff): word wrap for unified and side-by-side diff viewsGPUI shell:
shell/gpui/src/diff/wrap.rs—wrap_diff_lines,wrap_sbs_rows,wrap_cols_from_bounds,selection_cols_in_fragment,visual_index_for_line.diff_view/unified_body.rs,diff_view/sbs_body.rs,diff_view/mouse.rs,selection.rs,annotate_view.rs.log/find.rstranslates find-match line indices to visual rows. SBS path chainssbs_line_to_row(pairing-aware) →visual_index_for_sbs_rowso find-next in SBS centers on the correct wrapped row.SwiftUI shell (
JayJayDiffUIpackage):NativeDiffView+WrappedGutter.swift.NativeDiffView.swift,DiffGutterTextView.swift,DiffTextContainerView.swift,DiffTextSupport.swift,NativeDiffView+Gutter.swift,SideBySideDiffRendering.swiftto support wrapped rendering with gutter alignment.DiffTextContainerViewnow exposeswrapsText: Bool(defaulttrue). SBS (SideBySideDiffView) sets both panes' containers tofalse— per-side wrapping would desync the panes and gutters until we ship "wrap to tallest side + continuation rows". Unified view still wraps.DiffLayoutManagerTests.swiftupdated.3.
fix(core): rename backout → revert_change and land on @Crown-jewel fix carved out of #46:
repo.backout()mis-passed--insert-after rev, leaving the revert as a child of the change being reverted instead of on top of@. Rename torevert_changeand pass--onto @to matchjj revertsemantics.JayJayRepo.revert_change,ChangeActions.revertChange, callers inDAGView,RepoContentView+CommandPalette, and theRepoViewModelimpl.real_jj_repotest rewritten: now asserts the working-copy description stays atchild change, the revert change appears inlog, and its parent is@.CONTRIBUTING.md,README.md,docs/index.html) updated to therevertterminology.Test plan
cargo test -p jj-diff(perf threshold cases)cargo test -p jayjay-core(incl.revert_change_uses_jj_revert_and_creates_reverse_change)cargo clippy -p jj-diff -p jayjay-gpui --all-targets -- -D warningsjust test-gpuiandjust test-appjust build(full Swift app + regenerated uniffi bindings)@.