diff --git a/docs/editing.md b/docs/editing.md index 4927d97..9bebe87 100644 --- a/docs/editing.md +++ b/docs/editing.md @@ -460,7 +460,9 @@ Binary data, missing final newlines, permission/type changes, symlink targets, a Run **`:resolve`** in Normal mode for the current jj working copy. It opens a dedicated tab and selects the current conflicted file (otherwise the first conflict). Historical comparisons remain read-only; resolving never implicitly checks out or creates another change. -**Resolution** shows an editable **Draft** on the left and a read-only **Alternative** on the right, with aligned rows, line/character highlighting, context folds and synchronized scrolling. Both are projected from the current working buffer, including pre-existing unsaved edits: only conflict-marker regions are replaced by their selected alternatives. Already-merged text and edits outside those regions appear on both sides. The initial Draft is seeded from the first side, labelled in its border, and is a normal undoable buffer change; opening never writes it to disk. Alternatives remain frozen while later edits change only the Draft. +The resolver shows the editable **Resolution** on the left and a read-only **Other version** on the right, with aligned rows, line/character highlighting, context folds and synchronized scrolling. Both are projected from the current working buffer, including pre-existing unsaved edits: only conflict-marker regions are replaced by their selected alternatives. Already-merged text and edits outside those regions appear on both sides. The initial output is seeded from the first side as a normal undoable buffer change; opening never writes it to disk. References remain frozen while later edits change only the output. + +Pane borders label the editable output **Resolution** and its frozen reference **Other version**. Paths and branch-comparison metadata are omitted from these titles; unsaved state belongs only to the output, and source identity appears with the reference. Explorer **C** marks files jj still considers conflicted, including while edited/checking or confirmation is unavailable; it clears only when jj confirms resolution. | Command | Behavior | | --- | --- | diff --git a/src/diff.rs b/src/diff.rs index a9133b2..6b90352 100644 --- a/src/diff.rs +++ b/src/diff.rs @@ -359,8 +359,8 @@ impl DiffState { } pub(crate) fn set_resolver_paths(&mut self, root: PathBuf, paths: Vec, file: usize) { let comparison = self.comparison.get_or_insert_with(|| Comparison { - before: "Draft".into(), - after: "Alternative".into(), + before: "Resolution".into(), + after: "Other version".into(), files: Vec::new(), root: root.clone(), editable_paths: BTreeSet::new(), @@ -432,7 +432,7 @@ impl DiffState { self.loading = false; self.pending = None; self.request_kind = None; - self.side_labels = ["Draft", "Alternative"]; + self.side_labels = ["Resolution", "Other version"]; self.comparison_id = Some(self.comparison_id.unwrap_or(0).wrapping_add(1)); self.live_files.clear(); let file = &mut self.comparison.as_mut().unwrap().files[self.file]; @@ -2465,6 +2465,12 @@ mod tests { files.entries().nth(files.selected).unwrap().path, Path::new("first.txt") ); + assert!(!files.is_conflicted(Path::new("first.txt"))); + state + .files + .as_mut() + .unwrap() + .set_conflicted(Path::new("first.txt"), true); state.set_resolver_paths( PathBuf::new(), vec!["binary.bin".into(), "first.txt".into(), "second.txt".into()], @@ -2475,6 +2481,8 @@ mod tests { files.entries().nth(files.selected).unwrap().path, Path::new("second.txt") ); + assert!(files.is_conflicted(Path::new("first.txt"))); + assert!(!files.is_conflicted(Path::new("second.txt"))); } fn excerpt_state(len: usize, changed: &[usize]) -> DiffState { diff --git a/src/editor.rs b/src/editor.rs index 76021e5..0052dd2 100644 --- a/src/editor.rs +++ b/src/editor.rs @@ -1482,6 +1482,8 @@ pub struct ResolverFile { pub path: PathBuf, pub draft_buffer: usize, pub status: ResolverStatus, + /// Retained through draft edits and failed/pending jj confirmation. + jj_conflicted: bool, pub sources: Vec, pub text: bool, pub whole_file_choice: Option, @@ -1492,6 +1494,22 @@ pub struct ResolverFile { comparison_key: Option<(u64, usize)>, } +impl ResolverFile { + fn apply_jj_status(&mut self, status: crate::resolve::Status) { + self.status = match status { + crate::resolve::Status::Unresolved => { + self.jj_conflicted = true; + ResolverStatus::Unresolved + } + crate::resolve::Status::Resolved => { + self.jj_conflicted = false; + ResolverStatus::Resolved + } + crate::resolve::Status::Unconfirmed => ResolverStatus::Unconfirmed, + }; + } +} + pub struct ResolverWorkspace { pub files: Vec, pub current_file: usize, @@ -2285,6 +2303,16 @@ impl Editor { }; } } + if let Some(mut picker) = resolver + .files + .get_mut(resolver.current_file) + .and_then(|file| file.diff.files.take()) + { + for file in &resolver.files { + picker.set_conflicted(&file.path, file.jj_conflicted); + } + resolver.files[resolver.current_file].diff.files = Some(picker); + } } } @@ -2394,6 +2422,7 @@ impl Editor { files.push(ResolverFile { path: file.path.clone(), draft_buffer, status: ResolverStatus::Unresolved, text, whole_file_choice: None, + jj_conflicted: file.status != crate::resolve::Status::Resolved, error: file.notice.clone(), seed_source, projection, @@ -2478,12 +2507,8 @@ impl Editor { .map(split_lines); let was_text = resolver.files[file].text; resolver.files[file].text = saved_text.is_some(); - let status = match model.files[file].status { - crate::resolve::Status::Resolved => ResolverStatus::Resolved, - crate::resolve::Status::Unresolved => ResolverStatus::Unresolved, - crate::resolve::Status::Unconfirmed => ResolverStatus::Unconfirmed, - }; - resolver.files[file].status = status; + resolver.files[file].apply_jj_status(model.files[file].status); + let status = resolver.files[file].status; resolver.files[file].error = outcome.confirmation.clone(); resolver.files[file].whole_file_choice = None; if let Some(buffer) = if self.active.id == buffer { @@ -2639,6 +2664,7 @@ impl Editor { resolver.files[file].comparison_key = None; self.blur_file_picker(); self.refresh_resolver_comparison(); + self.refresh_resolver_status(); if self.resolver().unwrap().view == ResolverView::History { self.set_resolver_view(ResolverView::History); } @@ -12552,6 +12578,7 @@ mod tests { path: "file.txt".into(), draft_buffer: result, status: ResolverStatus::Unsaved, + jj_conflicted: true, text: true, whole_file_choice: None, error: None, @@ -12688,11 +12715,6 @@ mod tests { ordinary.diff().unwrap().display_rows ); assert!(resolver.diff_editing()); - assert_eq!(resolver.diff().unwrap().side_label(Focus::Before), "Draft"); - assert_eq!( - resolver.diff().unwrap().side_label(Focus::After), - "Alternative" - ); assert!(resolver.diff().unwrap().side_editable(Focus::Before)); assert!(!resolver.diff().unwrap().side_editable(Focus::After)); let draft_buffer = resolver.buffer_id(); @@ -12791,6 +12813,7 @@ mod tests { path: "conflict.txt".into(), draft_buffer, status: ResolverStatus::Unsaved, + jj_conflicted: true, text: true, whole_file_choice: None, error: None, @@ -12812,6 +12835,73 @@ mod tests { editor } + #[test] + fn resolver_explorer_retains_last_jj_conflict_state_until_confirmation() { + let mut editor = resolver_editor("draft", &[("draft", false), ("other", false)]); + let path = Path::new("conflict.txt"); + let marker = |editor: &Editor| { + editor + .resolver_workspace_diff() + .unwrap() + .files + .as_ref() + .unwrap() + .is_conflicted(path) + }; + assert_eq!( + editor.resolver().unwrap().files[0].status, + ResolverStatus::Unsaved + ); + assert!(marker(&editor)); + for confirmed in [ + crate::resolve::Status::Unresolved, + crate::resolve::Status::Resolved, + crate::resolve::Status::Unresolved, + ] { + editor.resolvers.get_mut(&editor.tab_id).unwrap().files[0].apply_jj_status(confirmed); + editor.refresh_resolver_status(); + let conflicted = confirmed == crate::resolve::Status::Unresolved; + assert_eq!(marker(&editor), conflicted); + assert_eq!( + editor.resolver().unwrap().files[0].status, + ResolverStatus::Unsaved + ); + + editor.resolvers.get_mut(&editor.tab_id).unwrap().files[0].status = + ResolverStatus::Checking; + editor.refresh_resolver_status(); + assert_eq!( + editor.resolver().unwrap().files[0].status, + ResolverStatus::Checking + ); + assert_eq!(marker(&editor), conflicted); + + editor.resolvers.get_mut(&editor.tab_id).unwrap().files[0] + .apply_jj_status(crate::resolve::Status::Unconfirmed); + assert_eq!( + editor.resolver().unwrap().files[0].status, + ResolverStatus::Unconfirmed + ); + editor.active.saved_lines.clone_from(&editor.active.lines); + editor.refresh_resolver_status(); + assert_eq!( + editor.resolver().unwrap().files[0].status, + ResolverStatus::Unconfirmed + ); + assert_eq!(marker(&editor), conflicted); + editor.apply(vec![Effect::Insert(" edit".into())]).unwrap(); + editor.refresh_resolver_status(); + assert_eq!( + editor.resolver().unwrap().files[0].status, + ResolverStatus::Unsaved + ); + assert_eq!(marker(&editor), conflicted); + + editor.select_resolver_file(0); + assert_eq!(marker(&editor), conflicted); + } + } + #[test] fn resolver_takes_only_current_diff_hunk_and_undo_preserves_native_newlines() { for (draft, alternative, chosen) in [ @@ -12944,6 +13034,7 @@ mod tests { path: "second.txt".into(), draft_buffer: buffer, status: ResolverStatus::Unresolved, + jj_conflicted: true, text: true, whole_file_choice: None, error: None, diff --git a/src/files.rs b/src/files.rs index a25ebf6..54b59f9 100644 --- a/src/files.rs +++ b/src/files.rs @@ -101,6 +101,7 @@ pub struct FilePicker { tree_searching: bool, warning: Option, repository_changes: Vec, + conflicted_paths: HashSet, has_virtual_entries: bool, } @@ -306,6 +307,7 @@ impl FilePicker { tree_searching: false, warning, repository_changes: Vec::new(), + conflicted_paths: HashSet::new(), }; for root in roots { let tree = picker.compact(root, None); @@ -917,12 +919,28 @@ impl FilePicker { topology_changed || metadata_changed } + /// Last authoritative conflict state, independent of ordinary change status. + pub fn is_conflicted(&self, path: &Path) -> bool { + self.conflicted_paths.contains(path) + } + + pub(crate) fn set_conflicted(&mut self, path: &Path, conflicted: bool) { + if conflicted { + if !self.conflicted_paths.contains(path) { + self.conflicted_paths.insert(path.to_owned()); + } + } else { + self.conflicted_paths.remove(path); + } + } + pub(crate) fn replace_paths(&mut self, paths: impl IntoIterator) { self.replace_catalog(Self::from_paths(self.root.clone(), paths)); } fn replace_catalog(&mut self, mut replacement: Self) { replacement.presentation = self.presentation; + replacement.conflicted_paths = std::mem::take(&mut self.conflicted_paths); let by_path: HashMap<_, _> = replacement .nodes .iter() diff --git a/src/tui.rs b/src/tui.rs index bec6f7b..7f2cea8 100644 --- a/src/tui.rs +++ b/src/tui.rs @@ -3942,6 +3942,31 @@ fn diff_pane_panel( return area; } let focus = if before { Focus::Before } else { Focus::After }; + if let Some(resolver) = resolver { + let file = resolver.files.get(resolver.current_file); + let title = if before { + " Resolution " + } else { + " Other version " + }; + let state = if before { + format!( + " editable · {} ", + file.map_or("loading", |file| resolver_status(&file.status).0) + ) + } else { + " read-only ".to_owned() + }; + let source = if before { + String::new() + } else { + file.and_then(|file| file.sources.get(resolver.selected_source)) + .map_or_else(String::new, |source| { + format!(" {} ", display_text(&source.label)) + }) + }; + return diff_panel(frame, area, title, &state, &source, diff.focus == focus); + } let label = diff.side_label(focus); let title = file.map_or_else( || format!(" Diff · {} ", diff.comparison_mode.label()), @@ -3964,60 +3989,33 @@ fn diff_pane_panel( ) }, ); - let revision = if let Some(resolver) = resolver { - let source = resolver - .files - .get(resolver.current_file) - .and_then(|file| { - file.sources.get(if before { - file.seed_source - } else { - resolver.selected_source - }) - }) - .map_or("", |source| source.label.as_str()); - if before { - format!(" seeded from {} ", display_text(source)) - } else { - format!(" {} ", display_text(source)) - } - } else { - diff.comparison - .as_ref() - .map_or_else(String::new, |comparison| { - if diff.view_mode == ViewMode::Split && diff.pane_layout() == PaneLayout::Split { - format!( - " {} ", - display_text(if before { - &comparison.before - } else { - &comparison.after - }) - ) - } else { - format!( - " {} → {} ", - display_text(&comparison.before), - display_text(&comparison.after) - ) - } - }) - }; + let revision = diff + .comparison + .as_ref() + .map_or_else(String::new, |comparison| { + if diff.view_mode == ViewMode::Split && diff.pane_layout() == PaneLayout::Split { + format!( + " {} ", + display_text(if before { + &comparison.before + } else { + &comparison.after + }) + ) + } else { + format!( + " {} → {} ", + display_text(&comparison.before), + display_text(&comparison.after) + ) + } + }); let access = if diff.side_editable(focus) { " editable " } else { " read-only " }; - let state = if let Some(resolver) = resolver { - let status = resolver - .files - .get(resolver.current_file) - .map_or("loading", |file| resolver_status(&file.status).0); - Cow::Owned(format!("{access}· {status} ")) - } else { - Cow::Borrowed(access) - }; - diff_panel(frame, area, &title, &state, &revision, diff.focus == focus) + diff_panel(frame, area, &title, access, &revision, diff.focus == focus) } #[allow(clippy::too_many_arguments)] @@ -5465,7 +5463,10 @@ fn draw_file_list( )); spans.push(Span::raw(" ")); } - if let Some(status) = status { + if picker.is_conflicted(entry.path) { + spans.push(Span::styled("C", Style::default().fg(Color::Yellow))); + spans.push(Span::raw(" ")); + } else if let Some(status) = status { let color = match status { FileStatus::Added => Color::Green, FileStatus::Removed => Color::Red, @@ -6994,7 +6995,6 @@ mod tests { assert!(draft.height > 20); assert!(views.panels.files.is_none()); let screen = screen_text(&terminal); - assert!(screen.contains("Draft") && screen.contains("Alternative")); assert!(screen.contains("unchanged lines hidden")); assert!(!screen.contains("context 30")); assert!(!screen.contains("<<<<<<<") && !screen.contains("%%%%%%%")); @@ -7003,6 +7003,27 @@ mod tests { .map(|x| terminal.backend().buffer()[(x, y)].symbol()) .collect::() }; + let left_heading = line_at(&terminal, draft, draft.y); + let right_heading = line_at(&terminal, alternative, alternative.y); + assert!(left_heading.contains("Resolution") && left_heading.contains("editable")); + assert!(right_heading.contains("Other version") && right_heading.contains("read-only")); + for heading in [&left_heading, &right_heading] { + assert!(!heading.contains("Branch changes")); + assert!( + !heading.contains(resolver.files[current_file].path.to_string_lossy().as_ref()) + ); + assert!(!heading.split_whitespace().any(|word| word == "M")); + } + assert!( + line_at(&terminal, alternative, alternative.bottom() - 1) + .contains(&resolver.files[current_file].sources[alternative_index].label) + ); + assert!( + line_at(&terminal, draft, draft.bottom() - 1) + .chars() + .all(|ch| !ch.is_alphanumeric()), + "The output must not repeat its seed source metadata" + ); for (side, area) in [draft, alternative].into_iter().enumerate() { let content = diff_inner(area); let x = content.right() - geometry.text_widths[side]; @@ -7233,6 +7254,11 @@ mod tests { let geometry = views.panels.diff.as_ref().unwrap(); let left = diff_inner(geometry.regions[1].0); let right = diff_inner(geometry.regions[2].0); + let left_heading = line_at(&terminal, geometry.regions[1].0, geometry.regions[1].0.y); + let right_heading = line_at(&terminal, geometry.regions[2].0, geometry.regions[2].0.y); + assert!(left_heading.contains("unsaved")); + assert!(!right_heading.contains("unsaved")); + assert!(right_heading.contains("read-only")); assert!(line_at(&terminal, left, left.y + 1).contains("manual opening context 00")); assert!(line_at(&terminal, right, right.y + 1).contains("opening context 00")); assert!(!line_at(&terminal, right, right.y + 1).contains("manual")); @@ -9175,6 +9201,60 @@ mod tests { assert!(!text.contains('✓')); } + #[test] + fn native_explorer_marks_only_confirmed_conflicts() { + let directory = Directory::new(); + directory.file("file.txt", ""); + let mut picker = FilePicker::scan( + directory.0.clone(), + FilePresentation::Floating, + FileView::List, + ) + .unwrap(); + picker.set_repository_changes(vec![crate::repository::WorkingFileChange { + path: "file.txt".into(), + status: FileStatus::Modified, + old_path: None, + }]); + let mut terminal = Terminal::new(ratatui::backend::TestBackend::new(80, 12)).unwrap(); + for view in [FileView::List, FileView::Tree] { + picker.set_view(view); + for conflicted in [true, false] { + picker.set_conflicted(std::path::Path::new("file.txt"), conflicted); + terminal + .draw(|frame| { + draw_file_list( + frame, + &picker, + frame.area(), + "", + &mut WheelPanels::default(), + None, + ); + }) + .unwrap(); + let text = screen_text(&terminal); + let row = text.lines().find(|row| row.contains("file.txt")).unwrap(); + assert!(row.contains(if conflicted { + "C file.txt" + } else { + "M file.txt" + })); + if conflicted { + let buffer = terminal.backend().buffer(); + let (x, y) = (0..buffer.area.height) + .find_map(|y| { + (0..buffer.area.width) + .find(|&x| buffer[(x, y)].symbol() == "C") + .map(|x| (x, y)) + }) + .unwrap(); + assert_eq!(buffer[(x, y)].fg, Color::Yellow); + } + } + } + } + #[test] fn normal_file_names_stay_put_when_status_arrives_or_clears() { let directory = Directory::new();