diffedit: start the builtin ui with all boxes checked

(fixes #5390)

Currently, there is a blocker: when quitting the ui with 'q' after making no
changes, you get prompted with 'you have modified N files, are you sure?', which
seems like a bug in the scm_record library.

CHANGELOG.md

@@ -58,6 +58,10 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
   [streampager](https://github.com/markbt/streampager/). It can handle large
   inputs better.
 
+* The terminal UI of `jj diffedit` now starts with the whole diff checked, meaning
+  you need to uncheck what you want to discard, instead of checking what you want to
+  preserve.
+
 ### Deprecations
 
 ### New features

cli/src/cli_util.rs

@@ -164,6 +164,7 @@ use crate::formatter::FormatRecorder;
 use crate::formatter::Formatter;
 use crate::formatter::PlainTextFormatter;
 use crate::merge_tools::DiffEditor;
+use crate::merge_tools::InitialSelection;
 use crate::merge_tools::MergeEditor;
 use crate::merge_tools::MergeToolConfigError;
 use crate::operation_templater::OperationTemplateLanguage;
@@ -2950,7 +2951,13 @@ impl DiffSelector {
                 // whereas we want to update the left tree. Unmatched paths
                 // shouldn't be based off the right tree.
                 let right_tree = right_tree.store().get_root_tree(&selected_tree_id)?;
-                Ok(editor.edit(left_tree, &right_tree, matcher, format_instructions)?)
+                Ok(editor.edit(
+                    left_tree,
+                    &right_tree,
+                    matcher,
+                    format_instructions,
+                    InitialSelection::None,
+                )?)
             }
         }
     }

cli/src/commands/diffedit.rs

@@ -25,6 +25,7 @@ use crate::cli_util::CommandHelper;
 use crate::cli_util::RevisionArg;
 use crate::command_error::CommandError;
 use crate::complete;
+use crate::merge_tools::InitialSelection;
 use crate::ui::Ui;
 
 /// Touch up the content changes in a revision with a diff editor
@@ -132,7 +133,13 @@ don't make any changes, then the operation will be aborted.",
     };
     let base_tree = merge_commit_trees(tx.repo(), base_commits.as_slice())?;
     let tree = target_commit.tree()?;
-    let tree_id = diff_editor.edit(&base_tree, &tree, &EverythingMatcher, format_instructions)?;
+    let tree_id = diff_editor.edit(
+        &base_tree,
+        &tree,
+        &EverythingMatcher,
+        format_instructions,
+        InitialSelection::All,
+    )?;
     if tree_id == *target_commit.tree_id() {
         writeln!(ui.status(), "Nothing changed.")?;
     } else {

cli/src/commands/unsquash.rs

@@ -21,6 +21,7 @@ use crate::cli_util::RevisionArg;
 use crate::command_error::user_error;
 use crate::command_error::CommandError;
 use crate::description_util::combine_messages;
+use crate::merge_tools::InitialSelection;
 use crate::ui::Ui;
 
 /// Move changes from a revision's parent into the revision
@@ -106,6 +107,7 @@ aborted.
             &parent_tree,
             &EverythingMatcher,
             format_instructions,
+            InitialSelection::None,
         )?;
         if new_parent_tree_id == parent_base_tree.id() {
             return Err(user_error("No changes selected"));

cli/src/merge_tools/builtin.rs

@@ -233,14 +233,30 @@ fn read_file_contents(
     }
 }
 
+#[derive(Clone, Copy, Debug)]
+pub enum InitialSelection {
+    All,
+    None,
+}
+
+impl InitialSelection {
+    fn is_checked(&self) -> bool {
+        match self {
+            InitialSelection::All => true,
+            InitialSelection::None => false,
+        }
+    }
+}
+
 fn make_section_changed_lines(
     contents: &str,
     change_type: scm_record::ChangeType,
+    initial_selection: InitialSelection,
 ) -> Vec<scm_record::SectionChangedLine<'static>> {
     contents
         .split_inclusive('\n')
         .map(|line| scm_record::SectionChangedLine {
-            is_checked: false,
+            is_checked: initial_selection.is_checked(),
             change_type,
             line: Cow::Owned(line.to_owned()),
         })
