resolve: try to resolve all conflicted files in fileset

If many files are conflicted, it would be nice to be able to resolve all
conflicts at once without having to run `jj resolve` multiple times.
This is especially nice for merge tools which try to automatically
resolve conflicts without user input, but it is also good for regular
merge editors like VS Code.

This change makes the behavior of `jj resolve` more consistent with
other commands which accept filesets since it will use the entire
fileset instead of picking an arbitrary file from the fileset.

Since we don't support passing directories to merge tools yet, the
current implementation just calls the merge tool repeatedly in a loop
until every file is resolved, or until an error occurs. If an error
occurs after successfully resolving at least one file, it is downgraded
to a warning instead. This means the user can just close the editor at
any point to cancel resolution on all remaining files.

CHANGELOG.md

@@ -42,6 +42,10 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 * `jj absorb` now abandons the source commit if it becomes empty and has no
   description.
 
+* `jj resolve` will now attempt to resolve all conflicted files instead of
+  resolving the first conflicted file. To resolve a single file, pass a file
+  path to `jj resolve`.
+
 ### Deprecations
 
 * `--config-toml=TOML` is deprecated in favor of `--config=NAME=VALUE` and

cli/src/cli_util.rs

@@ -1459,9 +1459,19 @@ to the current parents may contain changes from multiple commits.
     ) -> Result<MergeEditor, MergeToolConfigError> {
         let conflict_marker_style = self.env.conflict_marker_style();
         if let Some(name) = tool_name {
-            MergeEditor::with_name(name, self.settings(), conflict_marker_style)
+            MergeEditor::with_name(
+                name,
+                self.settings(),
+                self.path_converter(),
+                conflict_marker_style,
+            )
         } else {
-            MergeEditor::from_settings(ui, self.settings(), conflict_marker_style)
+            MergeEditor::from_settings(
+                ui,
+                self.settings(),
+                self.path_converter(),
+                conflict_marker_style,
+            )
         }
     }
 

cli/src/command_error.rs

@@ -420,7 +420,12 @@ impl From<DiffRenderError> for CommandError {
 
 impl From<ConflictResolveError> for CommandError {
     fn from(err: ConflictResolveError) -> Self {
-        user_error_with_message("Failed to resolve conflicts", err)
+        match err {
+            ConflictResolveError::Backend(err) => err.into(),
+            ConflictResolveError::Io(err) => err.into(),
+            ConflictResolveError::PartialResolution { .. } => user_error(err),
+            _ => user_error_with_message("Failed to resolve conflicts", err),
+        }
     }
 }
 

cli/src/commands/resolve.rs

@@ -26,12 +26,16 @@ use crate::cli_util::RevisionArg;
 use crate::command_error::cli_error;
 use crate::command_error::CommandError;
 use crate::complete;
+use crate::merge_tools::ConflictResolveError;
 use crate::ui::Ui;
 
-/// Resolve a conflicted file with an external merge tool
+/// Resolve conflicted files with an external merge tool
 ///
 /// Only conflicts that can be resolved with a 3-way merge are supported. See
-/// docs for merge tool configuration instructions.
+/// docs for merge tool configuration instructions. External merge tools will be
+/// invoked for each conflicted file one-by-one until all conflicts are
+/// resolved. To stop resolving conflicts, exit the merge tool without making
+/// any changes.
 ///
 /// Note that conflicts can also be resolved without using this command. You may
 /// edit the conflict markers in the conflicted file directly with a text
@@ -52,18 +56,16 @@ pub(crate) struct ResolveArgs {
         add = ArgValueCandidates::new(complete::mutable_revisions),
     )]
     revision: RevisionArg,
-    /// Instead of resolving one conflict, list all the conflicts
+    /// Instead of resolving conflicts, list all the conflicts
     // TODO: Also have a `--summary` option. `--list` currently acts like
     // `diff --summary`, but should be more verbose.
     #[arg(long, short)]
     list: bool,
     /// Specify 3-way merge tool to be used
     #[arg(long, conflicts_with = "list", value_name = "NAME")]
     tool: Option<String>,
-    /// Restrict to these paths when searching for a conflict to resolve. We
-    /// will attempt to resolve the first conflict we can find. You can use
-    /// the `--list` argument to find paths to use here.
-    // TODO: Find the conflict we can resolve even if it's not the first one.
+    /// Only resolve conflicts in these paths. You can use the `--list` argument
+    /// to find paths to use here.
     #[arg(
         value_name = "FILESETS",
         value_hint = clap::ValueHint::AnyPath,
@@ -103,16 +105,32 @@ pub(crate) fn cmd_resolve(
         );
     };
 
-    let (repo_path, _) = conflicts.first().unwrap();
+    let repo_paths = conflicts
+        .iter()
+        .map(|(path, _)| path.as_ref())
+        .collect_vec();
     workspace_command.check_rewritable([commit.id()])?;
-    let merge_editor = workspace_command.merge_editor(ui, args.tool.as_deref())?;
-    writeln!(
-        ui.status(),
-        "Resolving conflicts in: {}",
-        workspace_command.format_file_path(repo_path)
-    )?;
     let mut tx = workspace_command.start_transaction();
-    let new_tree_id = merge_editor.edit_file(&tree, repo_path)?;
+    let merge_editor = tx
+        .base_workspace_helper()
+        .merge_editor(ui, args.tool.as_deref())?;
+    let mut suppressed_error = None;
+    let new_tree_id = match merge_editor.edit_files(ui, &tree, &repo_paths) {
+        Ok(new_tree_id) => new_tree_id,
+        Err(err) => {
+            // If resolution failed completely, we can return immediately since there's
+            // nothing to commit
+            let ConflictResolveError::PartialResolution { resolved_tree, .. } = &err else {
+                return Err(err.into());
+            };
+
+            // Some files were resolved successfully, so we want to commit the transaction
+            // before returning the error
+            let resolved_tree = resolved_tree.clone();
+            suppressed_error = Some(err.into());
+            resolved_tree
+        }
+    };
     let new_commit = tx
         .repo_mut()
         .rewrite_commit(command.settings(), &commit)
