Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion agents/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ Load this file before changing crate ownership, module boundaries, or cross-shel
| `shell/gpui` | Cross-platform GPUI shell; links the Rust crates directly (no UniFFI); also hosts the headless `review`/`--version` CLI dispatch that runs before any window init |
| `shell/mac` | SwiftUI shell + the `JayJayDiffUI` Swift package (AppKit diff renderer) |

Dependency direction (never invert): `primitives` and `jj-diff` are leaves → `jayjay-review` → `jayjay-core` → `jayjay-uniffi` / `shell/gpui`.
Dependency direction (never invert): `primitives` is the leaf → `jj-diff` (may use `primitives`) → `jayjay-review` → `jayjay-core` → `jayjay-uniffi` / `shell/gpui`.

- New shared types go in `jayjay-primitives`, not `jayjay-core`, so review/CLI code stays jj-lib-light.
- Anything two surfaces must agree on (change groups, review identity, note reconciliation) lives at or below `jayjay-review`/`jj-diff` and is consumed by all surfaces. Do not re-implement a diff or identity computation per surface; the GUI and `jayjay review notes` must reconcile through the same provider or notes silently report stale.
Expand Down
2 changes: 1 addition & 1 deletion agents/shell-parity.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ Update this matrix when the user guide adds a feature, a shell closes a gap, or
| Open a Repository | Yes | Yes | Repository-list history stays shell-local, while pins share the Rust-backed `repositories.json`. Both shells keep Pinned above Recent, preserve pins when Recent is cleared, expose live windows plus closed pins from the repository title, activate an existing window without duplication, open closed pins in a new window, and return to the repository list after the last repo window closes. |
| Main Window | Yes | Yes | DAG, detail header, file column, diff pane, status bar, bookmark/tag/conflict markers, and working-copy state should describe the same jj data. |
| Navigate History | Yes | Yes | Selection, keyboard navigation, toolbar revset filtering (presets and custom expressions), load-more behavior, context actions, drag/drop outcomes, and divergent-change handling should stay aligned. |
| Review Diffs | Yes | Partial — added/deleted side-by-side; rich previews | Text diff, unified/side-by-side modes, find, image diff, file review, and flat/tree file lists are GPUI-covered. Exact gaps: added and deleted files fall back to unified instead of rendering side-by-side; rich-preview gaps are enumerated in the next row. |
| Review Diffs | Yes | Partial — added/deleted side-by-side; rich previews | Text diff, unified/side-by-side modes, expandable collapsed context (Show 10 / Show all), find, image diff, file review, and flat/tree file lists are GPUI-covered. SwiftUI briefly tints small reveals when Reduce Motion is off; GPUI uses an atomic, animation-free swap. Exact gaps: added and deleted files fall back to unified instead of rendering side-by-side; rich-preview gaps are enumerated in the next row. |
| Rich File Previews | Yes | Partial — Markdown images; inline HTML | Raw/processed projection modes and cache identity match. GPUI has projection controls, banners, HTML external open, native SVG preview, and a rendered Markdown preview (native block renderer, single post-change document with scrolling — same single-view model as SwiftUI). Exact gaps: Markdown image blocks render as placeholders instead of actual images; GPUI has no inline sandboxed HTML preview toggle, only external-open. |
| Review Notes | Yes | Yes | GPUI supports add/edit/resolve/delete review notes, gutter dot markers, inline note rows, file-list badges, the noted-files filter, and a stale/orphaned banner. Inline note rendering is unified-view-only in both shells; side-by-side shows a note-count banner with "Show in Unified" in both, so that is not a GPUI gap. |
| Compare Changes | Yes | Yes | Shift-click compare, bookmark diff, reverse compare, clear compare, and interdiff loading should use the same rev semantics. |
Expand Down
21 changes: 21 additions & 0 deletions crates/jayjay-primitives/src/diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,3 +213,24 @@ mod tests {
assert!(!diff_edit_collapses_while_stats_pending(0));
}
}

#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)]
pub enum ContextExpansionError {
#[error("context region {region_id} is no longer available")]
UnknownRegion { region_id: u32 },
#[error("context expansion line count must be positive")]
InvalidLineCount,
#[error("context region {region_id} has invalid source bounds")]
InvalidRegion { region_id: u32 },
#[error("context source line {line_no} is unavailable")]
MissingSourceLine { line_no: u32 },
// Constructed only by the FFI bridge when the expansion session lock is unusable.
#[error("context expansion session is unavailable")]
SessionUnavailable,
}

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub struct LineSpan {
pub start: u32,
pub count: u32,
}
96 changes: 94 additions & 2 deletions crates/jayjay-uniffi/src/diff.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,43 @@
use std::sync::{Arc, Mutex};