@@ -250,6 +266,7 @@ fn make_section_changed_lines(
 fn make_diff_sections(
     left_contents: &str,
     right_contents: &str,
+    initial_selection: InitialSelection,
 ) -> Result<Vec<scm_record::Section<'static>>, BuiltinToolError> {
     let diff = Diff::by_line([left_contents.as_bytes(), right_contents.as_bytes()]);
     let mut sections = Vec::new();
@@ -285,8 +302,16 @@ fn make_diff_sections(
                     })?;
                 sections.push(scm_record::Section::Changed {
                     lines: [
-                        make_section_changed_lines(left_side, scm_record::ChangeType::Removed),
-                        make_section_changed_lines(right_side, scm_record::ChangeType::Added),
+                        make_section_changed_lines(
+                            left_side,
+                            scm_record::ChangeType::Removed,
+                            initial_selection,
+                        ),
+                        make_section_changed_lines(
+                            right_side,
+                            scm_record::ChangeType::Added,
+                            initial_selection,
+                        ),
                     ]
                     .concat(),
                 });
@@ -313,6 +338,7 @@ pub fn make_diff_files(
     right_tree: &MergedTree,
     changed_files: &[RepoPathBuf],
     conflict_marker_style: ConflictMarkerStyle,
+    initial_selection: InitialSelection,
 ) -> Result<Vec<scm_record::File<'static>>, BuiltinToolError> {
     let mut files = Vec::new();
     for changed_path in changed_files {
@@ -323,7 +349,7 @@ pub fn make_diff_files(
 
         if should_render_mode_section(&left_info, &right_info) {
             sections.push(scm_record::Section::FileMode {
-                is_checked: false,
+                is_checked: initial_selection.is_checked(),
                 before: left_info.file_mode,
                 after: right_info.file_mode,
             });
@@ -359,12 +385,16 @@ pub fn make_diff_files(
                     num_bytes: _,
                 },
             ) => sections.push(scm_record::Section::Changed {
-                lines: make_section_changed_lines(&contents, scm_record::ChangeType::Added),
+                lines: make_section_changed_lines(
+                    &contents,
+                    scm_record::ChangeType::Added,
+                    initial_selection,
+                ),
             }),
 
             (FileContents::Absent, FileContents::Binary { hash, num_bytes }) => {
                 sections.push(scm_record::Section::Binary {
-                    is_checked: false,
+                    is_checked: initial_selection.is_checked(),
                     old_description: None,
                     new_description: Some(Cow::Owned(describe_binary(hash.as_deref(), num_bytes))),
                 });
@@ -378,7 +408,11 @@ pub fn make_diff_files(
                 },
                 FileContents::Absent,
             ) => sections.push(scm_record::Section::Changed {
-                lines: make_section_changed_lines(&contents, scm_record::ChangeType::Removed),
+                lines: make_section_changed_lines(
+                    &contents,
+                    scm_record::ChangeType::Removed,
+                    initial_selection,
+                ),
             }),
 
             (
@@ -393,7 +427,11 @@ pub fn make_diff_files(
                     num_bytes: _,
                 },
             ) => {
-                sections.extend(make_diff_sections(&old_contents, &new_contents)?);
+                sections.extend(make_diff_sections(
+                    &old_contents,
+                    &new_contents,
+                    initial_selection,
+                )?);
             }
 
             (
@@ -416,7 +454,7 @@ pub fn make_diff_files(
                     num_bytes: new_num_bytes,
                 },
             ) => sections.push(scm_record::Section::Binary {
-                is_checked: false,
+                is_checked: initial_selection.is_checked(),
                 old_description: Some(Cow::Owned(describe_binary(
                     old_hash.as_deref(),
                     old_num_bytes,
@@ -429,7 +467,7 @@ pub fn make_diff_files(
 
             (FileContents::Binary { hash, num_bytes }, FileContents::Absent) => {
                 sections.push(scm_record::Section::Binary {
-                    is_checked: false,
+                    is_checked: initial_selection.is_checked(),
                     old_description: Some(Cow::Owned(describe_binary(hash.as_deref(), num_bytes))),
                     new_description: None,
                 });
@@ -528,6 +566,7 @@ pub fn edit_diff_builtin(
     right_tree: &MergedTree,
     matcher: &dyn Matcher,
     conflict_marker_style: ConflictMarkerStyle,
+    initial_selection: InitialSelection,
 ) -> Result<MergedTreeId, BuiltinToolError> {
     let store = left_tree.store().clone();
     // TODO: handle copy tracking
@@ -542,6 +581,7 @@ pub fn edit_diff_builtin(
         right_tree,
         &changed_files,
         conflict_marker_style,
+        initial_selection,
     )?;
     let mut input = scm_record::helpers::CrosstermInput;
     let recorder = scm_record::Recorder::new(
@@ -622,8 +662,11 @@ fn make_merge_sections(
                                         item: "conflicting hunk",
                                     }
                                 })?;
-                                let changed_lines =
-                                    make_section_changed_lines(contents, change_type);
+                                let changed_lines = make_section_changed_lines(
+                                    contents,
+                                    change_type,
+                                    InitialSelection::None,
+                                );
                                 Ok(changed_lines)
                             })
                             .flatten_ok()
@@ -731,6 +774,7 @@ mod tests {
             &right_tree,
             &changed_files,
             ConflictMarkerStyle::Diff,
+            InitialSelection::None,
         )
         .unwrap();
         insta::assert_debug_snapshot!(files, @r###"
@@ -868,6 +912,7 @@ mod tests {
             &right_tree,
             &changed_files,
             ConflictMarkerStyle::Diff,
+            InitialSelection::None,
         )
         .unwrap();
         insta::assert_debug_snapshot!(files, @r###"
@@ -939,6 +984,7 @@ mod tests {
             &right_tree,
             &changed_files,
             ConflictMarkerStyle::Diff,
+            InitialSelection::None,
         )
         .unwrap();
         insta::assert_debug_snapshot!(files, @r###"
@@ -1011,6 +1057,7 @@ mod tests {
             &right_tree,
             &changed_files,
             ConflictMarkerStyle::Diff,
+            InitialSelection::None,
         )
         .unwrap();
         insta::assert_debug_snapshot!(files, @r###"

cli/src/merge_tools/mod.rs

@@ -44,6 +44,7 @@ use thiserror::Error;
 use self::builtin::edit_diff_builtin;
 use self::builtin::edit_merge_builtin;
 use self::builtin::BuiltinToolError;
+pub use self::builtin::InitialSelection;
 pub(crate) use self::diff_working_copies::new_utf8_temp_dir;
 use self::diff_working_copies::DiffCheckoutError;
 use self::external::edit_diff_external;
@@ -255,14 +256,17 @@ impl DiffEditor {
         right_tree: &MergedTree,
         matcher: &dyn Matcher,
         format_instructions: impl FnOnce() -> String,
+        initial_selection: InitialSelection,
     ) -> Result<MergedTreeId, DiffEditError> {
         match &self.tool {
-            MergeTool::Builtin => {
-                Ok(
-                    edit_diff_builtin(left_tree, right_tree, matcher, self.conflict_marker_style)
-                        .map_err(Box::new)?,
-                )
-            }
+            MergeTool::Builtin => Ok(edit_diff_builtin(
+                left_tree,
+                right_tree,
+                matcher,
+                self.conflict_marker_style,
+                initial_selection,
+            )
+            .map_err(Box::new)?),
             MergeTool::External(editor) => {
                 let instructions = self.use_instructions.then(format_instructions);
                 edit_diff_external(