@@ -139,5 +157,9 @@ pub(crate) fn cmd_resolve(
             }
         }
     }
+
+    if let Some(err) = suppressed_error {
+        return Err(err);
+    }
     Ok(())
 }

cli/src/merge_tools/builtin.rs

@@ -658,26 +658,28 @@ fn make_merge_file(
 
 pub fn edit_merge_builtin(
     tree: &MergedTree,
-    merge_tool_file: &MergeToolFile,
+    merge_tool_files: &[MergeToolFile],
 ) -> Result<MergedTreeId, BuiltinToolError> {
     let mut input = scm_record::helpers::CrosstermInput;
     let recorder = scm_record::Recorder::new(
         scm_record::RecordState {
             is_read_only: false,
-            files: vec![make_merge_file(merge_tool_file)?],
+            files: merge_tool_files.iter().map(make_merge_file).try_collect()?,
             commits: Default::default(),
         },
         &mut input,
     );
     let state = recorder.run()?;
 
-    let file = state.files.into_iter().exactly_one().unwrap();
     apply_diff_builtin(
         tree.store(),
         tree,
         tree,
-        vec![merge_tool_file.repo_path.clone()],
-        &[file],
+        merge_tool_files
+            .iter()
+            .map(|file| file.repo_path.clone())
+            .collect_vec(),
+        &state.files,
     )
     .map_err(BuiltinToolError::BackendError)
 }

cli/src/merge_tools/external.rs

@@ -20,6 +20,8 @@ use jj_lib::matchers::Matcher;
 use jj_lib::merge::Merge;
 use jj_lib::merged_tree::MergedTree;
 use jj_lib::merged_tree::MergedTreeBuilder;
+use jj_lib::repo_path::RepoPathUiConverter;
+use jj_lib::store::Store;
 use jj_lib::working_copy::CheckoutOptions;
 use pollster::FutureExt;
 use thiserror::Error;
@@ -168,12 +170,13 @@ pub enum ExternalToolError {
     Io(#[source] std::io::Error),
 }
 
-pub fn run_mergetool_external(
+fn run_mergetool_external_single_file(
     editor: &ExternalMergeTool,
-    tree: &MergedTree,
+    store: &Store,
     merge_tool_file: &MergeToolFile,
     default_conflict_marker_style: ConflictMarkerStyle,
-) -> Result<MergedTreeId, ConflictResolveError> {
+    tree_builder: &mut MergedTreeBuilder,
+) -> Result<(), ConflictResolveError> {
     let MergeToolFile {
         repo_path,
         conflict,
@@ -273,16 +276,15 @@ pub fn run_mergetool_external(
     let new_file_ids = if editor.merge_tool_edits_conflict_markers || exit_status_implies_conflict {
         conflicts::update_from_content(
             file_merge,
-            tree.store(),
+            store,
             repo_path,
             output_file_contents.as_slice(),
             conflict_marker_style,
             conflict_marker_len,
         )
         .block_on()?
     } else {
-        let new_file_id = tree
-            .store()
+        let new_file_id = store
             .write_file(repo_path, &mut output_file_contents.as_slice())
             .block_on()?;
         Merge::normal(new_file_id)
@@ -310,8 +312,51 @@ pub fn run_mergetool_external(
         }),
         Err(new_file_ids) => conflict.with_new_file_ids(&new_file_ids),
     };
-    let mut tree_builder = MergedTreeBuilder::new(tree.id());
     tree_builder.set_or_remove(repo_path.to_owned(), new_tree_value);
+    Ok(())
+}
+
+pub fn run_mergetool_external(
+    ui: &Ui,
+    path_converter: &RepoPathUiConverter,
+    editor: &ExternalMergeTool,
+    tree: &MergedTree,
+    merge_tool_files: &[MergeToolFile],
+    default_conflict_marker_style: ConflictMarkerStyle,
+) -> Result<MergedTreeId, ConflictResolveError> {
+    // TODO: add support for "dir" invocation mode, similar to the
+    // "diff-invocation-mode" config option for diffs
+    let mut tree_builder = MergedTreeBuilder::new(tree.id());
+    for (i, merge_tool_file) in merge_tool_files.iter().enumerate() {
+        writeln!(
+            ui.status(),
+            "Resolving conflicts in: {}",
+            path_converter.format_file_path(&merge_tool_file.repo_path)
+        )?;
+        match run_mergetool_external_single_file(
+            editor,
+            tree.store(),
+            merge_tool_file,
+            default_conflict_marker_style,
+            &mut tree_builder,
+        ) {
+            Ok(()) => {}
+            Err(err) if i == 0 => {
+                // If the first resolution fails, just return the error normally
+                return Err(err);
+            }
+            Err(err) => {
+                // Some conflicts were already resolved, so we should return an error with the
+                // partially-resolved tree so that the caller can save the resolved files.
+                let resolved_tree = tree_builder.write_tree(tree.store())?;
+                return Err(ConflictResolveError::PartialResolution {
+                    source: Box::new(err),
+                    resolved_count: i,
+                    resolved_tree,
+                });
+            }
+        }
+    }
     let new_tree = tree_builder.write_tree(tree.store())?;
     Ok(new_tree)
 }