use jayjay_core::FileDiffStats;
use jayjay_core::diff::{
self, ChangeGroup, ConflictLineKind, DiffLine, DiffSpan, SideBySideRow, WrappedDiffLine,
WrappedSbsRow,
self, ChangeGroup, ConflictLineKind, ContextExpansion, ContextExpansionResult, DiffLine,
DiffSpan, FileDiff, SideBySideRow, WrappedDiffLine, WrappedSbsRow,
};

use jayjay_core::diff::ContextExpansionError;

#[derive(uniffi::Object)]
pub struct ExpandableDiff {
inner: Mutex<diff::ExpandableDiff>,
}

#[uniffi::export]
pub fn make_expandable_diff(
diff: FileDiff,
old_content: String,
new_content: String,
) -> Arc<ExpandableDiff> {
Arc::new(ExpandableDiff {
inner: Mutex::new(diff::ExpandableDiff::new(diff, old_content, new_content)),
})
}

#[uniffi::export]
impl ExpandableDiff {
pub fn expand(
&self,
region_id: u32,
expansion: ContextExpansion,
) -> Result<ContextExpansionResult, ContextExpansionError> {
self.inner
.lock()
.map_err(|_| ContextExpansionError::SessionUnavailable)?
.expand(region_id, expansion)
}
}

#[uniffi::export]
pub fn diff_edit_auto_collapsed_paths(stats: Vec<FileDiffStats>) -> Vec<String> {
jayjay_core::diff_edit_auto_collapsed_paths(&stats)
Expand Down Expand Up @@ -60,3 +94,61 @@ pub fn highlight_file_lines(path: String, content: String) -> Vec<Vec<DiffSpan>>
}

// `visual_index_for_*` stay Rust-only — exporting would copy the full wrapped Vec across FFI per lookup.

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn expandable_diff_object_reveals_context_repeatedly() {
let old_lines: Vec<String> = (1..=80).map(|line| format!("line {line}")).collect();
let mut new_lines = old_lines.clone();
new_lines[39] = "changed".to_owned();
let old = old_lines.join("\n") + "\n";
let new = new_lines.join("\n") + "\n";
let diff = diff::compute_file_diff("sample.txt", &old, &new, false);
let region = diff
.lines
.iter()
.find_map(|line| line.context_region)
.unwrap();
let expandable = make_expandable_diff(diff, old, new);

let first = expandable
.expand(region.id, ContextExpansion::ShowMore { line_count: 10 })
.unwrap();
let second = expandable
.expand(region.id, ContextExpansion::ShowMore { line_count: 10 })
.unwrap();

assert_eq!(first.inserted.count, 10);
assert_eq!(second.inserted.count, 10);
assert_eq!(second.diff.lines.len(), first.diff.lines.len() + 10);
}

