Skip to content

Commit

Permalink
cli: add a WorkspaceCommandHelper::edit_diff() proxy (#65, #87)
Browse files Browse the repository at this point in the history
I'm about to make callers pass in "base" Git ignores when writing a
tree from the working copy. `edit_diff()` will then need to pass
that. It'll be easier to do that if we have a proxy on
`WorkspaceCommandHelper`.
  • Loading branch information
martinvonz committed Mar 12, 2022
1 parent 51da2a2 commit 2291bff
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 13 deletions.
25 changes: 17 additions & 8 deletions src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use clap::{crate_version, Arg, ArgMatches, Command};
use criterion::Criterion;
use git2::{Oid, Repository};
use itertools::Itertools;
use jujutsu_lib::backend::{BackendError, CommitId, Timestamp, TreeValue};
use jujutsu_lib::backend::{BackendError, CommitId, Timestamp, TreeId, TreeValue};
use jujutsu_lib::commit::Commit;
use jujutsu_lib::commit_builder::CommitBuilder;
use jujutsu_lib::dag_walk::topo_order_reverse;
Expand All @@ -54,7 +54,7 @@ use jujutsu_lib::rewrite::{back_out_commit, merge_commit_trees, rebase_commit, D
use jujutsu_lib::settings::UserSettings;
use jujutsu_lib::store::Store;
use jujutsu_lib::transaction::Transaction;
use jujutsu_lib::tree::{merge_trees, TreeDiffIterator};
use jujutsu_lib::tree::{merge_trees, Tree, TreeDiffIterator};
use jujutsu_lib::working_copy::{CheckoutStats, ResetError, WorkingCopy};
use jujutsu_lib::workspace::{Workspace, WorkspaceInitError, WorkspaceLoadError};
use jujutsu_lib::{conflicts, dag_walk, diff, files, git, revset, tree};
Expand Down Expand Up @@ -589,6 +589,15 @@ impl WorkspaceCommandHelper {
Ok(())
}

fn edit_diff(
&self,
left_tree: &Tree,
right_tree: &Tree,
instructions: &str,
) -> Result<TreeId, DiffEditError> {
crate::diff_edit::edit_diff(&self.settings, left_tree, right_tree, instructions)
}

fn start_transaction(&self, description: &str) -> Transaction {
let mut tx = self.repo.start_transaction(description);
// TODO: Either do better shell-escaping here or store the values in some list
Expand Down Expand Up @@ -3042,7 +3051,7 @@ from the source will be moved into the destination.
short_commit_description(&source),
short_commit_description(&destination)
);
crate::diff_edit::edit_diff(ui, &parent_tree, &source_tree, &instructions)?
workspace_command.edit_diff(&parent_tree, &source_tree, &instructions)?
} else {
source_tree.id().clone()
};
Expand Down Expand Up @@ -3118,7 +3127,7 @@ from the source will be moved into the parent.
short_commit_description(parent)
);
new_parent_tree_id =
crate::diff_edit::edit_diff(ui, &parent.tree(), &commit.tree(), &instructions)?;
workspace_command.edit_diff(&parent.tree(), &commit.tree(), &instructions)?;
if &new_parent_tree_id == parent.tree().id() {
return Err(CommandError::UserError(String::from("No changes selected")));
}
Expand Down Expand Up @@ -3183,7 +3192,7 @@ aborted.
short_commit_description(&commit)
);
new_parent_tree_id =
crate::diff_edit::edit_diff(ui, &parent_base_tree, &parent.tree(), &instructions)?;
workspace_command.edit_diff(&parent_base_tree, &parent.tree(), &instructions)?;
if &new_parent_tree_id == parent_base_tree.id() {
return Err(CommandError::UserError(String::from("No changes selected")));
}
Expand Down Expand Up @@ -3245,7 +3254,7 @@ side. If you don't make any changes, then the operation will be aborted.
short_commit_description(&to_commit)
);
tree_id =
crate::diff_edit::edit_diff(ui, &from_commit.tree(), &to_commit.tree(), &instructions)?;
workspace_command.edit_diff(&from_commit.tree(), &to_commit.tree(), &instructions)?;
} else if args.is_present("paths") {
let matcher = matcher_from_values(
ui,
Expand Down Expand Up @@ -3304,7 +3313,7 @@ Adjust the right side until it shows the contents you want. If you
don't make any changes, then the operation will be aborted.",
short_commit_description(&commit)
);
let tree_id = crate::diff_edit::edit_diff(ui, &base_tree, &commit.tree(), &instructions)?;
let tree_id = workspace_command.edit_diff(&base_tree, &commit.tree(), &instructions)?;
if &tree_id == commit.tree().id() {
ui.write("Nothing changed.\n")?;
} else {
Expand Down Expand Up @@ -3344,7 +3353,7 @@ any changes, then the operation will be aborted.
",
short_commit_description(&commit)
);
let tree_id = crate::diff_edit::edit_diff(ui, &base_tree, &commit.tree(), &instructions)?;
let tree_id = workspace_command.edit_diff(&base_tree, &commit.tree(), &instructions)?;
if &tree_id == commit.tree().id() {
ui.write("Nothing changed.\n")?;
} else {
Expand Down
8 changes: 3 additions & 5 deletions src/diff_edit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,13 @@ use std::sync::Arc;
use jujutsu_lib::backend::{BackendError, TreeId};
use jujutsu_lib::matchers::EverythingMatcher;
use jujutsu_lib::repo_path::RepoPath;
use jujutsu_lib::settings::UserSettings;
use jujutsu_lib::store::Store;
use jujutsu_lib::tree::{merge_trees, Tree};
use jujutsu_lib::working_copy::{CheckoutError, TreeState};
use tempfile::tempdir;
use thiserror::Error;

use crate::ui::Ui;

#[derive(Debug, Error, PartialEq, Eq)]
pub enum DiffEditError {
#[error("The diff tool exited with a non-zero code")]
Expand Down Expand Up @@ -76,7 +75,7 @@ fn set_readonly_recursively(path: &Path) {
}

pub fn edit_diff(
ui: &mut Ui,
settings: &UserSettings,
left_tree: &Tree,
right_tree: &Tree,
instructions: &str,
Expand Down Expand Up @@ -130,8 +129,7 @@ pub fn edit_diff(

// TODO: Make this configuration have a table of possible editors and detect the
// best one here.
let editor_binary = ui
.settings()
let editor_binary = settings
.config()
.get_str("ui.diff-editor")
.unwrap_or_else(|_| "meld".to_string());
Expand Down

0 comments on commit 2291bff

Please sign in to comment.