From cb41e9cfaa6691f34d1d7cfec0d3b8f0de330cb3 Mon Sep 17 00:00:00 2001 From: Scott Taylor <scott.g.taylor@jpmchase.com> 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 | 21 ++++- cli/src/cli_util.rs | 13 +++ cli/src/command_error.rs | 1 + cli/src/commands/absorb.rs | 4 + cli/src/commands/diff.rs | 1 + cli/src/commands/evolog.rs | 2 + cli/src/commands/file/annotate.rs | 8 +- cli/src/commands/file/show.rs | 6 +- cli/src/commands/file/track.rs | 2 + cli/src/commands/file/untrack.rs | 2 + cli/src/commands/interdiff.rs | 1 + cli/src/commands/log.rs | 10 ++- cli/src/commands/operation/diff.rs | 24 +++++- cli/src/commands/operation/log.rs | 1 + cli/src/commands/operation/show.rs | 1 + cli/src/commands/show.rs | 9 ++- cli/src/commands/status.rs | 1 + cli/src/commit_templater.rs | 20 ++++- cli/src/config/misc.toml | 1 + cli/src/diff_util.rs | 87 +++++++++++++++------ cli/src/merge_tools/builtin.rs | 55 +++++++++++-- cli/src/merge_tools/diff_working_copies.rs | 20 ++++- cli/src/merge_tools/external.rs | 19 ++++- cli/src/merge_tools/mod.rs | 33 ++++++-- lib/src/annotate.rs | 64 +++++++++++---- lib/src/conflicts.rs | 12 ++- lib/src/default_index/revset_engine.rs | 15 +++- lib/src/local_working_copy.rs | 70 ++++++++++++++--- lib/src/settings.rs | 9 +++ lib/src/working_copy.rs | 6 ++ lib/src/workspace.rs | 16 +++- lib/tests/test_annotate.rs | 16 +++- lib/tests/test_conflicts.rs | 9 ++- lib/tests/test_local_working_copy.rs | 16 +++- lib/tests/test_local_working_copy_sparse.rs | 2 + 36 files changed, 486 insertions(+), 95 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e6809820c78..7b74375d526 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,10 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). * New `fork_point()` revset function can be used to obtain the fork point of multiple commits. +* 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 ## [0.23.0] - 2024-11-06 diff --git a/cli/examples/custom-working-copy/main.rs b/cli/examples/custom-working-copy/main.rs index 276ad1fd831..21dbf1cbd41 100644 --- a/cli/examples/custom-working-copy/main.rs +++ b/cli/examples/custom-working-copy/main.rs @@ -25,6 +25,7 @@ use jj_cli::ui::Ui; use jj_lib::backend::Backend; use jj_lib::backend::MergedTreeId; use jj_lib::commit::Commit; +use jj_lib::conflicts::ConflictMarkerStyle; use jj_lib::git_backend::GitBackend; use jj_lib::local_working_copy::LocalWorkingCopy; use jj_lib::op_store::OperationId; @@ -119,6 +120,7 @@ impl ConflictsWorkingCopy { state_path: PathBuf, operation_id: OperationId, workspace_id: WorkspaceId, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<Self, WorkingCopyStateError> { let inner = LocalWorkingCopy::init( store, @@ -126,6 +128,7 @@ impl ConflictsWorkingCopy { state_path, operation_id, workspace_id, + conflict_marker_style, )?; Ok(ConflictsWorkingCopy { inner: Box::new(inner), @@ -133,8 +136,18 @@ impl ConflictsWorkingCopy { }) } - fn load(store: Arc<Store>, working_copy_path: PathBuf, state_path: PathBuf) -> Self { - let inner = LocalWorkingCopy::load(store, working_copy_path.clone(), state_path); + fn load( + store: Arc<Store>, + working_copy_path: PathBuf, + state_path: PathBuf, + conflict_marker_style: ConflictMarkerStyle, + ) -> Self { + let inner = LocalWorkingCopy::load( + store, + working_copy_path.clone(), + state_path, + conflict_marker_style, + ); ConflictsWorkingCopy { inner: Box::new(inner), working_copy_path, @@ -186,6 +199,7 @@ impl WorkingCopyFactory for ConflictsWorkingCopyFactory { state_path: PathBuf, operation_id: OperationId, workspace_id: WorkspaceId, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<Box<dyn WorkingCopy>, WorkingCopyStateError> { Ok(Box::new(ConflictsWorkingCopy::init( store, @@ -193,6 +207,7 @@ impl WorkingCopyFactory for ConflictsWorkingCopyFactory { state_path, operation_id, workspace_id, + conflict_marker_style, )?)) } @@ -201,11 +216,13 @@ impl WorkingCopyFactory for ConflictsWorkingCopyFactory { store: Arc<Store>, working_copy_path: PathBuf, state_path: PathBuf, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<Box<dyn WorkingCopy>, WorkingCopyStateError> { Ok(Box::new(ConflictsWorkingCopy::load( store, working_copy_path, state_path, + conflict_marker_style, ))) } } diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index aa3d3e8a012..ac0bca19d5b 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -55,6 +55,7 @@ use jj_lib::backend::CommitId; use jj_lib::backend::MergedTreeId; use jj_lib::backend::TreeValue; use jj_lib::commit::Commit; +use jj_lib::conflicts::ConflictMarkerStyle; use jj_lib::file_util; use jj_lib::fileset; use jj_lib::fileset::FilesetDiagnostics; @@ -130,6 +131,7 @@ use tracing_chrome::ChromeLayerBuilder; use tracing_subscriber::prelude::*; use crate::command_error::cli_error; +use crate::command_error::config_error; use crate::command_error::config_error_with_message; use crate::command_error::handle_command_result; use crate::command_error::internal_error; @@ -711,6 +713,7 @@ pub struct WorkspaceCommandEnvironment { workspace_id: WorkspaceId, immutable_heads_expression: Rc<UserRevsetExpression>, short_prefixes_expression: Option<Rc<UserRevsetExpression>>, + conflict_marker_style: ConflictMarkerStyle, } impl WorkspaceCommandEnvironment { @@ -731,6 +734,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)?; @@ -792,6 +796,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, @@ -898,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, ) } @@ -1726,6 +1736,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 @@ -1793,6 +1804,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); @@ -2376,6 +2388,7 @@ jj git init --colocate", WorkspaceLoadError::StoreLoadError(err) => internal_error(err), WorkspaceLoadError::WorkingCopyState(err) => internal_error(err), WorkspaceLoadError::NonUnicodePath | WorkspaceLoadError::Path(_) => user_error(err), + WorkspaceLoadError::Config(err) => config_error(err), } } diff --git a/cli/src/command_error.rs b/cli/src/command_error.rs index 77196dd7cd4..9c3f21bdbf8 100644 --- a/cli/src/command_error.rs +++ b/cli/src/command_error.rs @@ -315,6 +315,7 @@ impl From<WorkspaceInitError> for CommandError { } WorkspaceInitError::SignInit(err @ SignInitError::UnknownBackend(_)) => user_error(err), WorkspaceInitError::SignInit(err) => internal_error(err), + WorkspaceInitError::Config(err) => config_error(err), } } } diff --git a/cli/src/commands/absorb.rs b/cli/src/commands/absorb.rs index 97446d24b9f..bd7524c60e6 100644 --- a/cli/src/commands/absorb.rs +++ b/cli/src/commands/absorb.rs @@ -29,6 +29,7 @@ use jj_lib::backend::FileId; use jj_lib::backend::TreeValue; use jj_lib::commit::Commit; use jj_lib::conflicts::materialized_diff_stream; +use jj_lib::conflicts::ConflictMarkerStyle; use jj_lib::conflicts::MaterializedTreeValue; use jj_lib::copies::CopyRecords; use jj_lib::diff::Diff; @@ -100,6 +101,7 @@ pub(crate) fn cmd_absorb( &destinations, &matcher, workspace_command.path_converter(), + workspace_command.env().conflict_marker_style(), ) .block_on()?; workspace_command.check_rewritable(selected_trees.keys())?; @@ -155,6 +157,7 @@ async fn split_hunks_to_trees( destinations: &Rc<ResolvedRevsetExpression>, matcher: &dyn Matcher, path_converter: &RepoPathUiConverter, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<HashMap<CommitId, MergedTreeBuilder>, CommandError> { let mut selected_trees: HashMap<CommitId, MergedTreeBuilder> = HashMap::new(); @@ -194,6 +197,7 @@ async fn split_hunks_to_trees( destinations, left_path, left_text.clone(), + conflict_marker_style, )?; let annotation_ranges = annotation .compact_line_ranges() diff --git a/cli/src/commands/diff.rs b/cli/src/commands/diff.rs index 244cc4f4c68..955b14e3bf2 100644 --- a/cli/src/commands/diff.rs +++ b/cli/src/commands/diff.rs @@ -110,6 +110,7 @@ pub(crate) fn cmd_diff( &matcher, ©_records, ui.term_width(), + workspace_command.env().conflict_marker_style(), )?; print_unmatched_explicit_paths( ui, diff --git a/cli/src/commands/evolog.rs b/cli/src/commands/evolog.rs index b9bb6c3c86c..d4a60c857e8 100644 --- a/cli/src/commands/evolog.rs +++ b/cli/src/commands/evolog.rs @@ -174,6 +174,7 @@ pub(crate) fn cmd_evolog( &commit, &EverythingMatcher, within_graph.width(), + workspace_command.env().conflict_marker_style(), )?; } let node_symbol = format_template(ui, &Some(commit.clone()), &node_template); @@ -198,6 +199,7 @@ pub(crate) fn cmd_evolog( &commit, &EverythingMatcher, width, + workspace_command.env().conflict_marker_style(), )?; } } diff --git a/cli/src/commands/file/annotate.rs b/cli/src/commands/file/annotate.rs index 3ea91271f92..07124e112d7 100644 --- a/cli/src/commands/file/annotate.rs +++ b/cli/src/commands/file/annotate.rs @@ -77,7 +77,13 @@ pub(crate) fn cmd_file_annotate( // exclude the revisions, but will ignore diffs in those revisions as if // ancestor revisions had new content. let domain = RevsetExpression::all(); - let annotation = get_annotation_for_file(repo.as_ref(), &starting_commit, &domain, &file_path)?; + let annotation = get_annotation_for_file( + repo.as_ref(), + &starting_commit, + &domain, + &file_path, + workspace_command.env().conflict_marker_style(), + )?; render_file_annotation(repo.as_ref(), ui, &template, &annotation)?; Ok(()) diff --git a/cli/src/commands/file/show.rs b/cli/src/commands/file/show.rs index 98619bb5e37..396acac2e27 100644 --- a/cli/src/commands/file/show.rs +++ b/cli/src/commands/file/show.rs @@ -127,7 +127,11 @@ fn write_tree_entries<P: AsRef<RepoPath>>( 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 30929372f44..e5d272902a8 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 bbcf0dc5d18..b1afe6dba63 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/interdiff.rs b/cli/src/commands/interdiff.rs index 75bc9439aba..ef28300f682 100644 --- a/cli/src/commands/interdiff.rs +++ b/cli/src/commands/interdiff.rs @@ -71,6 +71,7 @@ pub(crate) fn cmd_interdiff( &to, matcher.as_ref(), ui.term_width(), + workspace_command.env().conflict_marker_style(), )?; Ok(()) } diff --git a/cli/src/commands/log.rs b/cli/src/commands/log.rs index 22a5126f083..cc4ea87ee15 100644 --- a/cli/src/commands/log.rs +++ b/cli/src/commands/log.rs @@ -243,6 +243,7 @@ pub(crate) fn cmd_log( &commit, matcher.as_ref(), within_graph.width(), + workspace_command.env().conflict_marker_style(), )?; } @@ -285,7 +286,14 @@ pub(crate) fn cmd_log( .write(formatter, |formatter| template.format(&commit, formatter))?; if let Some(renderer) = &diff_renderer { let width = ui.term_width(); - renderer.show_patch(ui, formatter, &commit, matcher.as_ref(), width)?; + renderer.show_patch( + ui, + formatter, + &commit, + matcher.as_ref(), + width, + workspace_command.env().conflict_marker_style(), + )?; } } } diff --git a/cli/src/commands/operation/diff.rs b/cli/src/commands/operation/diff.rs index 941cf4db53c..d753e2e320d 100644 --- a/cli/src/commands/operation/diff.rs +++ b/cli/src/commands/operation/diff.rs @@ -22,6 +22,7 @@ use itertools::Itertools; use jj_lib::backend::ChangeId; use jj_lib::backend::CommitId; use jj_lib::commit::Commit; +use jj_lib::conflicts::ConflictMarkerStyle; use jj_lib::dag_walk; use jj_lib::git::REMOTE_NAME_FOR_LOCAL_GIT_REPO; use jj_lib::graph::GraphEdge; @@ -157,6 +158,7 @@ pub fn cmd_op_diff( (!args.no_graph).then_some(graph_style), &with_content_format, diff_renderer.as_ref(), + workspace_env.conflict_marker_style(), ) } @@ -175,6 +177,7 @@ pub fn show_op_diff( graph_style: Option<GraphStyle>, with_content_format: &LogContentFormat, diff_renderer: Option<&DiffRenderer>, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<(), CommandError> { let changes = compute_operation_commits_diff(current_repo, from_repo, to_repo)?; @@ -255,6 +258,7 @@ pub fn show_op_diff( diff_renderer, modified_change, within_graph.width(), + conflict_marker_style, )?; } @@ -280,7 +284,14 @@ pub fn show_op_diff( })?; if let Some(diff_renderer) = &diff_renderer { let width = with_content_format.width(); - show_change_diff(ui, formatter, diff_renderer, modified_change, width)?; + show_change_diff( + ui, + formatter, + diff_renderer, + modified_change, + width, + conflict_marker_style, + )?; } } } @@ -562,6 +573,7 @@ fn show_change_diff( diff_renderer: &DiffRenderer, change: &ModifiedChange, width: usize, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<(), CommandError> { match (&*change.removed_commits, &*change.added_commits) { (predecessors @ ([] | [_]), [commit]) => { @@ -574,11 +586,19 @@ fn show_change_diff( commit, &EverythingMatcher, width, + conflict_marker_style, )?; } ([commit], []) => { // TODO: Should we show a reverse diff? - diff_renderer.show_patch(ui, formatter, commit, &EverythingMatcher, width)?; + diff_renderer.show_patch( + ui, + formatter, + commit, + &EverythingMatcher, + width, + conflict_marker_style, + )?; } ([_, _, ..], _) | (_, [_, _, ..]) => {} ([], []) => panic!("ModifiedChange should have at least one entry"), diff --git a/cli/src/commands/operation/log.rs b/cli/src/commands/operation/log.rs index 5f5242c7fc7..1a134fa3fd1 100644 --- a/cli/src/commands/operation/log.rs +++ b/cli/src/commands/operation/log.rs @@ -179,6 +179,7 @@ fn do_op_log( (!args.no_graph).then_some(graph_style), with_content_format, diff_renderer.as_ref(), + workspace_env.conflict_marker_style(), ) }; Some(show) diff --git a/cli/src/commands/operation/show.rs b/cli/src/commands/operation/show.rs index bd5ef606f02..75944842cd3 100644 --- a/cli/src/commands/operation/show.rs +++ b/cli/src/commands/operation/show.rs @@ -101,5 +101,6 @@ pub fn cmd_op_show( (!args.no_graph).then_some(graph_style), &with_content_format, diff_renderer.as_ref(), + workspace_env.conflict_marker_style(), ) } diff --git a/cli/src/commands/show.rs b/cli/src/commands/show.rs index ad7e21bad97..e70bba8eb9e 100644 --- a/cli/src/commands/show.rs +++ b/cli/src/commands/show.rs @@ -59,6 +59,13 @@ pub(crate) fn cmd_show( let mut formatter = ui.stdout_formatter(); let formatter = formatter.as_mut(); template.format(&commit, formatter)?; - diff_renderer.show_patch(ui, formatter, &commit, &EverythingMatcher, ui.term_width())?; + diff_renderer.show_patch( + ui, + formatter, + &commit, + &EverythingMatcher, + ui.term_width(), + workspace_command.env().conflict_marker_style(), + )?; Ok(()) } diff --git a/cli/src/commands/status.rs b/cli/src/commands/status.rs index 794cf4f928b..466790428d6 100644 --- a/cli/src/commands/status.rs +++ b/cli/src/commands/status.rs @@ -82,6 +82,7 @@ pub(crate) fn cmd_status( &matcher, ©_records, width, + workspace_command.env().conflict_marker_style(), )?; } diff --git a/cli/src/commit_templater.rs b/cli/src/commit_templater.rs index 1685c2c38b3..5fcb801ef97 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<UserRevsetExpression>, + 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<UserRevsetExpression>, + conflict_marker_style: ConflictMarkerStyle, extensions: &[impl AsRef<dyn CommitTemplateLanguageExtension>], ) -> 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/misc.toml b/cli/src/config/misc.toml index d4f08353c5f..33d07febc4b 100644 --- a/cli/src/config/misc.toml +++ b/cli/src/config/misc.toml @@ -23,6 +23,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 7a7761882e9..c79f816ce7e 100644 --- a/cli/src/diff_util.rs +++ b/cli/src/diff_util.rs @@ -34,6 +34,7 @@ use jj_lib::backend::TreeValue; use jj_lib::commit::Commit; use jj_lib::conflicts::materialize_merge_result; 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; @@ -296,6 +297,7 @@ impl<'a> DiffRenderer<'a> { matcher: &dyn Matcher, copy_records: &CopyRecords, width: usize, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<(), DiffRenderError> { formatter.with_label("diff", |formatter| { self.show_diff_inner( @@ -306,6 +308,7 @@ impl<'a> DiffRenderer<'a> { matcher, copy_records, width, + conflict_marker_style, ) }) } @@ -320,6 +323,7 @@ impl<'a> DiffRenderer<'a> { matcher: &dyn Matcher, copy_records: &CopyRecords, width: usize, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<(), DiffRenderError> { let store = self.repo.store(); let path_converter = self.path_converter; @@ -333,7 +337,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, + conflict_marker_style, + )?; } DiffFormat::Types => { let tree_diff = @@ -348,12 +360,19 @@ 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, 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, + conflict_marker_style, + )?; } DiffFormat::Tool(tool) => { match tool.diff_invocation_mode { @@ -367,12 +386,21 @@ impl<'a> DiffRenderer<'a> { tree_diff, path_converter, tool, + 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, + conflict_marker_style, + ) + .map_err(DiffRenderError::DiffGenerate) } }?; } @@ -384,6 +412,7 @@ impl<'a> DiffRenderer<'a> { /// Generates diff between `from_commits` and `to_commit` based off their /// parents. The `from_commits` will temporarily be rebased onto the /// `to_commit` parents to exclude unrelated changes. + #[allow(clippy::too_many_arguments)] pub fn show_inter_diff( &self, ui: &Ui, @@ -392,6 +421,7 @@ impl<'a> DiffRenderer<'a> { to_commit: &Commit, matcher: &dyn Matcher, width: usize, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<(), DiffRenderError> { let from_tree = rebase_to_dest_parent(self.repo, from_commits, to_commit)?; let to_tree = to_commit.tree()?; @@ -404,6 +434,7 @@ impl<'a> DiffRenderer<'a> { matcher, ©_records, width, + conflict_marker_style, ) } @@ -415,6 +446,7 @@ impl<'a> DiffRenderer<'a> { commit: &Commit, matcher: &dyn Matcher, width: usize, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<(), DiffRenderError> { let from_tree = commit.parent_tree(self.repo)?; let to_tree = commit.tree()?; @@ -431,6 +463,7 @@ impl<'a> DiffRenderer<'a> { matcher, ©_records, width, + conflict_marker_style, ) } } @@ -839,7 +872,11 @@ fn file_content_for_diff(reader: &mut dyn io::Read) -> io::Result<FileContent> { }) } -fn diff_content(path: &RepoPath, value: MaterializedTreeValue) -> io::Result<FileContent> { +fn diff_content( + path: &RepoPath, + value: MaterializedTreeValue, + conflict_marker_style: ConflictMarkerStyle, +) -> io::Result<FileContent> { match value { MaterializedTreeValue::Absent => Ok(FileContent::empty()), MaterializedTreeValue::AccessDenied(err) => Ok(FileContent { @@ -865,7 +902,7 @@ fn diff_content(path: &RepoPath, value: MaterializedTreeValue) -> io::Result<Fil executable: _, } => { let mut data = vec![]; - materialize_merge_result(&contents, &mut data) + materialize_merge_result(&contents, conflict_marker_style, &mut data) .expect("Failed to materialize conflict to in-memory buffer"); Ok(FileContent { is_binary: false, @@ -909,6 +946,7 @@ pub fn show_color_words_diff( tree_diff: BoxStream<CopiesTreeDiffEntry>, path_converter: &RepoPathUiConverter, options: &ColorWordsDiffOptions, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<(), DiffRenderError> { let mut diff_stream = materialized_diff_stream(store, tree_diff); async { @@ -944,7 +982,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 { @@ -1006,8 +1044,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"), @@ -1035,7 +1073,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 { @@ -1057,18 +1095,18 @@ pub fn show_file_by_file_diff( tree_diff: BoxStream<CopiesTreeDiffEntry>, path_converter: &RepoPathUiConverter, tool: &ExternalMergeTool, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<(), DiffRenderError> { - fn create_file( - path: &RepoPath, - wc_dir: &Path, - value: MaterializedTreeValue, - ) -> Result<PathBuf, DiffRenderError> { + let create_file = |path: &RepoPath, + wc_dir: &Path, + value: MaterializedTreeValue| + -> Result<PathBuf, DiffRenderError> { 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"); @@ -1131,6 +1169,7 @@ struct GitDiffPart { fn git_diff_part( path: &RepoPath, value: MaterializedTreeValue, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<GitDiffPart, DiffRenderError> { const DUMMY_HASH: &str = "0000000000"; let mode; @@ -1182,7 +1221,7 @@ fn git_diff_part( mode = if executable { "100755" } else { "100644" }; hash = DUMMY_HASH.to_owned(); let mut data = vec![]; - materialize_merge_result(&contents, &mut data) + materialize_merge_result(&contents, conflict_marker_style, &mut data) .expect("Failed to materialize conflict to in-memory buffer"); content = FileContent { is_binary: false, // TODO: are we sure this is never binary? @@ -1450,6 +1489,7 @@ pub fn show_git_diff( store: &Store, tree_diff: BoxStream<CopiesTreeDiffEntry>, options: &UnifiedDiffOptions, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<(), DiffRenderError> { let mut diff_stream = materialized_diff_stream(store, tree_diff); async { @@ -1460,8 +1500,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!( @@ -1635,6 +1675,7 @@ pub fn show_diff_stat( path_converter: &RepoPathUiConverter, options: &DiffStatOptions, display_width: usize, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<(), DiffRenderError> { let mut stats: Vec<DiffStat> = vec![]; let mut unresolved_renames = HashSet::new(); @@ -1647,8 +1688,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 160cabad0fe..984f8b30521 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; 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<FileInfo, BuiltinToolError> { let value = tree.path_value(path)?; let materialized_value = materialize_tree_value(store, path, value) @@ -212,7 +214,7 @@ fn read_file_contents( executable: _, } => { let mut buf = Vec::new(); - materialize_merge_result(&contents, &mut buf) + materialize_merge_result(&contents, conflict_marker_style, &mut buf) .expect("Failed to materialize conflict to in-memory buffer"); // TODO: Render the ID somehow? let contents = buf_to_file_contents(None, buf); @@ -311,11 +313,13 @@ pub fn make_diff_files( left_tree: &MergedTree, right_tree: &MergedTree, changed_files: &[RepoPathBuf], + conflict_marker_style: ConflictMarkerStyle, ) -> Result<Vec<scm_record::File<'static>>, 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) { @@ -524,6 +528,7 @@ pub fn edit_diff_builtin( left_tree: &MergedTree, right_tree: &MergedTree, matcher: &dyn Matcher, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<MergedTreeId, BuiltinToolError> { let store = left_tree.store().clone(); // TODO: handle copy tracking @@ -532,7 +537,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 { @@ -698,7 +709,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 { @@ -828,7 +846,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 { @@ -892,7 +917,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 { @@ -957,7 +989,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 749255326d7..5dd2cceda6d 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; @@ -84,10 +85,11 @@ fn check_out( state_dir: PathBuf, tree: &MergedTree, sparse_patterns: Vec<RepoPathBuf>, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<TreeState, DiffCheckoutError> { 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)?; + let mut tree_state = TreeState::init(store, wc_dir, state_dir, conflict_marker_style)?; tree_state.set_sparse_patterns(sparse_patterns)?; tree_state.check_out(tree)?; Ok(tree_state) @@ -135,6 +137,7 @@ pub(crate) fn check_out_trees( right_tree: &MergedTree, matcher: &dyn Matcher, output_is: Option<DiffSide>, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<DiffWorkingCopies, DiffCheckoutError> { let changed_files: Vec<_> = left_tree .diff_stream(right_tree, matcher) @@ -153,6 +156,7 @@ pub(crate) fn check_out_trees( left_state_dir, left_tree, changed_files.clone(), + conflict_marker_style, )?; let right_tree_state = check_out( store.clone(), @@ -160,6 +164,7 @@ pub(crate) fn check_out_trees( right_state_dir, right_tree, changed_files.clone(), + conflict_marker_style, )?; let output_tree_state = output_is .map(|output_side| { @@ -174,6 +179,7 @@ pub(crate) fn check_out_trees( DiffSide::Right => right_tree, }, changed_files, + conflict_marker_style, ) }) .transpose()?; @@ -200,8 +206,16 @@ impl DiffEditWorkingCopies { matcher: &dyn Matcher, output_is: Option<DiffSide>, instructions: Option<&str>, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<Self, DiffEditError> { - 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, + conflict_marker_style, + )?; let got_output_field = output_is.is_some(); set_readonly_recursively(diff_wc.left_working_copy_path()) @@ -273,6 +287,7 @@ diff editing in mind and be a little inaccurate. pub fn snapshot_results( self, base_ignores: Arc<GitIgnoreFile>, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<MergedTreeId, DiffEditError> { if let Some(path) = self.instructions_path_to_cleanup { std::fs::remove_file(path).ok(); @@ -289,6 +304,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 26e13b16448..a45e81162bf 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; +use jj_lib::conflicts::ConflictMarkerStyle; use jj_lib::gitignore::GitIgnoreFile; use jj_lib::matchers::Matcher; use jj_lib::merge::Merge; @@ -156,10 +157,11 @@ pub fn run_mergetool_external( repo_path: &RepoPath, conflict: MergedTreeValue, tree: &MergedTree, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<MergedTreeId, ConflictResolveError> { let initial_output_content: Vec<u8> = if editor.merge_tool_edits_conflict_markers { let mut materialized_conflict = vec![]; - materialize_merge_result(&content, &mut materialized_conflict) + materialize_merge_result(&content, conflict_marker_style, &mut materialized_conflict) .expect("Writing to an in-memory buffer should never fail"); materialized_conflict } else { @@ -229,6 +231,7 @@ pub fn run_mergetool_external( tree.store(), repo_path, output_file_contents.as_slice(), + conflict_marker_style, ) .block_on()? } else { @@ -258,6 +261,7 @@ pub fn edit_diff_external( matcher: &dyn Matcher, instructions: Option<&str>, base_ignores: Arc<GitIgnoreFile>, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<MergedTreeId, DiffEditError> { let got_output_field = find_all_variables(&editor.edit_args).contains(&"output"); let store = left_tree.store(); @@ -268,6 +272,7 @@ pub fn edit_diff_external( matcher, got_output_field.then_some(DiffSide::Right), instructions, + conflict_marker_style, )?; let patterns = diffedit_wc.working_copies.to_command_variables(); @@ -286,7 +291,7 @@ pub fn edit_diff_external( })); } - diffedit_wc.snapshot_results(base_ignores) + diffedit_wc.snapshot_results(base_ignores, conflict_marker_style) } /// Generates textual diff by the specified `tool` and writes into `writer`. @@ -297,9 +302,17 @@ 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 diff_wc = check_out_trees( + store, + left_tree, + right_tree, + matcher, + None, + conflict_marker_style, + )?; 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 18cf8546b36..64c40f0f3c7 100644 --- a/cli/src/merge_tools/mod.rs +++ b/cli/src/merge_tools/mod.rs @@ -21,6 +21,7 @@ use std::sync::Arc; use config::ConfigError; use jj_lib::backend::MergedTreeId; 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; @@ -181,6 +182,7 @@ pub struct DiffEditor { tool: MergeTool, base_ignores: Arc<GitIgnoreFile>, use_instructions: bool, + conflict_marker_style: ConflictMarkerStyle, } impl DiffEditor { @@ -221,6 +223,7 @@ impl DiffEditor { tool, base_ignores, use_instructions: settings.config().get_bool("ui.diff-instructions")?, + conflict_marker_style: settings.conflict_marker_style()?, }) } @@ -234,7 +237,10 @@ impl DiffEditor { ) -> Result<MergedTreeId, DiffEditError> { 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 +251,7 @@ impl DiffEditor { matcher, instructions.as_deref(), self.base_ignores.clone(), + self.conflict_marker_style, ) } } @@ -255,6 +262,7 @@ impl DiffEditor { #[derive(Clone, Debug)] pub struct MergeEditor { tool: MergeTool, + conflict_marker_file: ConflictMarkerStyle, } impl MergeEditor { @@ -263,7 +271,7 @@ impl MergeEditor { pub fn with_name(name: &str, settings: &UserSettings) -> Result<Self, MergeToolConfigError> { let tool = get_tool_config(settings, name)? .unwrap_or_else(|| MergeTool::external(ExternalMergeTool::with_program(name))); - Self::new_inner(name, tool) + Self::new_inner(settings, name, tool) } /// Loads the default 3-way merge editor from the settings. @@ -275,16 +283,23 @@ impl MergeEditor { None } .unwrap_or_else(|| MergeTool::external(ExternalMergeTool::with_merge_args(&args))); - Self::new_inner(&args, tool) + Self::new_inner(settings, &args, tool) } - fn new_inner(name: impl ToString, tool: MergeTool) -> Result<Self, MergeToolConfigError> { + fn new_inner( + settings: &UserSettings, + name: impl ToString, + tool: MergeTool, + ) -> Result<Self, MergeToolConfigError> { 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_file: settings.conflict_marker_style()?, + }) } /// Starts a merge editor for the specified file. @@ -319,7 +334,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_file, ), } } diff --git a/lib/src/annotate.rs b/lib/src/annotate.rs index a238d385814..6a212d276cc 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; use crate::conflicts::materialize_tree_value; +use crate::conflicts::ConflictMarkerStyle; use crate::conflicts::MaterializedTreeValue; use crate::diff::Diff; use crate::diff::DiffHunkKind; @@ -130,9 +131,13 @@ impl Source { } } - fn load(commit: &Commit, file_path: &RepoPath) -> Result<Self, BackendError> { + fn load( + commit: &Commit, + file_path: &RepoPath, + conflict_marker_style: ConflictMarkerStyle, + ) -> Result<Self, BackendError> { let tree = commit.tree()?; - let text = get_file_contents(commit.store(), file_path, &tree)?; + let text = get_file_contents(commit.store(), file_path, &tree, conflict_marker_style)?; Ok(Self::new(text.into())) } @@ -158,9 +163,17 @@ pub fn get_annotation_for_file( starting_commit: &Commit, domain: &Rc<ResolvedRevsetExpression>, file_path: &RepoPath, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<FileAnnotation, RevsetEvaluationError> { - let source = Source::load(starting_commit, file_path)?; - compute_file_annotation(repo, starting_commit.id(), domain, file_path, source) + let source = Source::load(starting_commit, file_path, conflict_marker_style)?; + compute_file_annotation( + repo, + starting_commit.id(), + domain, + file_path, + source, + conflict_marker_style, + ) } /// Get line by line annotations for a specific file path starting with the @@ -176,9 +189,17 @@ pub fn get_annotation_with_file_content( domain: &Rc<ResolvedRevsetExpression>, file_path: &RepoPath, starting_text: impl Into<Vec<u8>>, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<FileAnnotation, RevsetEvaluationError> { let source = Source::new(BString::new(starting_text.into())); - compute_file_annotation(repo, starting_commit_id, domain, file_path, source) + compute_file_annotation( + repo, + starting_commit_id, + domain, + file_path, + source, + conflict_marker_style, + ) } fn compute_file_annotation( @@ -187,10 +208,18 @@ fn compute_file_annotation( domain: &Rc<ResolvedRevsetExpression>, file_path: &RepoPath, mut source: Source, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<FileAnnotation, RevsetEvaluationError> { source.fill_line_map(); let text = source.text.clone(); - let line_map = process_commits(repo, starting_commit_id, source, domain, file_path)?; + let line_map = process_commits( + repo, + starting_commit_id, + source, + domain, + file_path, + conflict_marker_style, + )?; Ok(FileAnnotation { line_map, text }) } @@ -203,6 +232,7 @@ fn process_commits( starting_source: Source, domain: &Rc<ResolvedRevsetExpression>, file_name: &RepoPath, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<OriginalLineMap, RevsetEvaluationError> { let predicate = RevsetFilterPredicate::File(FilesetExpression::file_path(file_name.to_owned())); // TODO: If the domain isn't a contiguous range, changes masked out by it @@ -227,6 +257,7 @@ fn process_commits( &mut commit_source_map, &commit_id, &edge_list, + conflict_marker_style, )?; if commit_source_map.is_empty() { // No more lines to propagate to ancestors. @@ -246,6 +277,7 @@ fn process_commit( commit_source_map: &mut CommitSourceMap, current_commit_id: &CommitId, edges: &[GraphEdge<CommitId>], + conflict_marker_style: ConflictMarkerStyle, ) -> Result<(), BackendError> { let Some(mut current_source) = commit_source_map.remove(current_commit_id) else { return Ok(()); @@ -257,7 +289,7 @@ fn process_commit( hash_map::Entry::Occupied(entry) => entry.into_mut(), hash_map::Entry::Vacant(entry) => { let commit = repo.store().get_commit(entry.key())?; - entry.insert(Source::load(&commit, file_name)?) + entry.insert(Source::load(&commit, file_name, conflict_marker_style)?) } }; @@ -343,6 +375,7 @@ fn get_file_contents( store: &Store, path: &RepoPath, tree: &MergedTree, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<Vec<u8>, BackendError> { let file_value = tree.path_value(path)?; let effective_file_value = materialize_tree_value(store, path, file_value).block_on()?; @@ -360,13 +393,16 @@ fn get_file_contents( } MaterializedTreeValue::FileConflict { id, contents, .. } => { let mut materialized_conflict_buffer = Vec::new(); - materialize_merge_result(&contents, &mut materialized_conflict_buffer).map_err( - |io_err| BackendError::ReadFile { - path: path.to_owned(), - source: Box::new(io_err), - id: id.first().clone().unwrap(), - }, - )?; + materialize_merge_result( + &contents, + conflict_marker_style, + &mut materialized_conflict_buffer, + ) + .map_err(|io_err| BackendError::ReadFile { + path: path.to_owned(), + source: Box::new(io_err), + id: id.first().clone().unwrap(), + })?; Ok(materialized_conflict_buffer) } _ => Ok(Vec::new()), diff --git a/lib/src/conflicts.rs b/lib/src/conflicts.rs index 958d96ed48e..ca522ef473e 100644 --- a/lib/src/conflicts.rs +++ b/lib/src/conflicts.rs @@ -134,6 +134,14 @@ pub async fn extract_as_single_hunk( Ok(builder.build()) } +/// Describes what style should be used when materializing conflicts +#[derive(Clone, Copy, PartialEq, Eq, Debug, serde::Deserialize)] +#[serde(rename_all = "kebab-case")] +pub enum ConflictMarkerStyle { + /// Style which shows a snapshot and a series of diffs to apply + Diff, +} + /// A type similar to `MergedTreeValue` but with associated data to include in /// e.g. the working copy or in a diff. pub enum MaterializedTreeValue { @@ -233,6 +241,7 @@ async fn materialize_tree_value_no_access_denied( pub fn materialize_merge_result( single_hunk: &Merge<BString>, + _conflict_marker_style: ConflictMarkerStyle, output: &mut dyn Write, ) -> std::io::Result<()> { let merge_result = files::merge(single_hunk); @@ -493,6 +502,7 @@ pub async fn update_from_content( store: &Store, path: &RepoPath, content: &[u8], + conflict_marker_style: ConflictMarkerStyle, ) -> BackendResult<Merge<Option<FileId>>> { let simplified_file_ids = file_ids.clone().simplify(); let simplified_file_ids = &simplified_file_ids; @@ -504,7 +514,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 c5e57e82066..544c43fd5a4 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; use crate::conflicts::materialize_tree_value; +use crate::conflicts::ConflictMarkerStyle; use crate::conflicts::MaterializedTreeValue; use crate::default_index::AsCompositeIndex; use crate::default_index::CompositeIndex; @@ -1300,8 +1301,10 @@ fn matches_diff_from_parent( let left_future = materialize_tree_value(store, &entry.path, left_value); let right_future = materialize_tree_value(store, &entry.path, right_value); let (left_value, right_value) = futures::try_join!(left_future, right_future)?; - let left_content = to_file_content(&entry.path, left_value)?; - let right_content = to_file_content(&entry.path, right_value)?; + // Use "diff" setting for now (will be updated in later commit) + let left_content = to_file_content(&entry.path, left_value, ConflictMarkerStyle::Diff)?; + let right_content = + to_file_content(&entry.path, right_value, ConflictMarkerStyle::Diff)?; // Filter lines prior to comparison. This might produce inferior // hunks due to lack of contexts, but is way faster than full diff. let left_lines = match_lines(&left_content, text_pattern); @@ -1328,7 +1331,11 @@ fn match_lines<'a: 'b, 'b>( }) } -fn to_file_content(path: &RepoPath, value: MaterializedTreeValue) -> BackendResult<Vec<u8>> { +fn to_file_content( + path: &RepoPath, + value: MaterializedTreeValue, + conflict_marker_style: ConflictMarkerStyle, +) -> BackendResult<Vec<u8>> { match value { MaterializedTreeValue::Absent => Ok(vec![]), MaterializedTreeValue::AccessDenied(_) => Ok(vec![]), @@ -1347,7 +1354,7 @@ fn to_file_content(path: &RepoPath, value: MaterializedTreeValue) -> BackendResu MaterializedTreeValue::GitSubmodule(_) => Ok(vec![]), MaterializedTreeValue::FileConflict { contents, .. } => { let mut content = vec![]; - materialize_merge_result(&contents, &mut content) + materialize_merge_result(&contents, conflict_marker_style, &mut content) .expect("Failed to materialize conflict to in-memory buffer"); Ok(content) } diff --git a/lib/src/local_working_copy.rs b/lib/src/local_working_copy.rs index 0b6ff39d09c..6bd301a8bf6 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; 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; @@ -335,6 +336,7 @@ pub struct TreeState { sparse_patterns: Vec<RepoPathBuf>, own_mtime: MillisSinceEpoch, symlink_support: bool, + conflict_marker_style: ConflictMarkerStyle, /// The most recent clock value returned by Watchman. Will only be set if /// the repo is configured to use the Watchman filesystem monitor and @@ -678,13 +680,19 @@ impl TreeState { store: Arc<Store>, working_copy_path: PathBuf, state_path: PathBuf, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<TreeState, TreeStateError> { - let mut wc = TreeState::empty(store, working_copy_path, state_path); + let mut wc = TreeState::empty(store, working_copy_path, state_path, conflict_marker_style); wc.save()?; Ok(wc) } - fn empty(store: Arc<Store>, working_copy_path: PathBuf, state_path: PathBuf) -> TreeState { + fn empty( + store: Arc<Store>, + working_copy_path: PathBuf, + state_path: PathBuf, + conflict_marker_style: ConflictMarkerStyle, + ) -> TreeState { let tree_id = store.empty_merged_tree_id(); TreeState { store, @@ -695,6 +703,7 @@ impl TreeState { sparse_patterns: vec![RepoPathBuf::root()], own_mtime: MillisSinceEpoch(0), symlink_support: check_symlink_support().unwrap_or(false), + conflict_marker_style, watchman_clock: None, } } @@ -703,11 +712,17 @@ impl TreeState { store: Arc<Store>, working_copy_path: PathBuf, state_path: PathBuf, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<TreeState, TreeStateError> { let tree_state_path = state_path.join("tree_state"); let file = match File::open(&tree_state_path) { Err(ref err) if err.kind() == io::ErrorKind::NotFound => { - return TreeState::init(store, working_copy_path, state_path); + return TreeState::init( + store, + working_copy_path, + state_path, + conflict_marker_style, + ); } Err(err) => { return Err(TreeStateError::ReadTreeState { @@ -718,7 +733,7 @@ impl TreeState { Ok(file) => file, }; - let mut wc = TreeState::empty(store, working_copy_path, state_path); + let mut wc = TreeState::empty(store, working_copy_path, state_path, conflict_marker_style); wc.read(&tree_state_path, file)?; Ok(wc) } @@ -906,6 +921,7 @@ impl TreeState { progress, start_tracking_matcher, max_new_file_size, + conflict_marker_style, } = options; let sparse_matcher = self.sparse_matcher(); @@ -950,6 +966,7 @@ impl TreeState { directory_to_visit, *progress, *max_new_file_size, + *conflict_marker_style, ) })?; @@ -1024,6 +1041,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 +1127,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 +1158,7 @@ impl TreeState { directory_to_visit, progress, max_new_file_size, + conflict_marker_style, )?; } } else if matcher.matches(&path) { @@ -1177,6 +1197,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 +1266,7 @@ impl TreeState { maybe_current_file_state: Option<&FileState>, current_tree: &MergedTree, new_file_state: &FileState, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<Option<MergedTreeValue>, SnapshotError> { let clean = match maybe_current_file_state { None => { @@ -1274,7 +1296,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 +1326,7 @@ impl TreeState { disk_path: &Path, current_tree_values: &MergedTreeValue, executable: FileExecutableFlag, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<MergedTreeValue, SnapshotError> { // 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 +1355,7 @@ impl TreeState { self.store.as_ref(), repo_path, &content, + conflict_marker_style, ) .block_on()?; match new_file_ids.into_resolved() { @@ -1598,7 +1628,7 @@ impl TreeState { executable, } => { let mut data = vec![]; - materialize_merge_result(&contents, &mut data) + materialize_merge_result(&contents, self.conflict_marker_style, &mut data) .expect("Failed to materialize conflict to in-memory buffer"); self.write_conflict(&disk_path, data, executable)? } @@ -1700,6 +1730,7 @@ pub struct LocalWorkingCopy { state_path: PathBuf, checkout_state: OnceCell<CheckoutState>, tree_state: OnceCell<TreeState>, + conflict_marker_style: ConflictMarkerStyle, } impl WorkingCopy for LocalWorkingCopy { @@ -1743,6 +1774,7 @@ impl WorkingCopy for LocalWorkingCopy { // TODO: It's expensive to reload the whole tree. We should copy it from `self` if it // hasn't changed. tree_state: OnceCell::new(), + conflict_marker_style: self.conflict_marker_style, }; let old_operation_id = wc.operation_id().clone(); let old_tree_id = wc.tree_id()?.clone(); @@ -1771,6 +1803,7 @@ impl LocalWorkingCopy { state_path: PathBuf, operation_id: OperationId, workspace_id: WorkspaceId, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<LocalWorkingCopy, WorkingCopyStateError> { let proto = crate::protos::working_copy::Checkout { operation_id: operation_id.to_bytes(), @@ -1782,19 +1815,23 @@ impl LocalWorkingCopy { .open(state_path.join("checkout")) .unwrap(); file.write_all(&proto.encode_to_vec()).unwrap(); - let tree_state = - TreeState::init(store.clone(), working_copy_path.clone(), state_path.clone()).map_err( - |err| WorkingCopyStateError { - message: "Failed to initialize working copy state".to_string(), - err: err.into(), - }, - )?; + let tree_state = TreeState::init( + store.clone(), + working_copy_path.clone(), + state_path.clone(), + conflict_marker_style, + ) + .map_err(|err| WorkingCopyStateError { + message: "Failed to initialize working copy state".to_string(), + err: err.into(), + })?; Ok(LocalWorkingCopy { store, working_copy_path, state_path, checkout_state: OnceCell::new(), tree_state: OnceCell::with_value(tree_state), + conflict_marker_style, }) } @@ -1802,6 +1839,7 @@ impl LocalWorkingCopy { store: Arc<Store>, working_copy_path: PathBuf, state_path: PathBuf, + conflict_marker_style: ConflictMarkerStyle, ) -> LocalWorkingCopy { LocalWorkingCopy { store, @@ -1809,6 +1847,7 @@ impl LocalWorkingCopy { state_path, checkout_state: OnceCell::new(), tree_state: OnceCell::new(), + conflict_marker_style, } } @@ -1857,6 +1896,7 @@ impl LocalWorkingCopy { self.store.clone(), self.working_copy_path.clone(), self.state_path.clone(), + self.conflict_marker_style, ) }) .map_err(|err| WorkingCopyStateError { @@ -1919,6 +1959,7 @@ impl WorkingCopyFactory for LocalWorkingCopyFactory { state_path: PathBuf, operation_id: OperationId, workspace_id: WorkspaceId, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<Box<dyn WorkingCopy>, WorkingCopyStateError> { Ok(Box::new(LocalWorkingCopy::init( store, @@ -1926,6 +1967,7 @@ impl WorkingCopyFactory for LocalWorkingCopyFactory { state_path, operation_id, workspace_id, + conflict_marker_style, )?)) } @@ -1934,11 +1976,13 @@ impl WorkingCopyFactory for LocalWorkingCopyFactory { store: Arc<Store>, working_copy_path: PathBuf, state_path: PathBuf, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<Box<dyn WorkingCopy>, WorkingCopyStateError> { Ok(Box::new(LocalWorkingCopy::load( store, working_copy_path, state_path, + conflict_marker_style, ))) } } diff --git a/lib/src/settings.rs b/lib/src/settings.rs index 6158debc814..36b3ebbe720 100644 --- a/lib/src/settings.rs +++ b/lib/src/settings.rs @@ -26,6 +26,7 @@ use crate::backend::ChangeId; use crate::backend::Commit; use crate::backend::Signature; use crate::backend::Timestamp; +use crate::conflicts::ConflictMarkerStyle; use crate::fmt_util::binary_prefix; use crate::fsmonitor::FsmonitorSettings; use crate::signing::SignBehavior; @@ -261,6 +262,14 @@ impl UserSettings { pub fn sign_settings(&self) -> SignSettings { SignSettings::from_settings(self) } + + pub fn conflict_marker_style(&self) -> Result<ConflictMarkerStyle, config::ConfigError> { + Ok(self + .config + .get::<ConflictMarkerStyle>("ui.conflict-marker-style") + .optional()? + .unwrap_or(ConflictMarkerStyle::Diff)) + } } /// This Rng uses interior mutability to allow generating random values using an diff --git a/lib/src/working_copy.rs b/lib/src/working_copy.rs index 4555e7cd28b..817914ed03f 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; @@ -87,6 +88,7 @@ pub trait WorkingCopyFactory { state_path: PathBuf, operation_id: OperationId, workspace_id: WorkspaceId, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<Box<dyn WorkingCopy>, WorkingCopyStateError>; /// Load an existing working copy. @@ -95,6 +97,7 @@ pub trait WorkingCopyFactory { store: Arc<Store>, working_copy_path: PathBuf, state_path: PathBuf, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<Box<dyn WorkingCopy>, WorkingCopyStateError>; } @@ -222,6 +225,8 @@ pub struct SnapshotOptions<'a> { /// (depending on implementation) /// return `SnapshotError::NewFileTooLarge`. pub max_new_file_size: u64, + /// Conflict marker style to use when materializing files + pub conflict_marker_style: ConflictMarkerStyle, } impl SnapshotOptions<'_> { @@ -233,6 +238,7 @@ impl SnapshotOptions<'_> { progress: None, start_tracking_matcher: &EverythingMatcher, max_new_file_size: u64::MAX, + conflict_marker_style: ConflictMarkerStyle::Diff, } } } diff --git a/lib/src/workspace.rs b/lib/src/workspace.rs index 801e7115fca..3dc43cbbfc1 100644 --- a/lib/src/workspace.rs +++ b/lib/src/workspace.rs @@ -23,11 +23,13 @@ use std::path::Path; use std::path::PathBuf; use std::sync::Arc; +use config::ConfigError; use thiserror::Error; use crate::backend::BackendInitError; use crate::backend::MergedTreeId; use crate::commit::Commit; +use crate::conflicts::ConflictMarkerStyle; use crate::file_util::IoResultExt as _; use crate::file_util::PathError; use crate::local_backend::LocalBackend; @@ -78,6 +80,8 @@ pub enum WorkspaceInitError { Backend(#[from] BackendInitError), #[error(transparent)] SignInit(#[from] SignInitError), + #[error(transparent)] + Config(#[from] ConfigError), } #[derive(Error, Debug)] @@ -94,6 +98,8 @@ pub enum WorkspaceLoadError { WorkingCopyState(#[from] WorkingCopyStateError), #[error(transparent)] Path(#[from] PathError), + #[error(transparent)] + Config(#[from] ConfigError), } /// The combination of a repo and a working copy. @@ -147,6 +153,7 @@ fn init_working_copy( working_copy_state_path.clone(), repo.op_id().clone(), workspace_id, + user_settings.conflict_marker_style()?, )?; let working_copy_type_path = working_copy_state_path.join("type"); fs::write(&working_copy_type_path, working_copy.name()).context(&working_copy_type_path)?; @@ -524,6 +531,7 @@ pub trait WorkspaceLoader { &self, store: &Arc<Store>, working_copy_factory: &dyn WorkingCopyFactory, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<Box<dyn WorkingCopy>, WorkspaceLoadError>; } @@ -599,7 +607,11 @@ impl WorkspaceLoader for DefaultWorkspaceLoader { let repo_loader = RepoLoader::init_from_file_system(user_settings, &self.repo_path, store_factories)?; let working_copy_factory = get_working_copy_factory(self, working_copy_factories)?; - let working_copy = self.load_working_copy(repo_loader.store(), working_copy_factory)?; + let working_copy = self.load_working_copy( + repo_loader.store(), + working_copy_factory, + user_settings.conflict_marker_style()?, + )?; let workspace = Workspace::new( &self.workspace_root, self.repo_path.clone(), @@ -617,11 +629,13 @@ impl WorkspaceLoader for DefaultWorkspaceLoader { &self, store: &Arc<Store>, working_copy_factory: &dyn WorkingCopyFactory, + conflict_marker_style: ConflictMarkerStyle, ) -> Result<Box<dyn WorkingCopy>, WorkspaceLoadError> { Ok(working_copy_factory.load_working_copy( store.clone(), self.workspace_root.clone(), self.working_copy_state_path.clone(), + conflict_marker_style, )?) } } diff --git a/lib/tests/test_annotate.rs b/lib/tests/test_annotate.rs index d0cc459e128..09cae455b59 100644 --- a/lib/tests/test_annotate.rs +++ b/lib/tests/test_annotate.rs @@ -25,6 +25,7 @@ use jj_lib::backend::Signature; use jj_lib::backend::Timestamp; use jj_lib::backend::TreeValue; use jj_lib::commit::Commit; +use jj_lib::conflicts::ConflictMarkerStyle; use jj_lib::repo::MutableRepo; use jj_lib::repo::Repo; use jj_lib::repo_path::RepoPath; @@ -70,7 +71,9 @@ fn annotate_within( domain: &Rc<ResolvedRevsetExpression>, file_path: &RepoPath, ) -> String { - let annotation = get_annotation_for_file(repo, commit, domain, file_path).unwrap(); + let annotation = + get_annotation_for_file(repo, commit, domain, file_path, ConflictMarkerStyle::Diff) + .unwrap(); format_annotation(repo, &annotation) } @@ -86,8 +89,15 @@ fn annotate_parent_tree(repo: &dyn Repo, commit: &Commit, file_path: &RepoPath) value => panic!("unexpected path value: {value:?}"), }; let domain = RevsetExpression::all(); - let annotation = - get_annotation_with_file_content(repo, commit.id(), &domain, file_path, text).unwrap(); + let annotation = get_annotation_with_file_content( + repo, + commit.id(), + &domain, + file_path, + text, + ConflictMarkerStyle::Diff, + ) + .unwrap(); format_annotation(repo, &annotation) } diff --git a/lib/tests/test_conflicts.rs b/lib/tests/test_conflicts.rs index 75066a94e3e..7a4dcb00155 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; 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; @@ -835,7 +836,7 @@ fn test_update_conflict_from_content() { // old conflict id back. let materialized = materialize_conflict_string(store, path, &conflict); let parse = |content| { - update_from_content(&conflict, store, path, content) + update_from_content(&conflict, store, path, content, ConflictMarkerStyle::Diff) .block_on() .unwrap() }; @@ -884,7 +885,7 @@ fn test_update_conflict_from_content_modify_delete() { // old conflict id back. let materialized = materialize_conflict_string(store, path, &conflict); let parse = |content| { - update_from_content(&conflict, store, path, content) + update_from_content(&conflict, store, path, content, ConflictMarkerStyle::Diff) .block_on() .unwrap() }; @@ -938,7 +939,7 @@ fn test_update_conflict_from_content_simplified_conflict() { let materialized = materialize_conflict_string(store, path, &conflict); let materialized_simplified = materialize_conflict_string(store, path, &simplified_conflict); let parse = |content| { - update_from_content(&conflict, store, path, content) + update_from_content(&conflict, store, path, content, ConflictMarkerStyle::Diff) .block_on() .unwrap() }; @@ -1067,6 +1068,6 @@ fn materialize_conflict_string( let contents = extract_as_single_hunk(conflict, store, path) .block_on() .unwrap(); - materialize_merge_result(&contents, &mut result).unwrap(); + materialize_merge_result(&contents, ConflictMarkerStyle::Diff, &mut result).unwrap(); String::from_utf8(result).unwrap() } diff --git a/lib/tests/test_local_working_copy.rs b/lib/tests/test_local_working_copy.rs index f4fa960c111..38f060a4484 100644 --- a/lib/tests/test_local_working_copy.rs +++ b/lib/tests/test_local_working_copy.rs @@ -26,6 +26,7 @@ use itertools::Itertools; use jj_lib::backend::MergedTreeId; use jj_lib::backend::TreeId; use jj_lib::backend::TreeValue; +use jj_lib::conflicts::ConflictMarkerStyle; use jj_lib::file_util::check_symlink_support; use jj_lib::file_util::try_symlink; use jj_lib::fsmonitor::FsmonitorSettings; @@ -671,8 +672,12 @@ fn test_checkout_discard() { // 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()); - let reloaded_wc = - LocalWorkingCopy::load(store.clone(), workspace_root.clone(), state_path.clone()); + let reloaded_wc = LocalWorkingCopy::load( + store.clone(), + workspace_root.clone(), + state_path.clone(), + ConflictMarkerStyle::Diff, + ); assert!(reloaded_wc.file_states().unwrap().contains_path(file1_path)); assert!(!reloaded_wc.file_states().unwrap().contains_path(file2_path)); drop(locked_ws); @@ -683,7 +688,12 @@ fn test_checkout_discard() { assert!(!wc.file_states().unwrap().contains_path(file2_path)); assert!(!file1_path.to_fs_path_unchecked(&workspace_root).is_file()); assert!(file2_path.to_fs_path_unchecked(&workspace_root).is_file()); - let reloaded_wc = LocalWorkingCopy::load(store.clone(), workspace_root, state_path); + let reloaded_wc = LocalWorkingCopy::load( + store.clone(), + workspace_root, + state_path, + ConflictMarkerStyle::Diff, + ); assert!(reloaded_wc.file_states().unwrap().contains_path(file1_path)); assert!(!reloaded_wc.file_states().unwrap().contains_path(file2_path)); } diff --git a/lib/tests/test_local_working_copy_sparse.rs b/lib/tests/test_local_working_copy_sparse.rs index fb21726b1ec..a652635323d 100644 --- a/lib/tests/test_local_working_copy_sparse.rs +++ b/lib/tests/test_local_working_copy_sparse.rs @@ -14,6 +14,7 @@ use futures::StreamExt as _; use itertools::Itertools; +use jj_lib::conflicts::ConflictMarkerStyle; use jj_lib::local_working_copy::LocalWorkingCopy; use jj_lib::matchers::EverythingMatcher; use jj_lib::repo::Repo; @@ -119,6 +120,7 @@ fn test_sparse_checkout() { repo.store().clone(), ws.workspace_root().to_path_buf(), wc.state_path().to_path_buf(), + ConflictMarkerStyle::Diff, ); assert_eq!( wc.file_states().unwrap().paths().collect_vec(),