#[test]
fn expandable_diff_object_reports_stale_region() {
let old = (1..=30)
.map(|line| format!("line {line}"))
.collect::<Vec<_>>()
.join("\n")
+ "\n";
let mut new = old.clone();
new = new.replace("line 15", "changed");
let diff = diff::compute_file_diff("sample.txt", &old, &new, false);
let region = diff
.lines
.iter()
.find_map(|line| line.context_region)
.unwrap();
let expandable = make_expandable_diff(diff, old, new);

expandable
.expand(region.id, ContextExpansion::ShowAll)
.unwrap();
assert!(matches!(
expandable.expand(region.id, ContextExpansion::ShowAll),
Err(ContextExpansionError::UnknownRegion { region_id }) if region_id == region.id
));
}
}
40 changes: 39 additions & 1 deletion crates/jayjay-uniffi/src/types/diff.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use jayjay_core as core;
use jayjay_core::diff::{
ChangeGroup, CollapsedDiff, ConflictBlock, ConflictBlockSection, ConflictLineKind,
ContextExpansion, ContextExpansionError, ContextExpansionResult, ContextRegion,
DiffDisplayItem, DiffLine, DiffSide, DiffSpan, DiffSpanStyle, DisplayLineMapping, FileDiff,
RowSide, SideBySideRow, WrappedDiffLine, WrappedSbsRow, WrappedSide,
LineSpan, RowSide, SideBySideRow, WrappedDiffLine, WrappedSbsRow, WrappedSide,
};
use jayjay_core::syntax::SyntaxToken;
use jayjay_core::{
Expand Down Expand Up @@ -114,6 +115,20 @@ pub enum DiffSide {
New,
}

#[uniffi::remote(Record)]
pub struct ContextRegion {
pub id: u32,
pub old_start_line: u32,
pub new_start_line: u32,
pub line_count: u32,
}

#[uniffi::remote(Enum)]
pub enum ContextExpansion {
ShowMore { line_count: u32 },
ShowAll,
}

#[uniffi::remote(Record)]
pub struct ChangeGroup {
pub index: u32,
Expand Down Expand Up @@ -178,6 +193,7 @@ pub struct DiffLine {
pub spans: Vec<core::diff::DiffSpan>,
pub conflict_kind: core::diff::ConflictLineKind,
pub no_eof_newline: bool,
pub context_region: Option<core::diff::ContextRegion>,
}

#[uniffi::remote(Record)]
Expand All @@ -194,6 +210,18 @@ pub struct CollapsedDiff {
pub display_to_full: Vec<core::diff::DisplayLineMapping>,
}

#[uniffi::remote(Record)]
pub struct ContextExpansionResult {
pub diff: core::diff::FileDiff,
pub inserted: core::diff::LineSpan,
}

#[uniffi::remote(Record)]
pub struct LineSpan {
pub start: u32,
pub count: u32,
}

#[uniffi::remote(Record)]
pub struct DisplayLineMapping {
pub display_line: u32,
Expand All @@ -208,11 +236,21 @@ pub struct RowSide {
pub conflict_kind: core::diff::ConflictLineKind,
}

#[uniffi::remote(Error)]
pub enum ContextExpansionError {
UnknownRegion { region_id: u32 },
InvalidLineCount,
InvalidRegion { region_id: u32 },
MissingSourceLine { line_no: u32 },
SessionUnavailable,
}

#[uniffi::remote(Record)]
pub struct SideBySideRow {
pub old: core::diff::RowSide,
pub new: core::diff::RowSide,
pub full_width: bool,
pub context_region: Option<core::diff::ContextRegion>,
}

#[uniffi::remote(Record)]
Expand Down
1 change: 1 addition & 0 deletions crates/jj-diff/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ keywords = ["diff", "jujutsu", "git", "syntax-highlighting"]
categories = ["development-tools", "text-processing"]

[dependencies]
jayjay-primitives = { path = "../jayjay-primitives" }
similar = { workspace = true }
unicode-width = { workspace = true }
unicode-segmentation = { workspace = true }
Expand Down
39 changes: 23 additions & 16 deletions crates/jj-diff/src/compute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use super::context::collapse_context;
use super::highlights::apply_highlights;
use super::line_diff::{LineOp, line_diff};
use super::render_highlights::{HighlightInputs, apply_rendered_highlights, plain_spans};
use super::types::{ConflictLineKind, DiffLine, DiffSpan, DiffSpanStyle, FileDiff, LineMap};
use super::types::{ConflictLineKind, DiffLine, DiffSpan, DiffSpanStyle, FileDiff, LineIndex};
use crate::syntax;

/// Standalone per-line highlight for blame/annotate — do not fold back into a diff-against-empty; that produced Added spans, collapsed context, and EOF markers in blame views.
Expand All @@ -17,13 +17,13 @@ pub fn highlight_file(path: &str, content: &str) -> Vec<Vec<DiffSpan>> {
} else {
syntax::highlight(content, language)
};
let line_map = LineMap::from_text(content);
let line_index = LineIndex::from_text(content);
let mut lines = Vec::new();
let mut n: u32 = 1;
while let Some((byte_start, text)) = line_map.get(n) {
while let Some((byte_start, text)) = line_index.get(content, n) {
lines.push(apply_highlights(
text,
*byte_start,
byte_start,
&highlights,
DiffSpanStyle::Context,
));
Expand Down Expand Up @@ -58,7 +58,7 @@ pub fn compute_file_diff_full_plain(
/// File extensions that are generated/data — skip syntax highlighting.
const SKIP_HIGHLIGHT_EXTENSIONS: &[&str] = &["lock", "csv", "tsv", "svg"];

fn should_skip_highlight(path: &str) -> bool {
pub(crate) fn should_skip_highlight(path: &str) -> bool {
if let Some(ext) = path.rsplit('.').next() {
return SKIP_HIGHLIGHT_EXTENSIONS.contains(&ext);
}
Expand All @@ -84,8 +84,8 @@ fn compute_file_diff_impl(
};
}

let old_line_map = LineMap::from_text(old);
let new_line_map = LineMap::from_text(new);
let old_line_index = LineIndex::from_text(old);
let new_line_index = LineIndex::from_text(new);
let skip_highlight = force_skip_highlight || should_skip_highlight(path);

let old_lines: Vec<&str> = old.lines().collect();
Expand All @@ -100,14 +100,15 @@ fn compute_file_diff_impl(
while op_pos < line_ops.len() {
match line_ops[op_pos] {
LineOp::Equal => {
if let Some((_byte_start, text)) = new_line_map.get(new_idx) {
if let Some((_byte_start, text)) = new_line_index.get(new, new_idx) {
result_lines.push(DiffLine {
old_line_no: Some(old_idx),
new_line_no: Some(new_idx),
style: DiffSpanStyle::Context,
spans: plain_spans(text, DiffSpanStyle::Context),
conflict_kind: ConflictLineKind::None,
no_eof_newline: false,
context_region: None,
});
}
old_idx += 1;
Expand All @@ -133,16 +134,18 @@ fn compute_file_diff_impl(
for i in 0..paired_count {
let old_ln = removed_indices[i];
let new_ln = added_indices[i];
if let (Some((_old_byte, old_text)), Some((_new_byte, new_text))) =
(old_line_map.get(old_ln), new_line_map.get(new_ln))
{
if let (Some((_old_byte, old_text)), Some((_new_byte, new_text))) = (
old_line_index.get(old, old_ln),
new_line_index.get(new, new_ln),
) {
result_lines.push(DiffLine {
old_line_no: Some(old_ln),
new_line_no: None,
style: DiffSpanStyle::Removed,
spans: plain_spans(old_text, DiffSpanStyle::Removed),
conflict_kind: ConflictLineKind::None,
no_eof_newline: false,
context_region: None,
});
result_lines.push(DiffLine {
old_line_no: None,
Expand All @@ -151,45 +154,49 @@ fn compute_file_diff_impl(
spans: plain_spans(new_text, DiffSpanStyle::Added),
conflict_kind: ConflictLineKind::None,
no_eof_newline: false,
context_region: None,
});
}
}

for &old_ln in &removed_indices[paired_count..] {
if let Some((_byte_start, text)) = old_line_map.get(old_ln) {
if let Some((_byte_start, text)) = old_line_index.get(old, old_ln) {
result_lines.push(DiffLine {
old_line_no: Some(old_ln),
new_line_no: None,
style: DiffSpanStyle::Removed,
spans: plain_spans(text, DiffSpanStyle::Unchanged),
conflict_kind: ConflictLineKind::None,
no_eof_newline: false,
context_region: None,
});
}
}

for &new_ln in &added_indices[paired_count..] {
if let Some((_byte_start, text)) = new_line_map.get(new_ln) {
if let Some((_byte_start, text)) = new_line_index.get(new, new_ln) {
result_lines.push(DiffLine {
old_line_no: None,
new_line_no: Some(new_ln),
style: DiffSpanStyle::Added,
spans: plain_spans(text, DiffSpanStyle::Unchanged),
conflict_kind: ConflictLineKind::None,
no_eof_newline: false,
context_region: None,
});
}
}
}
LineOp::Add => {
if let Some((_byte_start, text)) = new_line_map.get(new_idx) {
if let Some((_byte_start, text)) = new_line_index.get(new, new_idx) {
result_lines.push(DiffLine {
old_line_no: None,
new_line_no: Some(new_idx),
style: DiffSpanStyle::Added,
spans: plain_spans(text, DiffSpanStyle::Unchanged),
conflict_kind: ConflictLineKind::None,
no_eof_newline: false,
context_region: None,
});
}
new_idx += 1;
Expand Down Expand Up @@ -226,8 +233,8 @@ fn compute_file_diff_impl(
HighlightInputs {
old,
new,
old_line_map: &old_line_map,
new_line_map: &new_line_map,
old_line_index: &old_line_index,
new_line_index: &new_line_index,
language,
skip_highlight,
collapse,
Expand Down
Loading
Loading