Skip to content

Commit

Permalink
conflicts: add "ui.conflict-marker-style" config
Browse files Browse the repository at this point in the history
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 jj-vcs#1
fn example(word: String) {
    println!("word is {word}");
%%%%%%% Changes from base to side jj-vcs#2
-fn example(w: String) {
+fn example(w: &str) {
     println!("word is {w}");
>>>>>>> Conflict 1 of 1 ends
}
```
  • Loading branch information
scott2000 committed Nov 18, 2024
1 parent 896ab5f commit f977575
Show file tree
Hide file tree
Showing 37 changed files with 494 additions and 95 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

* `jj config unset <TABLE-NAME>` no longer removes a table (such as `[ui]`.)
Expand Down
21 changes: 19 additions & 2 deletions cli/examples/custom-working-copy/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -119,22 +120,34 @@ impl ConflictsWorkingCopy {
state_path: PathBuf,
operation_id: OperationId,
workspace_id: WorkspaceId,
conflict_marker_style: ConflictMarkerStyle,
) -> Result<Self, WorkingCopyStateError> {
let inner = LocalWorkingCopy::init(
store,
working_copy_path.clone(),
state_path,
operation_id,
workspace_id,
conflict_marker_style,
)?;
Ok(ConflictsWorkingCopy {
inner: Box::new(inner),
working_copy_path,
})
}

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,
Expand Down Expand Up @@ -186,13 +199,15 @@ 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,
working_copy_path,
state_path,
operation_id,
workspace_id,
conflict_marker_style,
)?))
}

Expand All @@ -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,
)))
}
}
Expand Down
13 changes: 13 additions & 0 deletions cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand All @@ -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)?;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
)
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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),
}
}

Expand Down
1 change: 1 addition & 0 deletions cli/src/command_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
}
}
Expand Down
4 changes: 4 additions & 0 deletions cli/src/commands/absorb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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())?;
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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()
Expand Down
1 change: 1 addition & 0 deletions cli/src/commands/diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ pub(crate) fn cmd_diff(
&matcher,
&copy_records,
ui.term_width(),
workspace_command.env().conflict_marker_style(),
)?;
print_unmatched_explicit_paths(
ui,
Expand Down
2 changes: 2 additions & 0 deletions cli/src/commands/evolog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -198,6 +199,7 @@ pub(crate) fn cmd_evolog(
&commit,
&EverythingMatcher,
width,
workspace_command.env().conflict_marker_style(),
)?;
}
}
Expand Down
8 changes: 7 additions & 1 deletion cli/src/commands/file/annotate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
Expand Down
6 changes: 5 additions & 1 deletion cli/src/commands/file/show.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())?;
Expand Down
2 changes: 2 additions & 0 deletions cli/src/commands/file/track.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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 {
Expand Down
2 changes: 2 additions & 0 deletions cli/src/commands/file/untrack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?
Expand Down Expand Up @@ -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)?;
Expand Down
1 change: 1 addition & 0 deletions cli/src/commands/interdiff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ pub(crate) fn cmd_interdiff(
&to,
matcher.as_ref(),
ui.term_width(),
workspace_command.env().conflict_marker_style(),
)?;
Ok(())
}
10 changes: 9 additions & 1 deletion cli/src/commands/log.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ pub(crate) fn cmd_log(
&commit,
matcher.as_ref(),
within_graph.width(),
workspace_command.env().conflict_marker_style(),
)?;
}

Expand Down Expand Up @@ -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(),
)?;
}
}
}
Expand Down
24 changes: 22 additions & 2 deletions cli/src/commands/operation/diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(),
)
}

Expand All @@ -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)?;

Expand Down Expand Up @@ -255,6 +258,7 @@ pub fn show_op_diff(
diff_renderer,
modified_change,
within_graph.width(),
conflict_marker_style,
)?;
}

Expand All @@ -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,
)?;
}
}
}
Expand Down Expand Up @@ -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]) => {
Expand All @@ -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"),
Expand Down
Loading

0 comments on commit f977575

Please sign in to comment.