From e5cb9f94f665881ddc15f210604385424c18e31f Mon Sep 17 00:00:00 2001 From: Scott Taylor Date: Sat, 16 Nov 2024 11:25:30 -0600 Subject: [PATCH] conflicts: add "ui.conflict-marker-style" config Adds a new "ui.conflict-marker-style" config option. The "diff" option is the default jj-style conflict markers with a snapshot and a series of diffs to apply to the snapshot. New conflict marker style options will be added in later commits. The majority of the changes in this commit are from passing the config option down to the code that materializes the conflicts. Example of "diff" conflict markers: ``` <<<<<<< Conflict 1 of 1 +++++++ Contents of side #1 fn example(word: String) { println!("word is {word}"); %%%%%%% Changes from base to side #2 -fn example(w: String) { +fn example(w: &str) { println!("word is {w}"); >>>>>>> Conflict 1 of 1 ends } ``` --- CHANGELOG.md | 4 + cli/examples/custom-working-copy/main.rs | 12 +- cli/src/cli_util.rs | 69 ++++- cli/src/commands/file/show.rs | 6 +- cli/src/commands/file/track.rs | 2 + cli/src/commands/file/untrack.rs | 2 + cli/src/commands/operation/diff.rs | 4 +- cli/src/commands/operation/log.rs | 11 +- cli/src/commands/operation/show.rs | 10 +- cli/src/commands/sparse.rs | 3 +- cli/src/commands/workspace/add.rs | 3 +- cli/src/commit_templater.rs | 20 +- cli/src/config-schema.json | 8 + cli/src/config/misc.toml | 1 + cli/src/diff_util.rs | 91 ++++-- cli/src/merge_tools/builtin.rs | 55 +++- cli/src/merge_tools/diff_working_copies.rs | 16 +- cli/src/merge_tools/external.rs | 16 +- cli/src/merge_tools/mod.rs | 79 ++++- lib/src/annotate.rs | 7 +- lib/src/conflicts.rs | 30 +- lib/src/default_index/revset_engine.rs | 3 +- lib/src/local_working_copy.rs | 64 +++- lib/src/settings.rs | 8 + lib/src/working_copy.rs | 27 +- lib/src/workspace.rs | 4 +- lib/tests/test_conflicts.rs | 47 +-- lib/tests/test_local_working_copy.rs | 293 +++++++++++++++--- .../test_local_working_copy_concurrent.rs | 49 ++- lib/tests/test_local_working_copy_sparse.rs | 25 +- 30 files changed, 797 insertions(+), 172 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3dd2150327..2d3e5c8150 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,6 +63,10 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - `op diff` - `restore` +* New `ui.conflict-marker-style` config option to change how conflicts are + materialized in the working copy. The default option ("diff") renders + conflicts as a snapshot with a list of diffs to apply to the snapshot. + ### Fixed bugs * `jj config unset ` no longer removes a table (such as `[ui]`.) diff --git a/cli/examples/custom-working-copy/main.rs b/cli/examples/custom-working-copy/main.rs index 276ad1fd83..7ddd5011d3 100644 --- a/cli/examples/custom-working-copy/main.rs +++ b/cli/examples/custom-working-copy/main.rs @@ -35,6 +35,7 @@ use jj_lib::settings::UserSettings; use jj_lib::signing::Signer; use jj_lib::store::Store; use jj_lib::working_copy::CheckoutError; +use jj_lib::working_copy::CheckoutOptions; use jj_lib::working_copy::CheckoutStats; use jj_lib::working_copy::LockedWorkingCopy; use jj_lib::working_copy::ResetError; @@ -240,14 +241,18 @@ impl LockedWorkingCopy for LockedConflictsWorkingCopy { self.inner.snapshot(&options) } - fn check_out(&mut self, commit: &Commit) -> Result { + fn check_out( + &mut self, + commit: &Commit, + options: &CheckoutOptions, + ) -> Result { let conflicts = commit .tree()? .conflicts() .map(|(path, _value)| format!("{}\n", path.as_internal_file_string())) .join(""); std::fs::write(self.wc_path.join(".conflicts"), conflicts).unwrap(); - self.inner.check_out(commit) + self.inner.check_out(commit, options) } fn rename_workspace(&mut self, new_workspace_id: WorkspaceId) { @@ -269,8 +274,9 @@ impl LockedWorkingCopy for LockedConflictsWorkingCopy { fn set_sparse_patterns( &mut self, new_sparse_patterns: Vec, + options: &CheckoutOptions, ) -> Result { - self.inner.set_sparse_patterns(new_sparse_patterns) + self.inner.set_sparse_patterns(new_sparse_patterns, options) } fn finish( diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index cbba0a0a6f..f8f3675af4 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -58,6 +58,7 @@ use jj_lib::backend::TreeValue; use jj_lib::commit::Commit; use jj_lib::config::ConfigError; use jj_lib::config::ConfigNamePathBuf; +use jj_lib::conflicts::ConflictMarkerStyle; use jj_lib::file_util; use jj_lib::fileset; use jj_lib::fileset::FilesetDiagnostics; @@ -113,6 +114,7 @@ use jj_lib::str_util::StringPattern; use jj_lib::transaction::Transaction; use jj_lib::view::View; use jj_lib::working_copy; +use jj_lib::working_copy::CheckoutOptions; use jj_lib::working_copy::CheckoutStats; use jj_lib::working_copy::SnapshotOptions; use jj_lib::working_copy::WorkingCopy; @@ -457,6 +459,7 @@ impl CommandHelper { let stale_wc_commit = repo.store().get_commit(wc_commit_id)?; let mut workspace_command = self.workspace_helper_no_snapshot(ui)?; + let checkout_options = workspace_command.checkout_options(); let repo = workspace_command.repo().clone(); let (mut locked_ws, desired_wc_commit) = @@ -479,6 +482,7 @@ impl CommandHelper { repo.op_id().clone(), &stale_wc_commit, &desired_wc_commit, + &checkout_options, )?; // TODO: Share this code with new/checkout somehow. @@ -710,6 +714,7 @@ pub struct WorkspaceCommandEnvironment { workspace_id: WorkspaceId, immutable_heads_expression: Rc, short_prefixes_expression: Option>, + conflict_marker_style: ConflictMarkerStyle, } impl WorkspaceCommandEnvironment { @@ -730,6 +735,7 @@ impl WorkspaceCommandEnvironment { workspace_id: workspace.workspace_id().to_owned(), immutable_heads_expression: RevsetExpression::root(), short_prefixes_expression: None, + conflict_marker_style: command.settings().conflict_marker_style()?, }; env.immutable_heads_expression = env.load_immutable_heads_expression(ui)?; env.short_prefixes_expression = env.load_short_prefixes_expression(ui)?; @@ -791,6 +797,11 @@ impl WorkspaceCommandEnvironment { &self.immutable_heads_expression } + /// User-configured conflict marker style for materializing conflicts + pub fn conflict_marker_style(&self) -> ConflictMarkerStyle { + self.conflict_marker_style + } + fn load_immutable_heads_expression( &self, ui: &Ui, @@ -896,6 +907,7 @@ impl WorkspaceCommandEnvironment { self.revset_parse_context(), id_prefix_context, self.immutable_expression(), + self.conflict_marker_style, &self.command.data.commit_template_extensions, ) } @@ -1138,6 +1150,12 @@ impl WorkspaceCommandHelper { &self.env } + pub fn checkout_options(&self) -> CheckoutOptions { + CheckoutOptions { + conflict_marker_style: self.env.conflict_marker_style(), + } + } + pub fn unchecked_start_working_copy_mutation( &mut self, ) -> Result<(LockedWorkspace, Commit), CommandError> { @@ -1321,7 +1339,12 @@ to the current parents may contain changes from multiple commits. /// Creates textual diff renderer of the specified `formats`. pub fn diff_renderer(&self, formats: Vec) -> DiffRenderer<'_> { - DiffRenderer::new(self.repo().as_ref(), self.path_converter(), formats) + DiffRenderer::new( + self.repo().as_ref(), + self.path_converter(), + self.env.conflict_marker_style(), + formats, + ) } /// Loads textual diff renderer from the settings and command arguments. @@ -1354,13 +1377,20 @@ to the current parents may contain changes from multiple commits. tool_name: Option<&str>, ) -> Result { let base_ignores = self.base_ignores()?; + let conflict_marker_style = self.env.conflict_marker_style(); if let Some(name) = tool_name { - Ok(DiffEditor::with_name(name, self.settings(), base_ignores)?) + Ok(DiffEditor::with_name( + name, + self.settings(), + base_ignores, + conflict_marker_style, + )?) } else { Ok(DiffEditor::from_settings( ui, self.settings(), base_ignores, + conflict_marker_style, )?) } } @@ -1389,10 +1419,11 @@ to the current parents may contain changes from multiple commits. ui: &Ui, tool_name: Option<&str>, ) -> Result { + let conflict_marker_style = self.env.conflict_marker_style(); if let Some(name) = tool_name { - MergeEditor::with_name(name, self.settings()) + MergeEditor::with_name(name, self.settings(), conflict_marker_style) } else { - MergeEditor::from_settings(ui, self.settings()) + MergeEditor::from_settings(ui, self.settings(), conflict_marker_style) } } @@ -1720,6 +1751,7 @@ to the current parents may contain changes from multiple commits. .settings() .max_new_file_size() .map_err(snapshot_command_error)?; + let conflict_marker_style = self.env.conflict_marker_style(); let command = self.env.command.clone(); let mut locked_ws = self .workspace @@ -1787,6 +1819,7 @@ See https://martinvonz.github.io/jj/latest/working-copy/#stale-working-copy \ progress: progress.as_ref().map(|x| x as _), start_tracking_matcher: &auto_tracking_matcher, max_new_file_size, + conflict_marker_style, }) .map_err(snapshot_command_error)?; drop(progress); @@ -1842,11 +1875,13 @@ See https://martinvonz.github.io/jj/latest/working-copy/#stale-working-copy \ new_commit: &Commit, ) -> Result<(), CommandError> { assert!(self.may_update_working_copy); + let checkout_options = self.checkout_options(); let stats = update_working_copy( &self.user_repo.repo, &mut self.workspace, maybe_old_commit, new_commit, + &checkout_options, )?; if Some(new_commit) != maybe_old_commit { if let Some(mut formatter) = ui.status_formatter() { @@ -2101,7 +2136,7 @@ See https://martinvonz.github.io/jj/latest/working-copy/#stale-working-copy \ .collect(), )?; } - revset_util::warn_unresolvable_trunk(ui, new_repo, &self.env().revset_parse_context())?; + revset_util::warn_unresolvable_trunk(ui, new_repo, &self.env.revset_parse_context())?; Ok(()) } @@ -2409,18 +2444,22 @@ fn update_stale_working_copy( op_id: OperationId, stale_commit: &Commit, new_commit: &Commit, + options: &CheckoutOptions, ) -> Result { // The same check as start_working_copy_mutation(), but with the stale // working-copy commit. if stale_commit.tree_id() != locked_ws.locked_wc().old_tree_id() { return Err(user_error("Concurrent working copy operation. Try again.")); } - let stats = locked_ws.locked_wc().check_out(new_commit).map_err(|err| { - internal_error_with_message( - format!("Failed to check out commit {}", new_commit.id().hex()), - err, - ) - })?; + let stats = locked_ws + .locked_wc() + .check_out(new_commit, options) + .map_err(|err| { + internal_error_with_message( + format!("Failed to check out commit {}", new_commit.id().hex()), + err, + ) + })?; locked_ws.finish(op_id)?; Ok(stats) @@ -2616,13 +2655,19 @@ pub fn update_working_copy( workspace: &mut Workspace, old_commit: Option<&Commit>, new_commit: &Commit, + options: &CheckoutOptions, ) -> Result, CommandError> { let old_tree_id = old_commit.map(|commit| commit.tree_id().clone()); let stats = if Some(new_commit.tree_id()) != old_tree_id.as_ref() { // TODO: CheckoutError::ConcurrentCheckout should probably just result in a // warning for most commands (but be an error for the checkout command) let stats = workspace - .check_out(repo.op_id().clone(), old_tree_id.as_ref(), new_commit) + .check_out( + repo.op_id().clone(), + old_tree_id.as_ref(), + new_commit, + options, + ) .map_err(|err| { internal_error_with_message( format!("Failed to check out commit {}", new_commit.id().hex()), diff --git a/cli/src/commands/file/show.rs b/cli/src/commands/file/show.rs index 98619bb5e3..396acac2e2 100644 --- a/cli/src/commands/file/show.rs +++ b/cli/src/commands/file/show.rs @@ -127,7 +127,11 @@ fn write_tree_entries>( io::copy(&mut reader, &mut ui.stdout_formatter().as_mut())?; } MaterializedTreeValue::FileConflict { contents, .. } => { - materialize_merge_result(&contents, &mut ui.stdout_formatter())?; + materialize_merge_result( + &contents, + workspace_command.env().conflict_marker_style(), + &mut ui.stdout_formatter(), + )?; } MaterializedTreeValue::OtherConflict { id } => { ui.stdout_formatter().write_all(id.describe().as_bytes())?; diff --git a/cli/src/commands/file/track.rs b/cli/src/commands/file/track.rs index 30929372f4..e5d272902a 100644 --- a/cli/src/commands/file/track.rs +++ b/cli/src/commands/file/track.rs @@ -44,6 +44,7 @@ pub(crate) fn cmd_file_track( args: &FileTrackArgs, ) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; + let conflict_marker_style = workspace_command.env().conflict_marker_style(); let matcher = workspace_command .parse_file_patterns(ui, &args.paths)? .to_matcher(); @@ -57,6 +58,7 @@ pub(crate) fn cmd_file_track( progress: None, start_tracking_matcher: &matcher, max_new_file_size: command.settings().max_new_file_size()?, + conflict_marker_style, })?; let num_rebased = tx.repo_mut().rebase_descendants(command.settings())?; if num_rebased > 0 { diff --git a/cli/src/commands/file/untrack.rs b/cli/src/commands/file/untrack.rs index bbcf0dc5d1..b1afe6dba6 100644 --- a/cli/src/commands/file/untrack.rs +++ b/cli/src/commands/file/untrack.rs @@ -44,6 +44,7 @@ pub(crate) fn cmd_file_untrack( args: &FileUntrackArgs, ) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; + let conflict_marker_style = workspace_command.env().conflict_marker_style(); let store = workspace_command.repo().store().clone(); let matcher = workspace_command .parse_file_patterns(ui, &args.paths)? @@ -75,6 +76,7 @@ pub(crate) fn cmd_file_untrack( progress: None, start_tracking_matcher: &auto_tracking_matcher, max_new_file_size: command.settings().max_new_file_size()?, + conflict_marker_style, })?; if wc_tree_id != *new_commit.tree_id() { let wc_tree = store.get_root_tree(&wc_tree_id)?; diff --git a/cli/src/commands/operation/diff.rs b/cli/src/commands/operation/diff.rs index a98b1f0624..16622cc182 100644 --- a/cli/src/commands/operation/diff.rs +++ b/cli/src/commands/operation/diff.rs @@ -125,7 +125,9 @@ pub fn cmd_op_diff( let diff_renderer = { let formats = diff_formats_for_log(command.settings(), &args.diff_format, args.patch)?; let path_converter = workspace_env.path_converter(); - (!formats.is_empty()).then(|| DiffRenderer::new(merged_repo, path_converter, formats)) + let conflict_marker_style = workspace_env.conflict_marker_style(); + (!formats.is_empty()) + .then(|| DiffRenderer::new(merged_repo, path_converter, conflict_marker_style, formats)) }; let id_prefix_context = workspace_env.new_id_prefix_context(); let commit_summary_template = { diff --git a/cli/src/commands/operation/log.rs b/cli/src/commands/operation/log.rs index 72273b1b9c..22594340d3 100644 --- a/cli/src/commands/operation/log.rs +++ b/cli/src/commands/operation/log.rs @@ -167,8 +167,15 @@ fn do_op_log( )? }; let path_converter = workspace_env.path_converter(); - let diff_renderer = (!diff_formats.is_empty()) - .then(|| DiffRenderer::new(repo.as_ref(), path_converter, diff_formats.clone())); + let conflict_marker_style = workspace_env.conflict_marker_style(); + let diff_renderer = (!diff_formats.is_empty()).then(|| { + DiffRenderer::new( + repo.as_ref(), + path_converter, + conflict_marker_style, + diff_formats.clone(), + ) + }); show_op_diff( ui, diff --git a/cli/src/commands/operation/show.rs b/cli/src/commands/operation/show.rs index 6dd7d609de..2d79d62233 100644 --- a/cli/src/commands/operation/show.rs +++ b/cli/src/commands/operation/show.rs @@ -73,7 +73,15 @@ pub fn cmd_op_show( let diff_renderer = { let formats = diff_formats_for_log(command.settings(), &args.diff_format, args.patch)?; let path_converter = workspace_env.path_converter(); - (!formats.is_empty()).then(|| DiffRenderer::new(repo.as_ref(), path_converter, formats)) + let conflict_marker_style = workspace_env.conflict_marker_style(); + (!formats.is_empty()).then(|| { + DiffRenderer::new( + repo.as_ref(), + path_converter, + conflict_marker_style, + formats, + ) + }) }; // TODO: Should we make this customizable via clap arg? diff --git a/cli/src/commands/sparse.rs b/cli/src/commands/sparse.rs index 8faf56b0d4..e78f4a80e4 100644 --- a/cli/src/commands/sparse.rs +++ b/cli/src/commands/sparse.rs @@ -211,11 +211,12 @@ fn update_sparse_patterns_with( workspace_command: &mut WorkspaceCommandHelper, f: impl FnOnce(&mut Ui, &[RepoPathBuf]) -> Result, CommandError>, ) -> Result<(), CommandError> { + let checkout_options = workspace_command.checkout_options(); let (mut locked_ws, wc_commit) = workspace_command.start_working_copy_mutation()?; let new_patterns = f(ui, locked_ws.locked_wc().sparse_patterns()?)?; let stats = locked_ws .locked_wc() - .set_sparse_patterns(new_patterns) + .set_sparse_patterns(new_patterns, &checkout_options) .map_err(|err| internal_error_with_message("Failed to update working copy paths", err))?; let operation_id = locked_ws.locked_wc().old_operation_id().clone(); locked_ws.finish(operation_id)?; diff --git a/cli/src/commands/workspace/add.rs b/cli/src/commands/workspace/add.rs index 0d61d99b50..c77c97da46 100644 --- a/cli/src/commands/workspace/add.rs +++ b/cli/src/commands/workspace/add.rs @@ -146,10 +146,11 @@ pub fn cmd_workspace_add( }; if let Some(sparse_patterns) = sparsity { + let checkout_options = new_workspace_command.checkout_options(); let (mut locked_ws, _wc_commit) = new_workspace_command.start_working_copy_mutation()?; locked_ws .locked_wc() - .set_sparse_patterns(sparse_patterns) + .set_sparse_patterns(sparse_patterns, &checkout_options) .map_err(|err| internal_error_with_message("Failed to set sparse patterns", err))?; let operation_id = locked_ws.locked_wc().old_operation_id().clone(); locked_ws.finish(operation_id)?; diff --git a/cli/src/commit_templater.rs b/cli/src/commit_templater.rs index 68fa4bbce6..7b6b542382 100644 --- a/cli/src/commit_templater.rs +++ b/cli/src/commit_templater.rs @@ -24,6 +24,7 @@ use jj_lib::backend::BackendResult; use jj_lib::backend::ChangeId; use jj_lib::backend::CommitId; use jj_lib::commit::Commit; +use jj_lib::conflicts::ConflictMarkerStyle; use jj_lib::copies::CopiesTreeDiffEntry; use jj_lib::copies::CopyRecords; use jj_lib::extensions_map::ExtensionsMap; @@ -95,6 +96,7 @@ pub struct CommitTemplateLanguage<'repo> { revset_parse_context: RevsetParseContext<'repo>, id_prefix_context: &'repo IdPrefixContext, immutable_expression: Rc, + conflict_marker_style: ConflictMarkerStyle, build_fn_table: CommitTemplateBuildFnTable<'repo>, keyword_cache: CommitKeywordCache<'repo>, cache_extensions: ExtensionsMap, @@ -103,6 +105,7 @@ pub struct CommitTemplateLanguage<'repo> { impl<'repo> CommitTemplateLanguage<'repo> { /// Sets up environment where commit template will be transformed to /// evaluation tree. + #[allow(clippy::too_many_arguments)] pub fn new( repo: &'repo dyn Repo, path_converter: &'repo RepoPathUiConverter, @@ -110,6 +113,7 @@ impl<'repo> CommitTemplateLanguage<'repo> { revset_parse_context: RevsetParseContext<'repo>, id_prefix_context: &'repo IdPrefixContext, immutable_expression: Rc, + conflict_marker_style: ConflictMarkerStyle, extensions: &[impl AsRef], ) -> Self { let mut build_fn_table = CommitTemplateBuildFnTable::builtin(); @@ -129,6 +133,7 @@ impl<'repo> CommitTemplateLanguage<'repo> { revset_parse_context, id_prefix_context, immutable_expression, + conflict_marker_style, build_fn_table, keyword_cache: CommitKeywordCache::default(), cache_extensions, @@ -1526,6 +1531,7 @@ fn builtin_tree_diff_methods<'repo>() -> CommitTemplateBuildMethodFnMap<'repo, T }) .transpose()?; let path_converter = language.path_converter; + let conflict_marker_style = language.conflict_marker_style; let template = (self_property, context_property) .map(move |(diff, context)| { // TODO: load defaults from UserSettings? @@ -1543,6 +1549,7 @@ fn builtin_tree_diff_methods<'repo>() -> CommitTemplateBuildMethodFnMap<'repo, T tree_diff, path_converter, &options, + conflict_marker_style, ) }) }) @@ -1564,8 +1571,9 @@ fn builtin_tree_diff_methods<'repo>() -> CommitTemplateBuildMethodFnMap<'repo, T ) }) .transpose()?; + let conflict_marker_style = language.conflict_marker_style; let template = (self_property, context_property) - .map(|(diff, context)| { + .map(move |(diff, context)| { let options = diff_util::UnifiedDiffOptions { context: context.unwrap_or(diff_util::DEFAULT_CONTEXT_LINES), line_diff: diff_util::LineDiffOptions { @@ -1573,7 +1581,13 @@ fn builtin_tree_diff_methods<'repo>() -> CommitTemplateBuildMethodFnMap<'repo, T }, }; diff.into_formatted(move |formatter, store, tree_diff| { - diff_util::show_git_diff(formatter, store, tree_diff, &options) + diff_util::show_git_diff( + formatter, + store, + tree_diff, + &options, + conflict_marker_style, + ) }) }) .into_template(); @@ -1591,6 +1605,7 @@ fn builtin_tree_diff_methods<'repo>() -> CommitTemplateBuildMethodFnMap<'repo, T width_node, )?; let path_converter = language.path_converter; + let conflict_marker_style = language.conflict_marker_style; let template = (self_property, width_property) .map(move |(diff, width)| { let options = diff_util::DiffStatOptions { @@ -1606,6 +1621,7 @@ fn builtin_tree_diff_methods<'repo>() -> CommitTemplateBuildMethodFnMap<'repo, T path_converter, &options, width, + conflict_marker_style, ) }) }) diff --git a/cli/src/config-schema.json b/cli/src/config-schema.json index f5b3599929..b9c347018c 100644 --- a/cli/src/config-schema.json +++ b/cli/src/config-schema.json @@ -158,6 +158,14 @@ "merge-editor": { "type": "string", "description": "Tool to use for resolving three-way merges. Behavior for a given tool name can be configured in merge-tools.TOOL tables" + }, + "conflict-marker-style": { + "type": "string", + "description": "Conflict marker style to use when materializing conflicts in the working copy", + "enum": [ + "diff" + ], + "default": "diff" } } }, diff --git a/cli/src/config/misc.toml b/cli/src/config/misc.toml index 228d2ffaa1..262efbedc1 100644 --- a/cli/src/config/misc.toml +++ b/cli/src/config/misc.toml @@ -22,6 +22,7 @@ paginate = "auto" pager = { command = ["less", "-FRX"], env = { LESSCHARSET = "utf-8" } } log-word-wrap = false log-synthetic-elided-nodes = true +conflict-marker-style = "diff" [ui.movement] edit = false diff --git a/cli/src/diff_util.rs b/cli/src/diff_util.rs index ff34989252..6d593da665 100644 --- a/cli/src/diff_util.rs +++ b/cli/src/diff_util.rs @@ -35,6 +35,7 @@ use jj_lib::commit::Commit; use jj_lib::config::ConfigError; use jj_lib::conflicts::materialize_merge_result_to_bytes; use jj_lib::conflicts::materialized_diff_stream; +use jj_lib::conflicts::ConflictMarkerStyle; use jj_lib::conflicts::MaterializedTreeDiffEntry; use jj_lib::conflicts::MaterializedTreeValue; use jj_lib::copies::CopiesTreeDiffEntry; @@ -267,6 +268,7 @@ pub enum DiffRenderError { pub struct DiffRenderer<'a> { repo: &'a dyn Repo, path_converter: &'a RepoPathUiConverter, + conflict_marker_style: ConflictMarkerStyle, formats: Vec, } @@ -274,12 +276,14 @@ impl<'a> DiffRenderer<'a> { pub fn new( repo: &'a dyn Repo, path_converter: &'a RepoPathUiConverter, + conflict_marker_style: ConflictMarkerStyle, formats: Vec, ) -> Self { DiffRenderer { repo, - formats, path_converter, + conflict_marker_style, + formats, } } @@ -331,7 +335,15 @@ impl<'a> DiffRenderer<'a> { DiffFormat::Stat(options) => { let tree_diff = from_tree.diff_stream_with_copies(to_tree, matcher, copy_records); - show_diff_stat(formatter, store, tree_diff, path_converter, options, width)?; + show_diff_stat( + formatter, + store, + tree_diff, + path_converter, + options, + width, + self.conflict_marker_style, + )?; } DiffFormat::Types => { let tree_diff = @@ -346,12 +358,25 @@ impl<'a> DiffRenderer<'a> { DiffFormat::Git(options) => { let tree_diff = from_tree.diff_stream_with_copies(to_tree, matcher, copy_records); - show_git_diff(formatter, store, tree_diff, options)?; + show_git_diff( + formatter, + store, + tree_diff, + options, + self.conflict_marker_style, + )?; } DiffFormat::ColorWords(options) => { let tree_diff = from_tree.diff_stream_with_copies(to_tree, matcher, copy_records); - show_color_words_diff(formatter, store, tree_diff, path_converter, options)?; + show_color_words_diff( + formatter, + store, + tree_diff, + path_converter, + options, + self.conflict_marker_style, + )?; } DiffFormat::Tool(tool) => { match tool.diff_invocation_mode { @@ -365,12 +390,21 @@ impl<'a> DiffRenderer<'a> { tree_diff, path_converter, tool, + self.conflict_marker_style, ) } DiffToolMode::Dir => { let mut writer = formatter.raw()?; - generate_diff(ui, writer.as_mut(), from_tree, to_tree, matcher, tool) - .map_err(DiffRenderError::DiffGenerate) + generate_diff( + ui, + writer.as_mut(), + from_tree, + to_tree, + matcher, + tool, + self.conflict_marker_style, + ) + .map_err(DiffRenderError::DiffGenerate) } }?; } @@ -837,7 +871,11 @@ fn file_content_for_diff(reader: &mut dyn io::Read) -> io::Result { }) } -fn diff_content(path: &RepoPath, value: MaterializedTreeValue) -> io::Result { +fn diff_content( + path: &RepoPath, + value: MaterializedTreeValue, + conflict_marker_style: ConflictMarkerStyle, +) -> io::Result { match value { MaterializedTreeValue::Absent => Ok(FileContent::empty()), MaterializedTreeValue::AccessDenied(err) => Ok(FileContent { @@ -863,7 +901,7 @@ fn diff_content(path: &RepoPath, value: MaterializedTreeValue) -> io::Result Ok(FileContent { is_binary: false, - contents: materialize_merge_result_to_bytes(&contents).into(), + contents: materialize_merge_result_to_bytes(&contents, conflict_marker_style).into(), }), MaterializedTreeValue::OtherConflict { id } => Ok(FileContent { is_binary: false, @@ -902,6 +940,7 @@ pub fn show_color_words_diff( tree_diff: BoxStream, path_converter: &RepoPathUiConverter, options: &ColorWordsDiffOptions, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<(), DiffRenderError> { let mut diff_stream = materialized_diff_stream(store, tree_diff); async { @@ -937,7 +976,7 @@ pub fn show_color_words_diff( formatter.labeled("header"), "Added {description} {right_ui_path}:" )?; - let right_content = diff_content(right_path, right_value)?; + let right_content = diff_content(right_path, right_value, conflict_marker_style)?; if right_content.is_empty() { writeln!(formatter.labeled("empty"), " (empty)")?; } else if right_content.is_binary { @@ -999,8 +1038,8 @@ pub fn show_color_words_diff( ) } }; - let left_content = diff_content(left_path, left_value)?; - let right_content = diff_content(right_path, right_value)?; + let left_content = diff_content(left_path, left_value, conflict_marker_style)?; + let right_content = diff_content(right_path, right_value, conflict_marker_style)?; if left_path == right_path { writeln!( formatter.labeled("header"), @@ -1028,7 +1067,7 @@ pub fn show_color_words_diff( formatter.labeled("header"), "Removed {description} {right_ui_path}:" )?; - let left_content = diff_content(left_path, left_value)?; + let left_content = diff_content(left_path, left_value, conflict_marker_style)?; if left_content.is_empty() { writeln!(formatter.labeled("empty"), " (empty)")?; } else if left_content.is_binary { @@ -1050,18 +1089,18 @@ pub fn show_file_by_file_diff( tree_diff: BoxStream, path_converter: &RepoPathUiConverter, tool: &ExternalMergeTool, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<(), DiffRenderError> { - fn create_file( - path: &RepoPath, - wc_dir: &Path, - value: MaterializedTreeValue, - ) -> Result { + let create_file = |path: &RepoPath, + wc_dir: &Path, + value: MaterializedTreeValue| + -> Result { let fs_path = path.to_fs_path(wc_dir)?; std::fs::create_dir_all(fs_path.parent().unwrap())?; - let content = diff_content(path, value)?; + let content = diff_content(path, value, conflict_marker_style)?; std::fs::write(&fs_path, content.contents)?; Ok(fs_path) - } + }; let temp_dir = new_utf8_temp_dir("jj-diff-")?; let left_wc_dir = temp_dir.path().join("left"); @@ -1124,6 +1163,7 @@ struct GitDiffPart { fn git_diff_part( path: &RepoPath, value: MaterializedTreeValue, + conflict_marker_style: ConflictMarkerStyle, ) -> Result { const DUMMY_HASH: &str = "0000000000"; let mode; @@ -1176,7 +1216,8 @@ fn git_diff_part( hash = DUMMY_HASH.to_owned(); content = FileContent { is_binary: false, // TODO: are we sure this is never binary? - contents: materialize_merge_result_to_bytes(&contents).into(), + contents: materialize_merge_result_to_bytes(&contents, conflict_marker_style) + .into(), }; } MaterializedTreeValue::OtherConflict { id } => { @@ -1440,6 +1481,7 @@ pub fn show_git_diff( store: &Store, tree_diff: BoxStream, options: &UnifiedDiffOptions, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<(), DiffRenderError> { let mut diff_stream = materialized_diff_stream(store, tree_diff); async { @@ -1450,8 +1492,8 @@ pub fn show_git_diff( let right_path_string = right_path.as_internal_file_string(); let (left_value, right_value) = values?; - let left_part = git_diff_part(left_path, left_value)?; - let right_part = git_diff_part(right_path, right_value)?; + let left_part = git_diff_part(left_path, left_value, conflict_marker_style)?; + let right_part = git_diff_part(right_path, right_value, conflict_marker_style)?; formatter.with_label("file_header", |formatter| { writeln!( @@ -1625,6 +1667,7 @@ pub fn show_diff_stat( path_converter: &RepoPathUiConverter, options: &DiffStatOptions, display_width: usize, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<(), DiffRenderError> { let mut stats: Vec = vec![]; let mut unresolved_renames = HashSet::new(); @@ -1637,8 +1680,8 @@ pub fn show_diff_stat( let (left, right) = values?; let left_path = path.source(); let right_path = path.target(); - let left_content = diff_content(left_path, left)?; - let right_content = diff_content(right_path, right)?; + let left_content = diff_content(left_path, left, conflict_marker_style)?; + let right_content = diff_content(right_path, right, conflict_marker_style)?; let left_ui_path = path_converter.format_file_path(left_path); let path = if left_path == right_path { diff --git a/cli/src/merge_tools/builtin.rs b/cli/src/merge_tools/builtin.rs index ebd0e7bee9..fc8b6703f2 100644 --- a/cli/src/merge_tools/builtin.rs +++ b/cli/src/merge_tools/builtin.rs @@ -14,6 +14,7 @@ use jj_lib::backend::MergedTreeId; use jj_lib::backend::TreeValue; use jj_lib::conflicts::materialize_merge_result_to_bytes; use jj_lib::conflicts::materialize_tree_value; +use jj_lib::conflicts::ConflictMarkerStyle; use jj_lib::conflicts::MaterializedTreeValue; use jj_lib::diff::Diff; use jj_lib::diff::DiffHunkKind; @@ -141,6 +142,7 @@ fn read_file_contents( store: &Store, tree: &MergedTree, path: &RepoPath, + conflict_marker_style: ConflictMarkerStyle, ) -> Result { let value = tree.path_value(path)?; let materialized_value = materialize_tree_value(store, path, value) @@ -211,7 +213,7 @@ fn read_file_contents( contents, executable: _, } => { - let buf = materialize_merge_result_to_bytes(&contents).into(); + let buf = materialize_merge_result_to_bytes(&contents, conflict_marker_style).into(); // TODO: Render the ID somehow? let contents = buf_to_file_contents(None, buf); Ok(FileInfo { @@ -309,11 +311,13 @@ pub fn make_diff_files( left_tree: &MergedTree, right_tree: &MergedTree, changed_files: &[RepoPathBuf], + conflict_marker_style: ConflictMarkerStyle, ) -> Result>, BuiltinToolError> { let mut files = Vec::new(); for changed_path in changed_files { - let left_info = read_file_contents(store, left_tree, changed_path)?; - let right_info = read_file_contents(store, right_tree, changed_path)?; + let left_info = read_file_contents(store, left_tree, changed_path, conflict_marker_style)?; + let right_info = + read_file_contents(store, right_tree, changed_path, conflict_marker_style)?; let mut sections = Vec::new(); if should_render_mode_section(&left_info, &right_info) { @@ -522,6 +526,7 @@ pub fn edit_diff_builtin( left_tree: &MergedTree, right_tree: &MergedTree, matcher: &dyn Matcher, + conflict_marker_style: ConflictMarkerStyle, ) -> Result { let store = left_tree.store().clone(); // TODO: handle copy tracking @@ -530,7 +535,13 @@ pub fn edit_diff_builtin( .map(|TreeDiffEntry { path, values }| values.map(|_| path)) .try_collect() .block_on()?; - let files = make_diff_files(&store, left_tree, right_tree, &changed_files)?; + let files = make_diff_files( + &store, + left_tree, + right_tree, + &changed_files, + conflict_marker_style, + )?; let mut input = scm_record::helpers::CrosstermInput; let recorder = scm_record::Recorder::new( scm_record::RecordState { @@ -696,7 +707,14 @@ mod tests { changed_path.to_owned(), added_path.to_owned(), ]; - let files = make_diff_files(store, &left_tree, &right_tree, &changed_files).unwrap(); + let files = make_diff_files( + store, + &left_tree, + &right_tree, + &changed_files, + ConflictMarkerStyle::Diff, + ) + .unwrap(); insta::assert_debug_snapshot!(files, @r###" [ File { @@ -826,7 +844,14 @@ mod tests { let right_tree = testutils::create_tree(&test_repo.repo, &[(added_empty_file_path, "")]); let changed_files = vec![added_empty_file_path.to_owned()]; - let files = make_diff_files(store, &left_tree, &right_tree, &changed_files).unwrap(); + let files = make_diff_files( + store, + &left_tree, + &right_tree, + &changed_files, + ConflictMarkerStyle::Diff, + ) + .unwrap(); insta::assert_debug_snapshot!(files, @r###" [ File { @@ -890,7 +915,14 @@ mod tests { let right_tree = testutils::create_tree(&test_repo.repo, &[]); let changed_files = vec![added_empty_file_path.to_owned()]; - let files = make_diff_files(store, &left_tree, &right_tree, &changed_files).unwrap(); + let files = make_diff_files( + store, + &left_tree, + &right_tree, + &changed_files, + ConflictMarkerStyle::Diff, + ) + .unwrap(); insta::assert_debug_snapshot!(files, @r###" [ File { @@ -955,7 +987,14 @@ mod tests { testutils::create_tree(&test_repo.repo, &[(empty_file_path, "modified\n")]); let changed_files = vec![empty_file_path.to_owned()]; - let files = make_diff_files(store, &left_tree, &right_tree, &changed_files).unwrap(); + let files = make_diff_files( + store, + &left_tree, + &right_tree, + &changed_files, + ConflictMarkerStyle::Diff, + ) + .unwrap(); insta::assert_debug_snapshot!(files, @r###" [ File { diff --git a/cli/src/merge_tools/diff_working_copies.rs b/cli/src/merge_tools/diff_working_copies.rs index 749255326d..a80b3b624c 100644 --- a/cli/src/merge_tools/diff_working_copies.rs +++ b/cli/src/merge_tools/diff_working_copies.rs @@ -8,6 +8,7 @@ use std::sync::Arc; use futures::StreamExt; use jj_lib::backend::MergedTreeId; +use jj_lib::conflicts::ConflictMarkerStyle; use jj_lib::fsmonitor::FsmonitorSettings; use jj_lib::gitignore::GitIgnoreFile; use jj_lib::local_working_copy::TreeState; @@ -19,6 +20,7 @@ use jj_lib::merged_tree::TreeDiffEntry; use jj_lib::repo_path::RepoPathBuf; use jj_lib::store::Store; use jj_lib::working_copy::CheckoutError; +use jj_lib::working_copy::CheckoutOptions; use jj_lib::working_copy::SnapshotOptions; use pollster::FutureExt; use tempfile::TempDir; @@ -84,12 +86,13 @@ fn check_out( state_dir: PathBuf, tree: &MergedTree, sparse_patterns: Vec, + options: &CheckoutOptions, ) -> Result { std::fs::create_dir(&wc_dir).map_err(DiffCheckoutError::SetUpDir)?; std::fs::create_dir(&state_dir).map_err(DiffCheckoutError::SetUpDir)?; let mut tree_state = TreeState::init(store, wc_dir, state_dir)?; - tree_state.set_sparse_patterns(sparse_patterns)?; - tree_state.check_out(tree)?; + tree_state.set_sparse_patterns(sparse_patterns, options)?; + tree_state.check_out(tree, options)?; Ok(tree_state) } @@ -135,6 +138,7 @@ pub(crate) fn check_out_trees( right_tree: &MergedTree, matcher: &dyn Matcher, output_is: Option, + options: &CheckoutOptions, ) -> Result { let changed_files: Vec<_> = left_tree .diff_stream(right_tree, matcher) @@ -153,6 +157,7 @@ pub(crate) fn check_out_trees( left_state_dir, left_tree, changed_files.clone(), + options, )?; let right_tree_state = check_out( store.clone(), @@ -160,6 +165,7 @@ pub(crate) fn check_out_trees( right_state_dir, right_tree, changed_files.clone(), + options, )?; let output_tree_state = output_is .map(|output_side| { @@ -174,6 +180,7 @@ pub(crate) fn check_out_trees( DiffSide::Right => right_tree, }, changed_files, + options, ) }) .transpose()?; @@ -200,8 +207,9 @@ impl DiffEditWorkingCopies { matcher: &dyn Matcher, output_is: Option, instructions: Option<&str>, + options: &CheckoutOptions, ) -> Result { - let diff_wc = check_out_trees(store, left_tree, right_tree, matcher, output_is)?; + let diff_wc = check_out_trees(store, left_tree, right_tree, matcher, output_is, options)?; let got_output_field = output_is.is_some(); set_readonly_recursively(diff_wc.left_working_copy_path()) @@ -273,6 +281,7 @@ diff editing in mind and be a little inaccurate. pub fn snapshot_results( self, base_ignores: Arc, + conflict_marker_style: ConflictMarkerStyle, ) -> Result { if let Some(path) = self.instructions_path_to_cleanup { std::fs::remove_file(path).ok(); @@ -289,6 +298,7 @@ diff editing in mind and be a little inaccurate. progress: None, start_tracking_matcher: &EverythingMatcher, max_new_file_size: u64::MAX, + conflict_marker_style, })?; Ok(output_tree_state.current_tree_id().clone()) } diff --git a/cli/src/merge_tools/external.rs b/cli/src/merge_tools/external.rs index 7476ac64d1..3bb1157f29 100644 --- a/cli/src/merge_tools/external.rs +++ b/cli/src/merge_tools/external.rs @@ -13,6 +13,7 @@ use jj_lib::backend::MergedTreeId; use jj_lib::backend::TreeValue; use jj_lib::conflicts; use jj_lib::conflicts::materialize_merge_result_to_bytes; +use jj_lib::conflicts::ConflictMarkerStyle; use jj_lib::gitignore::GitIgnoreFile; use jj_lib::matchers::Matcher; use jj_lib::merge::Merge; @@ -20,6 +21,7 @@ use jj_lib::merge::MergedTreeValue; use jj_lib::merged_tree::MergedTree; use jj_lib::merged_tree::MergedTreeBuilder; use jj_lib::repo_path::RepoPath; +use jj_lib::working_copy::CheckoutOptions; use pollster::FutureExt; use thiserror::Error; @@ -156,9 +158,10 @@ pub fn run_mergetool_external( repo_path: &RepoPath, conflict: MergedTreeValue, tree: &MergedTree, + conflict_marker_style: ConflictMarkerStyle, ) -> Result { let initial_output_content = if editor.merge_tool_edits_conflict_markers { - materialize_merge_result_to_bytes(&content) + materialize_merge_result_to_bytes(&content, conflict_marker_style) } else { BString::default() }; @@ -226,6 +229,7 @@ pub fn run_mergetool_external( tree.store(), repo_path, output_file_contents.as_slice(), + conflict_marker_style, ) .block_on()? } else { @@ -255,6 +259,7 @@ pub fn edit_diff_external( matcher: &dyn Matcher, instructions: Option<&str>, base_ignores: Arc, + options: &CheckoutOptions, ) -> Result { let got_output_field = find_all_variables(&editor.edit_args).contains(&"output"); let store = left_tree.store(); @@ -265,6 +270,7 @@ pub fn edit_diff_external( matcher, got_output_field.then_some(DiffSide::Right), instructions, + options, )?; let patterns = diffedit_wc.working_copies.to_command_variables(); @@ -283,7 +289,7 @@ pub fn edit_diff_external( })); } - diffedit_wc.snapshot_results(base_ignores) + diffedit_wc.snapshot_results(base_ignores, options.conflict_marker_style) } /// Generates textual diff by the specified `tool` and writes into `writer`. @@ -294,9 +300,13 @@ pub fn generate_diff( right_tree: &MergedTree, matcher: &dyn Matcher, tool: &ExternalMergeTool, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<(), DiffGenerateError> { let store = left_tree.store(); - let diff_wc = check_out_trees(store, left_tree, right_tree, matcher, None)?; + let options = CheckoutOptions { + conflict_marker_style, + }; + let diff_wc = check_out_trees(store, left_tree, right_tree, matcher, None, &options)?; set_readonly_recursively(diff_wc.left_working_copy_path()) .map_err(ExternalToolError::SetUpDir)?; set_readonly_recursively(diff_wc.right_working_copy_path()) diff --git a/cli/src/merge_tools/mod.rs b/cli/src/merge_tools/mod.rs index d3dbe2b757..abbafde506 100644 --- a/cli/src/merge_tools/mod.rs +++ b/cli/src/merge_tools/mod.rs @@ -21,6 +21,7 @@ use std::sync::Arc; use jj_lib::backend::MergedTreeId; use jj_lib::config::ConfigError; use jj_lib::conflicts::extract_as_single_hunk; +use jj_lib::conflicts::ConflictMarkerStyle; use jj_lib::gitignore::GitIgnoreFile; use jj_lib::matchers::Matcher; use jj_lib::merged_tree::MergedTree; @@ -29,6 +30,7 @@ use jj_lib::repo_path::RepoPath; use jj_lib::repo_path::RepoPathBuf; use jj_lib::settings::ConfigResultExt as _; use jj_lib::settings::UserSettings; +use jj_lib::working_copy::CheckoutOptions; use jj_lib::working_copy::SnapshotError; use pollster::FutureExt; use thiserror::Error; @@ -181,6 +183,7 @@ pub struct DiffEditor { tool: MergeTool, base_ignores: Arc, use_instructions: bool, + conflict_marker_style: ConflictMarkerStyle, } impl DiffEditor { @@ -190,10 +193,11 @@ impl DiffEditor { name: &str, settings: &UserSettings, base_ignores: Arc, + conflict_marker_style: ConflictMarkerStyle, ) -> Result { let tool = get_tool_config(settings, name)? .unwrap_or_else(|| MergeTool::external(ExternalMergeTool::with_program(name))); - Self::new_inner(tool, settings, base_ignores) + Self::new_inner(tool, settings, base_ignores, conflict_marker_style) } /// Loads the default diff editor from the settings. @@ -201,6 +205,7 @@ impl DiffEditor { ui: &Ui, settings: &UserSettings, base_ignores: Arc, + conflict_marker_style: ConflictMarkerStyle, ) -> Result { let args = editor_args_from_settings(ui, settings, "ui.diff-editor")?; let tool = if let CommandNameAndArgs::String(name) = &args { @@ -209,18 +214,20 @@ impl DiffEditor { None } .unwrap_or_else(|| MergeTool::external(ExternalMergeTool::with_edit_args(&args))); - Self::new_inner(tool, settings, base_ignores) + Self::new_inner(tool, settings, base_ignores, conflict_marker_style) } fn new_inner( tool: MergeTool, settings: &UserSettings, base_ignores: Arc, + conflict_marker_style: ConflictMarkerStyle, ) -> Result { Ok(DiffEditor { tool, base_ignores, use_instructions: settings.get_bool("ui.diff-instructions")?, + conflict_marker_style, }) } @@ -232,9 +239,15 @@ impl DiffEditor { matcher: &dyn Matcher, format_instructions: impl FnOnce() -> String, ) -> Result { + let checkout_options = CheckoutOptions { + conflict_marker_style: self.conflict_marker_style, + }; match &self.tool { MergeTool::Builtin => { - Ok(edit_diff_builtin(left_tree, right_tree, matcher).map_err(Box::new)?) + Ok( + edit_diff_builtin(left_tree, right_tree, matcher, self.conflict_marker_style) + .map_err(Box::new)?, + ) } MergeTool::External(editor) => { let instructions = self.use_instructions.then(format_instructions); @@ -245,6 +258,7 @@ impl DiffEditor { matcher, instructions.as_deref(), self.base_ignores.clone(), + &checkout_options, ) } } @@ -255,19 +269,28 @@ impl DiffEditor { #[derive(Clone, Debug)] pub struct MergeEditor { tool: MergeTool, + conflict_marker_style: ConflictMarkerStyle, } impl MergeEditor { /// Creates 3-way merge editor of the given name, and loads parameters from /// the settings. - pub fn with_name(name: &str, settings: &UserSettings) -> Result { + pub fn with_name( + name: &str, + settings: &UserSettings, + conflict_marker_style: ConflictMarkerStyle, + ) -> Result { let tool = get_tool_config(settings, name)? .unwrap_or_else(|| MergeTool::external(ExternalMergeTool::with_program(name))); - Self::new_inner(name, tool) + Self::new_inner(name, tool, conflict_marker_style) } /// Loads the default 3-way merge editor from the settings. - pub fn from_settings(ui: &Ui, settings: &UserSettings) -> Result { + pub fn from_settings( + ui: &Ui, + settings: &UserSettings, + conflict_marker_style: ConflictMarkerStyle, + ) -> Result { let args = editor_args_from_settings(ui, settings, "ui.merge-editor")?; let tool = if let CommandNameAndArgs::String(name) = &args { get_tool_config(settings, name)? @@ -275,16 +298,23 @@ impl MergeEditor { None } .unwrap_or_else(|| MergeTool::external(ExternalMergeTool::with_merge_args(&args))); - Self::new_inner(&args, tool) + Self::new_inner(&args, tool, conflict_marker_style) } - fn new_inner(name: impl ToString, tool: MergeTool) -> Result { + fn new_inner( + name: impl ToString, + tool: MergeTool, + conflict_marker_style: ConflictMarkerStyle, + ) -> Result { if matches!(&tool, MergeTool::External(mergetool) if mergetool.merge_args.is_empty()) { return Err(MergeToolConfigError::MergeArgsNotConfigured { tool_name: name.to_string(), }); } - Ok(MergeEditor { tool }) + Ok(MergeEditor { + tool, + conflict_marker_style, + }) } /// Starts a merge editor for the specified file. @@ -319,7 +349,13 @@ impl MergeEditor { Ok(tree_id) } MergeTool::External(editor) => external::run_mergetool_external( - editor, file_merge, content, repo_path, conflict, tree, + editor, + file_merge, + content, + repo_path, + conflict, + tree, + self.conflict_marker_style, ), } } @@ -343,7 +379,13 @@ mod tests { let get = |name, config_text| { let config = config_from_string(config_text); let settings = UserSettings::from_config(config); - DiffEditor::with_name(name, &settings, GitIgnoreFile::empty()).map(|editor| editor.tool) + DiffEditor::with_name( + name, + &settings, + GitIgnoreFile::empty(), + ConflictMarkerStyle::Diff, + ) + .map(|editor| editor.tool) }; insta::assert_debug_snapshot!(get(":builtin", "").unwrap(), @"Builtin"); @@ -402,8 +444,13 @@ mod tests { let config = config_from_string(text); let ui = Ui::with_config(&config).unwrap(); let settings = UserSettings::from_config(config); - DiffEditor::from_settings(&ui, &settings, GitIgnoreFile::empty()) - .map(|editor| editor.tool) + DiffEditor::from_settings( + &ui, + &settings, + GitIgnoreFile::empty(), + ConflictMarkerStyle::Diff, + ) + .map(|editor| editor.tool) }; // Default @@ -557,7 +604,8 @@ mod tests { let get = |name, config_text| { let config = config_from_string(config_text); let settings = UserSettings::from_config(config); - MergeEditor::with_name(name, &settings).map(|editor| editor.tool) + MergeEditor::with_name(name, &settings, ConflictMarkerStyle::Diff) + .map(|editor| editor.tool) }; insta::assert_debug_snapshot!(get(":builtin", "").unwrap(), @"Builtin"); @@ -606,7 +654,8 @@ mod tests { let config = config_from_string(text); let ui = Ui::with_config(&config).unwrap(); let settings = UserSettings::from_config(config); - MergeEditor::from_settings(&ui, &settings).map(|editor| editor.tool) + MergeEditor::from_settings(&ui, &settings, ConflictMarkerStyle::Diff) + .map(|editor| editor.tool) }; // Default diff --git a/lib/src/annotate.rs b/lib/src/annotate.rs index 41b354ef70..edb08bc883 100644 --- a/lib/src/annotate.rs +++ b/lib/src/annotate.rs @@ -34,6 +34,7 @@ use crate::backend::CommitId; use crate::commit::Commit; use crate::conflicts::materialize_merge_result_to_bytes; use crate::conflicts::materialize_tree_value; +use crate::conflicts::ConflictMarkerStyle; use crate::conflicts::MaterializedTreeValue; use crate::diff::Diff; use crate::diff::DiffHunkKind; @@ -358,9 +359,9 @@ fn get_file_contents( })?; Ok(file_contents.into()) } - MaterializedTreeValue::FileConflict { contents, .. } => { - Ok(materialize_merge_result_to_bytes(&contents)) - } + MaterializedTreeValue::FileConflict { contents, .. } => Ok( + materialize_merge_result_to_bytes(&contents, ConflictMarkerStyle::default()), + ), _ => Ok(BString::default()), } } diff --git a/lib/src/conflicts.rs b/lib/src/conflicts.rs index 0417422e28..287bd29d9c 100644 --- a/lib/src/conflicts.rs +++ b/lib/src/conflicts.rs @@ -233,31 +233,50 @@ async fn materialize_tree_value_no_access_denied( } } +/// Describes what style should be used when materializing conflicts. +#[derive(Clone, Copy, PartialEq, Eq, Debug, Default, serde::Deserialize)] +#[serde(rename_all = "kebab-case")] +pub enum ConflictMarkerStyle { + /// Style which shows a snapshot and a series of diffs to apply. + #[default] + Diff, +} + pub fn materialize_merge_result>( single_hunk: &Merge, + conflict_marker_style: ConflictMarkerStyle, output: &mut dyn Write, ) -> io::Result<()> { let merge_result = files::merge(single_hunk); match &merge_result { MergeResult::Resolved(content) => output.write_all(content), - MergeResult::Conflict(hunks) => materialize_conflict_hunks(hunks, output), + MergeResult::Conflict(hunks) => { + materialize_conflict_hunks(hunks, conflict_marker_style, output) + } } } -pub fn materialize_merge_result_to_bytes>(single_hunk: &Merge) -> BString { +pub fn materialize_merge_result_to_bytes>( + single_hunk: &Merge, + conflict_marker_style: ConflictMarkerStyle, +) -> BString { let merge_result = files::merge(single_hunk); match merge_result { MergeResult::Resolved(content) => content, MergeResult::Conflict(hunks) => { let mut output = Vec::new(); - materialize_conflict_hunks(&hunks, &mut output) + materialize_conflict_hunks(&hunks, conflict_marker_style, &mut output) .expect("writing to an in-memory buffer should never fail"); output.into() } } } -fn materialize_conflict_hunks(hunks: &[Merge], output: &mut dyn Write) -> io::Result<()> { +fn materialize_conflict_hunks( + hunks: &[Merge], + _conflict_marker_style: ConflictMarkerStyle, + output: &mut dyn Write, +) -> io::Result<()> { let num_conflicts = hunks .iter() .filter(|hunk| hunk.as_resolved().is_none()) @@ -514,6 +533,7 @@ pub async fn update_from_content( store: &Store, path: &RepoPath, content: &[u8], + conflict_marker_style: ConflictMarkerStyle, ) -> BackendResult>> { let simplified_file_ids = file_ids.clone().simplify(); let simplified_file_ids = &simplified_file_ids; @@ -525,7 +545,7 @@ pub async fn update_from_content( // copy. let mut old_content = Vec::with_capacity(content.len()); let merge_hunk = extract_as_single_hunk(simplified_file_ids, store, path).await?; - materialize_merge_result(&merge_hunk, &mut old_content).unwrap(); + materialize_merge_result(&merge_hunk, conflict_marker_style, &mut old_content).unwrap(); if content == old_content { return Ok(file_ids.clone()); } diff --git a/lib/src/default_index/revset_engine.rs b/lib/src/default_index/revset_engine.rs index 10e50473bc..8994d56fca 100644 --- a/lib/src/default_index/revset_engine.rs +++ b/lib/src/default_index/revset_engine.rs @@ -44,6 +44,7 @@ use crate::backend::MillisSinceEpoch; use crate::commit::Commit; use crate::conflicts::materialize_merge_result_to_bytes; use crate::conflicts::materialize_tree_value; +use crate::conflicts::ConflictMarkerStyle; use crate::conflicts::MaterializedTreeValue; use crate::default_index::AsCompositeIndex; use crate::default_index::CompositeIndex; @@ -1346,7 +1347,7 @@ fn to_file_content(path: &RepoPath, value: MaterializedTreeValue) -> BackendResu MaterializedTreeValue::Symlink { id: _, target } => Ok(target.into_bytes()), MaterializedTreeValue::GitSubmodule(_) => Ok(vec![]), MaterializedTreeValue::FileConflict { contents, .. } => { - Ok(materialize_merge_result_to_bytes(&contents).into()) + Ok(materialize_merge_result_to_bytes(&contents, ConflictMarkerStyle::default()).into()) } MaterializedTreeValue::OtherConflict { .. } => Ok(vec![]), MaterializedTreeValue::Tree(id) => { diff --git a/lib/src/local_working_copy.rs b/lib/src/local_working_copy.rs index 3b4e6b701f..6ea1ab092e 100644 --- a/lib/src/local_working_copy.rs +++ b/lib/src/local_working_copy.rs @@ -63,6 +63,7 @@ use crate::commit::Commit; use crate::conflicts; use crate::conflicts::materialize_merge_result_to_bytes; use crate::conflicts::materialize_tree_value; +use crate::conflicts::ConflictMarkerStyle; use crate::conflicts::MaterializedTreeValue; use crate::file_util::check_symlink_support; use crate::file_util::try_symlink; @@ -95,6 +96,7 @@ use crate::settings::HumanByteSize; use crate::store::Store; use crate::tree::Tree; use crate::working_copy::CheckoutError; +use crate::working_copy::CheckoutOptions; use crate::working_copy::CheckoutStats; use crate::working_copy::LockedWorkingCopy; use crate::working_copy::ResetError; @@ -906,6 +908,7 @@ impl TreeState { progress, start_tracking_matcher, max_new_file_size, + conflict_marker_style, } = options; let sparse_matcher = self.sparse_matcher(); @@ -950,6 +953,7 @@ impl TreeState { directory_to_visit, *progress, *max_new_file_size, + *conflict_marker_style, ) })?; @@ -1024,6 +1028,7 @@ impl TreeState { directory_to_visit: DirectoryToVisit, progress: Option<&SnapshotProgress>, max_new_file_size: u64, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<(), SnapshotError> { let DirectoryToVisit { dir, @@ -1109,6 +1114,7 @@ impl TreeState { Some(¤t_file_state), current_tree, &new_file_state, + conflict_marker_style, )?; if let Some(tree_value) = update { tree_entries_tx @@ -1139,6 +1145,7 @@ impl TreeState { directory_to_visit, progress, max_new_file_size, + conflict_marker_style, )?; } } else if matcher.matches(&path) { @@ -1177,6 +1184,7 @@ impl TreeState { maybe_current_file_state.as_ref(), current_tree, &new_file_state, + conflict_marker_style, )?; if let Some(tree_value) = update { tree_entries_tx.send((path.clone(), tree_value)).ok(); @@ -1245,6 +1253,7 @@ impl TreeState { maybe_current_file_state: Option<&FileState>, current_tree: &MergedTree, new_file_state: &FileState, + conflict_marker_style: ConflictMarkerStyle, ) -> Result, SnapshotError> { let clean = match maybe_current_file_state { None => { @@ -1274,7 +1283,13 @@ impl TreeState { }; let new_tree_values = match new_file_type { FileType::Normal { executable } => self - .write_path_to_store(repo_path, &disk_path, ¤t_tree_values, executable) + .write_path_to_store( + repo_path, + &disk_path, + ¤t_tree_values, + executable, + conflict_marker_style, + ) .block_on()?, FileType::Symlink => { let id = self @@ -1298,6 +1313,7 @@ impl TreeState { disk_path: &Path, current_tree_values: &MergedTreeValue, executable: FileExecutableFlag, + conflict_marker_style: ConflictMarkerStyle, ) -> Result { // If the file contained a conflict before and is now a normal file on disk, we // try to parse any conflict markers in the file into a conflict. @@ -1326,6 +1342,7 @@ impl TreeState { self.store.as_ref(), repo_path, &content, + conflict_marker_style, ) .block_on()?; match new_file_ids.into_resolved() { @@ -1441,7 +1458,11 @@ impl TreeState { Ok(()) } - pub fn check_out(&mut self, new_tree: &MergedTree) -> Result { + pub fn check_out( + &mut self, + new_tree: &MergedTree, + options: &CheckoutOptions, + ) -> Result { let old_tree = self.current_tree().map_err(|err| match err { err @ BackendError::ObjectNotFound { .. } => CheckoutError::SourceNotFound { source: Box::new(err), @@ -1449,7 +1470,12 @@ impl TreeState { other => CheckoutError::InternalBackendError(other), })?; let stats = self - .update(&old_tree, new_tree, self.sparse_matcher().as_ref()) + .update( + &old_tree, + new_tree, + self.sparse_matcher().as_ref(), + options.conflict_marker_style, + ) .block_on()?; self.tree_id = new_tree.id(); Ok(stats) @@ -1458,6 +1484,7 @@ impl TreeState { pub fn set_sparse_patterns( &mut self, sparse_patterns: Vec, + options: &CheckoutOptions, ) -> Result { let tree = self.current_tree().map_err(|err| match err { err @ BackendError::ObjectNotFound { .. } => CheckoutError::SourceNotFound { @@ -1470,9 +1497,21 @@ impl TreeState { let added_matcher = DifferenceMatcher::new(&new_matcher, &old_matcher); let removed_matcher = DifferenceMatcher::new(&old_matcher, &new_matcher); let empty_tree = MergedTree::resolved(Tree::empty(self.store.clone(), RepoPathBuf::root())); - let added_stats = self.update(&empty_tree, &tree, &added_matcher).block_on()?; + let added_stats = self + .update( + &empty_tree, + &tree, + &added_matcher, + options.conflict_marker_style, + ) + .block_on()?; let removed_stats = self - .update(&tree, &empty_tree, &removed_matcher) + .update( + &tree, + &empty_tree, + &removed_matcher, + options.conflict_marker_style, + ) .block_on()?; self.sparse_patterns = sparse_patterns; assert_eq!(added_stats.updated_files, 0); @@ -1493,6 +1532,7 @@ impl TreeState { old_tree: &MergedTree, new_tree: &MergedTree, matcher: &dyn Matcher, + conflict_marker_style: ConflictMarkerStyle, ) -> Result { // TODO: maybe it's better not include the skipped counts in the "intended" // counts @@ -1597,7 +1637,8 @@ impl TreeState { contents, executable, } => { - let data = materialize_merge_result_to_bytes(&contents).into(); + let data = + materialize_merge_result_to_bytes(&contents, conflict_marker_style).into(); self.write_conflict(&disk_path, data, executable)? } MaterializedTreeValue::OtherConflict { id } => { @@ -1982,7 +2023,11 @@ impl LockedWorkingCopy for LockedLocalWorkingCopy { Ok(tree_state.current_tree_id().clone()) } - fn check_out(&mut self, commit: &Commit) -> Result { + fn check_out( + &mut self, + commit: &Commit, + options: &CheckoutOptions, + ) -> Result { // TODO: Write a "pending_checkout" file with the new TreeId so we can // continue an interrupted update if we find such a file. let new_tree = commit.tree()?; @@ -1993,7 +2038,7 @@ impl LockedWorkingCopy for LockedLocalWorkingCopy { message: "Failed to load the working copy state".to_string(), err: err.into(), })? - .check_out(&new_tree)?; + .check_out(&new_tree, options)?; self.tree_state_dirty = true; Ok(stats) } @@ -2037,6 +2082,7 @@ impl LockedWorkingCopy for LockedLocalWorkingCopy { fn set_sparse_patterns( &mut self, new_sparse_patterns: Vec, + options: &CheckoutOptions, ) -> Result { // TODO: Write a "pending_checkout" file with new sparse patterns so we can // continue an interrupted update if we find such a file. @@ -2047,7 +2093,7 @@ impl LockedWorkingCopy for LockedLocalWorkingCopy { message: "Failed to load the working copy state".to_string(), err: err.into(), })? - .set_sparse_patterns(new_sparse_patterns)?; + .set_sparse_patterns(new_sparse_patterns, options)?; self.tree_state_dirty = true; Ok(stats) } diff --git a/lib/src/settings.rs b/lib/src/settings.rs index 61fb5f9533..f8b12c5ebd 100644 --- a/lib/src/settings.rs +++ b/lib/src/settings.rs @@ -28,6 +28,7 @@ use crate::backend::Commit; use crate::backend::Signature; use crate::backend::Timestamp; use crate::config::ConfigError; +use crate::conflicts::ConflictMarkerStyle; use crate::fmt_util::binary_prefix; use crate::fsmonitor::FsmonitorSettings; use crate::signing::SignBehavior; @@ -254,6 +255,13 @@ impl UserSettings { pub fn sign_settings(&self) -> SignSettings { SignSettings::from_settings(self) } + + pub fn conflict_marker_style(&self) -> Result { + Ok(self + .get("ui.conflict-marker-style") + .optional()? + .unwrap_or_default()) + } } /// General-purpose accessors. diff --git a/lib/src/working_copy.rs b/lib/src/working_copy.rs index 4555e7cd28..4f04190268 100644 --- a/lib/src/working_copy.rs +++ b/lib/src/working_copy.rs @@ -27,6 +27,7 @@ use tracing::instrument; use crate::backend::BackendError; use crate::backend::MergedTreeId; use crate::commit::Commit; +use crate::conflicts::ConflictMarkerStyle; use crate::dag_walk; use crate::fsmonitor::FsmonitorSettings; use crate::gitignore::GitIgnoreError; @@ -116,7 +117,11 @@ pub trait LockedWorkingCopy { fn snapshot(&mut self, options: &SnapshotOptions) -> Result; /// Check out the specified commit in the working copy. - fn check_out(&mut self, commit: &Commit) -> Result; + fn check_out( + &mut self, + commit: &Commit, + options: &CheckoutOptions, + ) -> Result; /// Update the workspace name. fn rename_workspace(&mut self, new_workspace_name: WorkspaceId); @@ -140,6 +145,7 @@ pub trait LockedWorkingCopy { fn set_sparse_patterns( &mut self, new_sparse_patterns: Vec, + options: &CheckoutOptions, ) -> Result; /// Finish the modifications to the working copy by writing the updated @@ -222,6 +228,8 @@ pub struct SnapshotOptions<'a> { /// (depending on implementation) /// return `SnapshotError::NewFileTooLarge`. pub max_new_file_size: u64, + /// Expected conflict marker style for checking for changed files. + pub conflict_marker_style: ConflictMarkerStyle, } impl SnapshotOptions<'_> { @@ -233,6 +241,7 @@ impl SnapshotOptions<'_> { progress: None, start_tracking_matcher: &EverythingMatcher, max_new_file_size: u64::MAX, + conflict_marker_style: ConflictMarkerStyle::default(), } } } @@ -240,6 +249,22 @@ impl SnapshotOptions<'_> { /// A callback for getting progress updates. pub type SnapshotProgress<'a> = dyn Fn(&RepoPath) + 'a + Sync; +/// Options used when checking out a tree in the working copy. +#[derive(Clone)] +pub struct CheckoutOptions { + /// Conflict marker style to use when materializing files + pub conflict_marker_style: ConflictMarkerStyle, +} + +impl CheckoutOptions { + /// Create an instance for use in tests. + pub fn empty_for_test() -> Self { + CheckoutOptions { + conflict_marker_style: ConflictMarkerStyle::default(), + } + } +} + /// Stats about a checkout operation on a working copy. All "files" mentioned /// below may also be symlinks or materialized conflicts. #[derive(Debug, PartialEq, Eq, Clone)] diff --git a/lib/src/workspace.rs b/lib/src/workspace.rs index 801e7115fc..a232662c7b 100644 --- a/lib/src/workspace.rs +++ b/lib/src/workspace.rs @@ -54,6 +54,7 @@ use crate::signing::SignInitError; use crate::signing::Signer; use crate::store::Store; use crate::working_copy::CheckoutError; +use crate::working_copy::CheckoutOptions; use crate::working_copy::CheckoutStats; use crate::working_copy::LockedWorkingCopy; use crate::working_copy::WorkingCopy; @@ -433,6 +434,7 @@ impl Workspace { operation_id: OperationId, old_tree_id: Option<&MergedTreeId>, commit: &Commit, + options: &CheckoutOptions, ) -> Result { let mut locked_ws = self.start_working_copy_mutation() @@ -449,7 +451,7 @@ impl Workspace { return Err(CheckoutError::ConcurrentCheckout); } } - let stats = locked_ws.locked_wc().check_out(commit)?; + let stats = locked_ws.locked_wc().check_out(commit, options)?; locked_ws .finish(operation_id) .map_err(|err| CheckoutError::Other { diff --git a/lib/tests/test_conflicts.rs b/lib/tests/test_conflicts.rs index b6f7a9371e..40518a5af9 100644 --- a/lib/tests/test_conflicts.rs +++ b/lib/tests/test_conflicts.rs @@ -18,6 +18,7 @@ use jj_lib::conflicts::extract_as_single_hunk; use jj_lib::conflicts::materialize_merge_result_to_bytes; use jj_lib::conflicts::parse_conflict; use jj_lib::conflicts::update_from_content; +use jj_lib::conflicts::ConflictMarkerStyle; use jj_lib::merge::Merge; use jj_lib::repo::Repo; use jj_lib::repo_path::RepoPath; @@ -74,7 +75,7 @@ fn test_materialize_conflict_basic() { vec![Some(left_id.clone()), Some(right_id.clone())], ); insta::assert_snapshot!( - &materialize_conflict_string(store, path, &conflict), + &materialize_conflict_string(store, path, &conflict, ConflictMarkerStyle::Diff), @r###" line 1 line 2 @@ -98,7 +99,7 @@ fn test_materialize_conflict_basic() { vec![Some(right_id.clone()), Some(left_id.clone())], ); insta::assert_snapshot!( - &materialize_conflict_string(store, path, &conflict), + &materialize_conflict_string(store, path, &conflict, ConflictMarkerStyle::Diff), @r###" line 1 line 2 @@ -171,7 +172,7 @@ fn test_materialize_conflict_multi_rebase_conflicts() { vec![Some(a_id.clone()), Some(b_id.clone()), Some(c_id.clone())], ); insta::assert_snapshot!( - &materialize_conflict_string(store, path, &conflict), + &materialize_conflict_string(store, path, &conflict, ConflictMarkerStyle::Diff), @r###" line 1 <<<<<<< Conflict 1 of 1 @@ -195,7 +196,7 @@ fn test_materialize_conflict_multi_rebase_conflicts() { vec![Some(c_id.clone()), Some(b_id.clone()), Some(a_id.clone())], ); insta::assert_snapshot!( - &materialize_conflict_string(store, path, &conflict), + &materialize_conflict_string(store, path, &conflict, ConflictMarkerStyle::Diff), @r###" line 1 <<<<<<< Conflict 1 of 1 @@ -219,7 +220,7 @@ fn test_materialize_conflict_multi_rebase_conflicts() { vec![Some(c_id.clone()), Some(a_id.clone()), Some(b_id.clone())], ); insta::assert_snapshot!( - &materialize_conflict_string(store, path, &conflict), + &materialize_conflict_string(store, path, &conflict, ConflictMarkerStyle::Diff), @r###" line 1 <<<<<<< Conflict 1 of 1 @@ -285,7 +286,8 @@ fn test_materialize_parse_roundtrip() { vec![Some(base_id.clone())], vec![Some(left_id.clone()), Some(right_id.clone())], ); - let materialized = materialize_conflict_string(store, path, &conflict); + let materialized = + materialize_conflict_string(store, path, &conflict, ConflictMarkerStyle::Diff); insta::assert_snapshot!( materialized, @r###" @@ -353,7 +355,8 @@ fn test_materialize_conflict_no_newlines_at_eof() { vec![Some(base_id.clone())], vec![Some(left_empty_id.clone()), Some(right_id.clone())], ); - let materialized = &materialize_conflict_string(store, path, &conflict); + let materialized = + &materialize_conflict_string(store, path, &conflict, ConflictMarkerStyle::Diff); insta::assert_snapshot!(materialized, @r###" <<<<<<< Conflict 1 of 1 @@ -413,7 +416,7 @@ fn test_materialize_conflict_modify_delete() { vec![Some(base_id.clone())], vec![Some(modified_id.clone()), Some(deleted_id.clone())], ); - insta::assert_snapshot!(&materialize_conflict_string(store, path, &conflict), @r###" + insta::assert_snapshot!(&materialize_conflict_string(store, path, &conflict, ConflictMarkerStyle::Diff), @r###" line 1 line 2 <<<<<<< Conflict 1 of 1 @@ -432,7 +435,7 @@ fn test_materialize_conflict_modify_delete() { vec![Some(base_id.clone())], vec![Some(deleted_id.clone()), Some(modified_id.clone())], ); - insta::assert_snapshot!(&materialize_conflict_string(store, path, &conflict), @r###" + insta::assert_snapshot!(&materialize_conflict_string(store, path, &conflict, ConflictMarkerStyle::Diff), @r###" line 1 line 2 <<<<<<< Conflict 1 of 1 @@ -451,7 +454,7 @@ fn test_materialize_conflict_modify_delete() { vec![Some(base_id.clone())], vec![Some(modified_id.clone()), None], ); - insta::assert_snapshot!(&materialize_conflict_string(store, path, &conflict), @r###" + insta::assert_snapshot!(&materialize_conflict_string(store, path, &conflict, ConflictMarkerStyle::Diff), @r###" <<<<<<< Conflict 1 of 1 %%%%%%% Changes from base to side #1 line 1 @@ -503,7 +506,7 @@ fn test_materialize_conflict_two_forward_diffs() { ], ); insta::assert_snapshot!( - &materialize_conflict_string(store, path, &conflict), + &materialize_conflict_string(store, path, &conflict, ConflictMarkerStyle::Diff), @r###" <<<<<<< Conflict 1 of 1 +++++++ Contents of side #1 @@ -919,9 +922,10 @@ fn test_update_conflict_from_content() { // If the content is unchanged compared to the materialized value, we get the // old conflict id back. - let materialized = materialize_conflict_string(store, path, &conflict); + let materialized = + materialize_conflict_string(store, path, &conflict, ConflictMarkerStyle::Diff); let parse = |content| { - update_from_content(&conflict, store, path, content) + update_from_content(&conflict, store, path, content, ConflictMarkerStyle::Diff) .block_on() .unwrap() }; @@ -968,9 +972,10 @@ fn test_update_conflict_from_content_modify_delete() { // If the content is unchanged compared to the materialized value, we get the // old conflict id back. - let materialized = materialize_conflict_string(store, path, &conflict); + let materialized = + materialize_conflict_string(store, path, &conflict, ConflictMarkerStyle::Diff); let parse = |content| { - update_from_content(&conflict, store, path, content) + update_from_content(&conflict, store, path, content, ConflictMarkerStyle::Diff) .block_on() .unwrap() }; @@ -1021,10 +1026,12 @@ fn test_update_conflict_from_content_simplified_conflict() { // If the content is unchanged compared to the materialized value, we get the // old conflict id back. Both the simplified and unsimplified materialized // conflicts should return the old conflict id. - let materialized = materialize_conflict_string(store, path, &conflict); - let materialized_simplified = materialize_conflict_string(store, path, &simplified_conflict); + let materialized = + materialize_conflict_string(store, path, &conflict, ConflictMarkerStyle::Diff); + let materialized_simplified = + materialize_conflict_string(store, path, &simplified_conflict, ConflictMarkerStyle::Diff); let parse = |content| { - update_from_content(&conflict, store, path, content) + update_from_content(&conflict, store, path, content, ConflictMarkerStyle::Diff) .block_on() .unwrap() }; @@ -1148,9 +1155,11 @@ fn materialize_conflict_string( store: &Store, path: &RepoPath, conflict: &Merge>, + conflict_marker_style: ConflictMarkerStyle, ) -> String { let contents = extract_as_single_hunk(conflict, store, path) .block_on() .unwrap(); - String::from_utf8(materialize_merge_result_to_bytes(&contents).into()).unwrap() + String::from_utf8(materialize_merge_result_to_bytes(&contents, conflict_marker_style).into()) + .unwrap() } diff --git a/lib/tests/test_local_working_copy.rs b/lib/tests/test_local_working_copy.rs index f4fa960c11..2c7f5f4ba2 100644 --- a/lib/tests/test_local_working_copy.rs +++ b/lib/tests/test_local_working_copy.rs @@ -44,6 +44,7 @@ use jj_lib::repo_path::RepoPathComponent; use jj_lib::secret_backend::SecretBackend; use jj_lib::settings::UserSettings; use jj_lib::working_copy::CheckoutError; +use jj_lib::working_copy::CheckoutOptions; use jj_lib::working_copy::CheckoutStats; use jj_lib::working_copy::SnapshotError; use jj_lib::working_copy::SnapshotOptions; @@ -279,10 +280,20 @@ fn test_checkout_file_transitions(backend: TestRepoBackend) { let right_commit = commit_with_tree(&store, right_tree_id.clone()); let ws = &mut test_workspace.workspace; - ws.check_out(repo.op_id().clone(), None, &left_commit) - .unwrap(); - ws.check_out(repo.op_id().clone(), None, &right_commit) - .unwrap(); + ws.check_out( + repo.op_id().clone(), + None, + &left_commit, + &CheckoutOptions::empty_for_test(), + ) + .unwrap(); + ws.check_out( + repo.op_id().clone(), + None, + &right_commit, + &CheckoutOptions::empty_for_test(), + ) + .unwrap(); // Check that the working copy is clean. let new_tree = test_workspace.snapshot().unwrap(); @@ -378,9 +389,20 @@ fn test_conflict_subdirectory() { let merged_commit = commit_with_tree(repo.store(), merged_tree.id()); let repo = &test_workspace.repo; let ws = &mut test_workspace.workspace; - ws.check_out(repo.op_id().clone(), None, &commit1).unwrap(); - ws.check_out(repo.op_id().clone(), None, &merged_commit) - .unwrap(); + ws.check_out( + repo.op_id().clone(), + None, + &commit1, + &CheckoutOptions::empty_for_test(), + ) + .unwrap(); + ws.check_out( + repo.op_id().clone(), + None, + &merged_commit, + &CheckoutOptions::empty_for_test(), + ) + .unwrap(); } #[test] @@ -429,7 +451,13 @@ fn test_acl() { let commit1 = repo.store().get_commit(commit1.id()).unwrap(); let commit2 = repo.store().get_commit(commit2.id()).unwrap(); - ws.check_out(repo.op_id().clone(), None, &commit1).unwrap(); + ws.check_out( + repo.op_id().clone(), + None, + &commit1, + &CheckoutOptions::empty_for_test(), + ) + .unwrap(); assert!(!secret_modified_path .to_fs_path_unchecked(&workspace_root) .is_file()); @@ -445,7 +473,13 @@ fn test_acl() { assert!(!became_public_path .to_fs_path_unchecked(&workspace_root) .is_file()); - ws.check_out(repo.op_id().clone(), None, &commit2).unwrap(); + ws.check_out( + repo.op_id().clone(), + None, + &commit2, + &CheckoutOptions::empty_for_test(), + ) + .unwrap(); assert!(!secret_modified_path .to_fs_path_unchecked(&workspace_root) .is_file()); @@ -474,7 +508,13 @@ fn test_tree_builder_file_directory_transition() { let mut check_out_tree = |tree_id: &TreeId| { let tree = repo.store().get_tree(RepoPath::root(), tree_id).unwrap(); let commit = commit_with_tree(repo.store(), MergedTreeId::Legacy(tree.id().clone())); - ws.check_out(repo.op_id().clone(), None, &commit).unwrap(); + ws.check_out( + repo.op_id().clone(), + None, + &commit, + &CheckoutOptions::empty_for_test(), + ) + .unwrap(); }; let parent_path = RepoPath::from_internal_string("foo/bar"); @@ -553,7 +593,14 @@ fn test_conflicting_changes_on_disk() { ) .unwrap(); - let stats = ws.check_out(repo.op_id().clone(), None, &commit).unwrap(); + let stats = ws + .check_out( + repo.op_id().clone(), + None, + &commit, + &CheckoutOptions::empty_for_test(), + ) + .unwrap(); assert_eq!( stats, CheckoutStats { @@ -604,7 +651,13 @@ fn test_reset() { let ws = &mut test_workspace.workspace; let commit = commit_with_tree(repo.store(), tree_with_file.id()); - ws.check_out(repo.op_id().clone(), None, &commit).unwrap(); + ws.check_out( + repo.op_id().clone(), + None, + &commit, + &CheckoutOptions::empty_for_test(), + ) + .unwrap(); // Test the setup: the file should exist on disk and in the tree state. assert!(ignored_path.to_fs_path_unchecked(&workspace_root).is_file()); @@ -656,7 +709,13 @@ fn test_checkout_discard() { let commit2 = commit_with_tree(repo.store(), tree2.id()); let ws = &mut test_workspace.workspace; - ws.check_out(repo.op_id().clone(), None, &commit1).unwrap(); + ws.check_out( + repo.op_id().clone(), + None, + &commit1, + &CheckoutOptions::empty_for_test(), + ) + .unwrap(); let wc: &LocalWorkingCopy = ws.working_copy().as_any().downcast_ref().unwrap(); let state_path = wc.state_path().to_path_buf(); @@ -667,7 +726,10 @@ fn test_checkout_discard() { // Start a checkout let mut locked_ws = ws.start_working_copy_mutation().unwrap(); - locked_ws.locked_wc().check_out(&commit2).unwrap(); + locked_ws + .locked_wc() + .check_out(&commit2, &CheckoutOptions::empty_for_test()) + .unwrap(); // The change should be reflected in the working copy but not saved assert!(!file1_path.to_fs_path_unchecked(&workspace_root).is_file()); assert!(file2_path.to_fs_path_unchecked(&workspace_root).is_file()); @@ -714,7 +776,14 @@ fn test_materialize_snapshot_conflicted_files() { .unwrap(); let commit = commit_with_tree(repo.store(), merged_tree.id()); - let stats = ws.check_out(repo.op_id().clone(), None, &commit).unwrap(); + let stats = ws + .check_out( + repo.op_id().clone(), + None, + &commit, + &CheckoutOptions::empty_for_test(), + ) + .unwrap(); assert_eq!( stats, CheckoutStats { @@ -965,7 +1034,13 @@ fn test_gitignores_in_ignored_dir() { let tree1 = create_tree(&test_workspace.repo, &[(gitignore_path, "ignored\n")]); let commit1 = commit_with_tree(test_workspace.repo.store(), tree1.id()); let ws = &mut test_workspace.workspace; - ws.check_out(op_id.clone(), None, &commit1).unwrap(); + ws.check_out( + op_id.clone(), + None, + &commit1, + &CheckoutOptions::empty_for_test(), + ) + .unwrap(); testutils::write_working_copy_file(&workspace_root, nested_gitignore_path, "!file\n"); testutils::write_working_copy_file(&workspace_root, ignored_path, "contents"); @@ -1017,7 +1092,14 @@ fn test_gitignores_checkout_never_overwrites_ignored() { // "contents". The exiting contents ("garbage") shouldn't be replaced in the // working copy. let ws = &mut test_workspace.workspace; - assert!(ws.check_out(repo.op_id().clone(), None, &commit).is_ok()); + assert!(ws + .check_out( + repo.op_id().clone(), + None, + &commit, + &CheckoutOptions::empty_for_test() + ) + .is_ok()); // Check that the old contents are in the working copy let path = workspace_root.join("modified"); @@ -1087,7 +1169,13 @@ fn test_gitignores_ignored_directory_already_tracked() { // Check out the tree with the files in `ignored/` let ws = &mut test_workspace.workspace; - ws.check_out(repo.op_id().clone(), None, &commit).unwrap(); + ws.check_out( + repo.op_id().clone(), + None, + &commit, + &CheckoutOptions::empty_for_test(), + ) + .unwrap(); // Make some changes inside the ignored directory and check that they are // detected when we snapshot. The files that are still there should not be @@ -1204,7 +1292,13 @@ fn test_git_submodule() { let commit2 = commit_with_tree(repo.store(), tree_id2.clone()); let ws = &mut test_workspace.workspace; - ws.check_out(repo.op_id().clone(), None, &commit1).unwrap(); + ws.check_out( + repo.op_id().clone(), + None, + &commit1, + &CheckoutOptions::empty_for_test(), + ) + .unwrap(); std::fs::create_dir(submodule_path.to_fs_path_unchecked(&workspace_root)).unwrap(); @@ -1229,7 +1323,13 @@ fn test_git_submodule() { // Check out new commit updating the submodule, which shouldn't fail because // of existing submodule files let ws = &mut test_workspace.workspace; - ws.check_out(repo.op_id().clone(), None, &commit2).unwrap(); + ws.check_out( + repo.op_id().clone(), + None, + &commit2, + &CheckoutOptions::empty_for_test(), + ) + .unwrap(); // Check that the files in the submodule are not deleted let file_in_submodule_path = added_submodule_path.to_fs_path_unchecked(&workspace_root); @@ -1246,7 +1346,12 @@ fn test_git_submodule() { // Check out the empty tree, which shouldn't fail let ws = &mut test_workspace.workspace; let stats = ws - .check_out(repo.op_id().clone(), None, &store.root_commit()) + .check_out( + repo.op_id().clone(), + None, + &store.root_commit(), + &CheckoutOptions::empty_for_test(), + ) .unwrap(); assert_eq!(stats.skipped_files, 1); } @@ -1265,7 +1370,13 @@ fn test_check_out_existing_file_cannot_be_removed() { let commit2 = commit_with_tree(repo.store(), tree2.id()); let ws = &mut test_workspace.workspace; - ws.check_out(repo.op_id().clone(), None, &commit1).unwrap(); + ws.check_out( + repo.op_id().clone(), + None, + &commit1, + &CheckoutOptions::empty_for_test(), + ) + .unwrap(); // Make the parent directory readonly. let writable_dir_perm = workspace_root.symlink_metadata().unwrap().permissions(); @@ -1273,7 +1384,12 @@ fn test_check_out_existing_file_cannot_be_removed() { readonly_dir_perm.set_readonly(true); std::fs::set_permissions(&workspace_root, readonly_dir_perm).unwrap(); - let result = ws.check_out(repo.op_id().clone(), None, &commit2); + let result = ws.check_out( + repo.op_id().clone(), + None, + &commit2, + &CheckoutOptions::empty_for_test(), + ); std::fs::set_permissions(&workspace_root, writable_dir_perm).unwrap(); // TODO: find a way to trigger the error on Windows @@ -1299,13 +1415,26 @@ fn test_check_out_existing_file_replaced_with_directory() { let commit2 = commit_with_tree(repo.store(), tree2.id()); let ws = &mut test_workspace.workspace; - ws.check_out(repo.op_id().clone(), None, &commit1).unwrap(); + ws.check_out( + repo.op_id().clone(), + None, + &commit1, + &CheckoutOptions::empty_for_test(), + ) + .unwrap(); std::fs::remove_file(file_path.to_fs_path_unchecked(&workspace_root)).unwrap(); std::fs::create_dir(file_path.to_fs_path_unchecked(&workspace_root)).unwrap(); // Checkout doesn't fail, but the file should be skipped. - let stats = ws.check_out(repo.op_id().clone(), None, &commit2).unwrap(); + let stats = ws + .check_out( + repo.op_id().clone(), + None, + &commit2, + &CheckoutOptions::empty_for_test(), + ) + .unwrap(); assert_eq!(stats.skipped_files, 1); assert!(file_path.to_fs_path_unchecked(&workspace_root).is_dir()); } @@ -1332,7 +1461,14 @@ fn test_check_out_existing_directory_symlink() { // Checkout doesn't fail, but the file should be skipped. let ws = &mut test_workspace.workspace; - let stats = ws.check_out(repo.op_id().clone(), None, &commit).unwrap(); + let stats = ws + .check_out( + repo.op_id().clone(), + None, + &commit, + &CheckoutOptions::empty_for_test(), + ) + .unwrap(); assert_eq!(stats.skipped_files, 1); // Therefore, "../escaped" shouldn't be created. @@ -1362,7 +1498,14 @@ fn test_check_out_existing_directory_symlink_icase_fs() { // Checkout doesn't fail, but the file should be skipped on icase fs. let ws = &mut test_workspace.workspace; - let stats = ws.check_out(repo.op_id().clone(), None, &commit).unwrap(); + let stats = ws + .check_out( + repo.op_id().clone(), + None, + &commit, + &CheckoutOptions::empty_for_test(), + ) + .unwrap(); if is_icase_fs { assert_eq!(stats.skipped_files, 1); } else { @@ -1406,7 +1549,14 @@ fn test_check_out_existing_file_symlink_icase_fs(victim_exists: bool) { // Checkout doesn't fail, but the file should be skipped on icase fs. let ws = &mut test_workspace.workspace; - let stats = ws.check_out(repo.op_id().clone(), None, &commit).unwrap(); + let stats = ws + .check_out( + repo.op_id().clone(), + None, + &commit, + &CheckoutOptions::empty_for_test(), + ) + .unwrap(); if is_icase_fs { assert_eq!(stats.skipped_files, 1); } else { @@ -1441,7 +1591,13 @@ fn test_check_out_file_removal_over_existing_directory_symlink() { // Check out "parent/escaped". let ws = &mut test_workspace.workspace; - ws.check_out(repo.op_id().clone(), None, &commit1).unwrap(); + ws.check_out( + repo.op_id().clone(), + None, + &commit1, + &CheckoutOptions::empty_for_test(), + ) + .unwrap(); // Pretend that "parent" was a symlink, which might be created by // e.g. checking out "PARENT" on case-insensitive fs. The file @@ -1454,7 +1610,14 @@ fn test_check_out_file_removal_over_existing_directory_symlink() { assert!(file_path.to_fs_path_unchecked(&workspace_root).exists()); // Check out empty tree, which tries to remove "parent/escaped". - let stats = ws.check_out(repo.op_id().clone(), None, &commit2).unwrap(); + let stats = ws + .check_out( + repo.op_id().clone(), + None, + &commit2, + &CheckoutOptions::empty_for_test(), + ) + .unwrap(); assert_eq!(stats.skipped_files, 1); // "../escaped" shouldn't be removed. @@ -1475,7 +1638,12 @@ fn test_check_out_malformed_file_path(file_path_str: &str) { // Checkout should fail let ws = &mut test_workspace.workspace; - let result = ws.check_out(repo.op_id().clone(), None, &commit); + let result = ws.check_out( + repo.op_id().clone(), + None, + &commit, + &CheckoutOptions::empty_for_test(), + ); assert_matches!(result, Err(CheckoutError::InvalidRepoPath(_))); // Therefore, "pwned" file shouldn't be created. @@ -1497,7 +1665,12 @@ fn test_check_out_malformed_file_path_windows(file_path_str: &str) { // Checkout should fail on Windows let ws = &mut test_workspace.workspace; - let result = ws.check_out(repo.op_id().clone(), None, &commit); + let result = ws.check_out( + repo.op_id().clone(), + None, + &commit, + &CheckoutOptions::empty_for_test(), + ); if cfg!(windows) { assert_matches!(result, Err(CheckoutError::InvalidRepoPath(_))); } else { @@ -1535,7 +1708,12 @@ fn test_check_out_reserved_file_path(file_path_str: &str) { // Checkout should fail. let ws = &mut test_workspace.workspace; - let result = ws.check_out(repo.op_id().clone(), None, &commit1); + let result = ws.check_out( + repo.op_id().clone(), + None, + &commit1, + &CheckoutOptions::empty_for_test(), + ); assert_matches!(result, Err(CheckoutError::ReservedPathComponent { .. })); // Therefore, "pwned" file shouldn't be created. @@ -1557,7 +1735,12 @@ fn test_check_out_reserved_file_path(file_path_str: &str) { } // Check out empty tree, which tries to remove the file. - let result = ws.check_out(repo.op_id().clone(), None, &commit2); + let result = ws.check_out( + repo.op_id().clone(), + None, + &commit2, + &CheckoutOptions::empty_for_test(), + ); assert_matches!(result, Err(CheckoutError::ReservedPathComponent { .. })); // The existing file shouldn't be removed. @@ -1587,7 +1770,12 @@ fn test_check_out_reserved_file_path_icase_fs(file_path_str: &str) { // Checkout should fail on icase fs. let ws = &mut test_workspace.workspace; - let result = ws.check_out(repo.op_id().clone(), None, &commit1); + let result = ws.check_out( + repo.op_id().clone(), + None, + &commit1, + &CheckoutOptions::empty_for_test(), + ); if is_icase_fs { assert_matches!(result, Err(CheckoutError::ReservedPathComponent { .. })); } else { @@ -1611,7 +1799,12 @@ fn test_check_out_reserved_file_path_icase_fs(file_path_str: &str) { std::fs::write(&disk_path, "").unwrap(); // Check out empty tree, which tries to remove the file. - let result = ws.check_out(repo.op_id().clone(), None, &commit2); + let result = ws.check_out( + repo.op_id().clone(), + None, + &commit2, + &CheckoutOptions::empty_for_test(), + ); if is_icase_fs { assert_matches!(result, Err(CheckoutError::ReservedPathComponent { .. })); } else { @@ -1649,7 +1842,12 @@ fn test_check_out_reserved_file_path_hfs_plus(file_path_str: &str) { // Checkout should fail on HFS+-like fs. let ws = &mut test_workspace.workspace; - let result = ws.check_out(repo.op_id().clone(), None, &commit1); + let result = ws.check_out( + repo.op_id().clone(), + None, + &commit1, + &CheckoutOptions::empty_for_test(), + ); if is_hfs_plus { assert_matches!(result, Err(CheckoutError::ReservedPathComponent { .. })); } else { @@ -1673,7 +1871,12 @@ fn test_check_out_reserved_file_path_hfs_plus(file_path_str: &str) { std::fs::write(&disk_path, "").unwrap(); // Check out empty tree, which tries to remove the file. - let result = ws.check_out(repo.op_id().clone(), None, &commit2); + let result = ws.check_out( + repo.op_id().clone(), + None, + &commit2, + &CheckoutOptions::empty_for_test(), + ); if is_hfs_plus { assert_matches!(result, Err(CheckoutError::ReservedPathComponent { .. })); } else { @@ -1721,7 +1924,12 @@ fn test_check_out_reserved_file_path_vfat(vfat_path_str: &str, file_path_strs: & // Checkout should fail on VFAT-like fs. let ws = &mut test_workspace.workspace; - let result = ws.check_out(repo.op_id().clone(), None, &commit1); + let result = ws.check_out( + repo.op_id().clone(), + None, + &commit1, + &CheckoutOptions::empty_for_test(), + ); if is_vfat { assert_matches!(result, Err(CheckoutError::ReservedPathComponent { .. })); } else { @@ -1747,7 +1955,12 @@ fn test_check_out_reserved_file_path_vfat(vfat_path_str: &str, file_path_strs: & } // Check out empty tree, which tries to remove the file. - let result = ws.check_out(repo.op_id().clone(), None, &commit2); + let result = ws.check_out( + repo.op_id().clone(), + None, + &commit2, + &CheckoutOptions::empty_for_test(), + ); if is_vfat { assert_matches!(result, Err(CheckoutError::ReservedPathComponent { .. })); } else { diff --git a/lib/tests/test_local_working_copy_concurrent.rs b/lib/tests/test_local_working_copy_concurrent.rs index b4f36b9c6b..4993571b7c 100644 --- a/lib/tests/test_local_working_copy_concurrent.rs +++ b/lib/tests/test_local_working_copy_concurrent.rs @@ -20,6 +20,7 @@ use jj_lib::repo::Repo; use jj_lib::repo_path::RepoPath; use jj_lib::repo_path::RepoPathBuf; use jj_lib::working_copy::CheckoutError; +use jj_lib::working_copy::CheckoutOptions; use jj_lib::working_copy::SnapshotOptions; use jj_lib::workspace::default_working_copy_factories; use jj_lib::workspace::Workspace; @@ -50,7 +51,13 @@ fn test_concurrent_checkout() { // Check out tree1 let ws1 = &mut test_workspace1.workspace; // The operation ID is not correct, but that doesn't matter for this test - ws1.check_out(repo.op_id().clone(), None, &commit1).unwrap(); + ws1.check_out( + repo.op_id().clone(), + None, + &commit1, + &CheckoutOptions::empty_for_test(), + ) + .unwrap(); // Check out tree2 from another process (simulated by another workspace // instance) @@ -61,12 +68,22 @@ fn test_concurrent_checkout() { &default_working_copy_factories(), ) .unwrap(); - ws2.check_out(repo.op_id().clone(), Some(&tree_id1), &commit2) - .unwrap(); + ws2.check_out( + repo.op_id().clone(), + Some(&tree_id1), + &commit2, + &CheckoutOptions::empty_for_test(), + ) + .unwrap(); // Checking out another tree (via the first workspace instance) should now fail. assert_matches!( - ws1.check_out(repo.op_id().clone(), Some(&tree_id1), &commit3), + ws1.check_out( + repo.op_id().clone(), + Some(&tree_id1), + &commit3, + &CheckoutOptions::empty_for_test() + ), Err(CheckoutError::ConcurrentCheckout) ); @@ -107,7 +124,12 @@ fn test_checkout_parallel() { let commit = commit_with_tree(repo.store(), tree.id()); test_workspace .workspace - .check_out(repo.op_id().clone(), None, &commit) + .check_out( + repo.op_id().clone(), + None, + &commit, + &CheckoutOptions::empty_for_test(), + ) .unwrap(); thread::scope(|s| { @@ -127,14 +149,17 @@ fn test_checkout_parallel() { ) .unwrap(); // The operation ID is not correct, but that doesn't matter for this test - let stats = workspace.check_out(op_id, None, &commit).unwrap(); + let stats = workspace + .check_out(op_id, None, &commit, &CheckoutOptions::empty_for_test()) + .unwrap(); assert_eq!(stats.updated_files, 0); assert_eq!(stats.added_files, 1); assert_eq!(stats.removed_files, 1); // Check that the working copy contains one of the trees. We may see a // different tree than the one we just checked out, but since - // write_tree() should take the same lock as check_out(), write_tree() - // should never produce a different tree. + // write_tree() should take the same lock as check_out(), write_tree(, + // &CheckoutOptions::empty_for_test()) should never produce a + // different tree. let mut locked_ws = workspace.start_working_copy_mutation().unwrap(); let new_tree_id = locked_ws .locked_wc() @@ -161,7 +186,13 @@ fn test_racy_checkout() { let mut num_matches = 0; for _ in 0..100 { let ws = &mut test_workspace.workspace; - ws.check_out(op_id.clone(), None, &commit).unwrap(); + ws.check_out( + op_id.clone(), + None, + &commit, + &CheckoutOptions::empty_for_test(), + ) + .unwrap(); assert_eq!( std::fs::read(path.to_fs_path_unchecked(&workspace_root)).unwrap(), b"1".to_vec() diff --git a/lib/tests/test_local_working_copy_sparse.rs b/lib/tests/test_local_working_copy_sparse.rs index fb21726b1e..61c93aaa75 100644 --- a/lib/tests/test_local_working_copy_sparse.rs +++ b/lib/tests/test_local_working_copy_sparse.rs @@ -19,6 +19,7 @@ use jj_lib::matchers::EverythingMatcher; use jj_lib::repo::Repo; use jj_lib::repo_path::RepoPath; use jj_lib::repo_path::RepoPathBuf; +use jj_lib::working_copy::CheckoutOptions; use jj_lib::working_copy::CheckoutStats; use jj_lib::working_copy::WorkingCopy; use pollster::FutureExt as _; @@ -62,7 +63,12 @@ fn test_sparse_checkout() { test_workspace .workspace - .check_out(repo.op_id().clone(), None, &commit) + .check_out( + repo.op_id().clone(), + None, + &commit, + &CheckoutOptions::empty_for_test(), + ) .unwrap(); let ws = &mut test_workspace.workspace; @@ -71,7 +77,7 @@ fn test_sparse_checkout() { let sparse_patterns = to_owned_path_vec(&[dir1_path]); let stats = locked_ws .locked_wc() - .set_sparse_patterns(sparse_patterns.clone()) + .set_sparse_patterns(sparse_patterns.clone(), &CheckoutOptions::empty_for_test()) .unwrap(); assert_eq!( stats, @@ -130,7 +136,7 @@ fn test_sparse_checkout() { let mut locked_wc = wc.start_mutation().unwrap(); let sparse_patterns = to_owned_path_vec(&[root_file1_path, dir1_subdir1_path, dir2_path]); let stats = locked_wc - .set_sparse_patterns(sparse_patterns.clone()) + .set_sparse_patterns(sparse_patterns.clone(), &CheckoutOptions::empty_for_test()) .unwrap(); assert_eq!( stats, @@ -195,7 +201,12 @@ fn test_sparse_commit() { let commit = commit_with_tree(repo.store(), tree.id()); test_workspace .workspace - .check_out(repo.op_id().clone(), None, &commit) + .check_out( + repo.op_id().clone(), + None, + &commit, + &CheckoutOptions::empty_for_test(), + ) .unwrap(); // Set sparse patterns to only dir1/ @@ -206,7 +217,7 @@ fn test_sparse_commit() { let sparse_patterns = to_owned_path_vec(&[dir1_path]); locked_ws .locked_wc() - .set_sparse_patterns(sparse_patterns) + .set_sparse_patterns(sparse_patterns, &CheckoutOptions::empty_for_test()) .unwrap(); locked_ws.finish(repo.op_id().clone()).unwrap(); @@ -247,7 +258,7 @@ fn test_sparse_commit() { let sparse_patterns = to_owned_path_vec(&[dir1_path, dir2_path]); locked_ws .locked_wc() - .set_sparse_patterns(sparse_patterns) + .set_sparse_patterns(sparse_patterns, &CheckoutOptions::empty_for_test()) .unwrap(); locked_ws.finish(op_id).unwrap(); @@ -283,7 +294,7 @@ fn test_sparse_commit_gitignore() { let sparse_patterns = to_owned_path_vec(&[dir1_path]); locked_ws .locked_wc() - .set_sparse_patterns(sparse_patterns) + .set_sparse_patterns(sparse_patterns, &CheckoutOptions::empty_for_test()) .unwrap(); locked_ws.finish(repo.op_id().clone()).unwrap();