Skip to content

Commit

Permalink
cli: print conflicted paths whenever the working copy is changed
Browse files Browse the repository at this point in the history 
This is disabled when the global `--quiet` flag is used.
  • Loading branch information
@bnjmnt4n
bnjmnt4n committed Apr 3, 2024
1 parent 363b508 commit 579ecb7
Show file tree
Hide file tree
Showing 9 changed files with 145 additions and 113 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

* new function `working_copies()` for revsets to show the working copy commits of all workspaces.

* The list of conflicted paths is printed whenever the working copy changes.
This can be disabled with the `--quiet` option.

### Fixed bugs

## [0.15.1] - 2024-03-06
Expand Down
105 changes: 103 additions & 2 deletions cli/src/cli_util.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

use core::fmt;
use std::borrow::Cow;
use std::collections::{HashMap, HashSet};
use std::collections::{BTreeMap, HashMap, HashSet};
use std::env::{self, ArgsOs, VarError};
use std::ffi::OsString;
use std::fmt::Debug;
Expand All @@ -34,13 +34,14 @@ use clap::error::{ContextKind, ContextValue};
use clap::{ArgAction, ArgMatches, Command, FromArgMatches};
use indexmap::{IndexMap, IndexSet};
use itertools::Itertools;
use jj_lib::backend::{ChangeId, CommitId, MergedTreeId};
use jj_lib::backend::{ChangeId, CommitId, MergedTreeId, TreeValue};
use jj_lib::commit::Commit;
use jj_lib::git_backend::GitBackend;
use jj_lib::gitignore::{GitIgnoreError, GitIgnoreFile};
use jj_lib::hex_util::to_reverse_hex;
use jj_lib::id_prefix::IdPrefixContext;
use jj_lib::matchers::{EverythingMatcher, Matcher, PrefixMatcher};
use jj_lib::merge::MergedTreeValue;
use jj_lib::merged_tree::MergedTree;
use jj_lib::object_id::ObjectId;
use jj_lib::op_store::{OpStoreError, OperationId, WorkspaceId};
Expand Down Expand Up @@ -1127,6 +1128,15 @@ See https://github.com/martinvonz/jj/blob/main/docs/working-copy.md#stale-workin
if let Some(stats) = stats {
print_checkout_stats(ui, stats, new_commit)?;
}
if Some(new_commit) != maybe_old_commit {
if let Some(mut formatter) = ui.status_formatter() {
let conflicts = new_commit.tree()?.conflicts().collect_vec();
if !conflicts.is_empty() {
writeln!(formatter, "There are unresolved conflicts at these paths:")?;
print_conflicted_paths(&conflicts, formatter.as_mut(), self)?;
}
}
}
Ok(())
}

Expand Down Expand Up @@ -1557,6 +1567,97 @@ pub fn check_stale_working_copy(
}
}

#[instrument(skip_all)]
pub fn print_conflicted_paths(
conflicts: &[(RepoPathBuf, MergedTreeValue)],
formatter: &mut dyn Formatter,
workspace_command: &WorkspaceCommandHelper,
) -> Result<(), CommandError> {
let formatted_paths = conflicts
.iter()
.map(|(path, _conflict)| workspace_command.format_file_path(path))
.collect_vec();
let max_path_len = formatted_paths.iter().map(|p| p.len()).max().unwrap_or(0);
let formatted_paths = formatted_paths
.into_iter()
.map(|p| format!("{:width$}", p, width = max_path_len.min(32) + 3));

for ((_, conflict), formatted_path) in std::iter::zip(conflicts.iter(), formatted_paths) {
let sides = conflict.num_sides();
let n_adds = conflict.adds().flatten().count();
let deletions = sides - n_adds;

let mut seen_objects = BTreeMap::new(); // Sort for consistency and easier testing
if deletions > 0 {
seen_objects.insert(
format!(
// Starting with a number sorts this first
"{deletions} deletion{}",
if deletions > 1 { "s" } else { "" }
),
"normal", // Deletions don't interfere with `jj resolve` or diff display
);
}
// TODO: We might decide it's OK for `jj resolve` to ignore special files in the
// `removes` of a conflict (see e.g. https://github.com/martinvonz/jj/pull/978). In
// that case, `conflict.removes` should be removed below.
for term in itertools::chain(conflict.removes(), conflict.adds()).flatten() {
seen_objects.insert(
match term {
TreeValue::File {
executable: false, ..
} => continue,
TreeValue::File {
executable: true, ..
} => "an executable",
TreeValue::Symlink(_) => "a symlink",
TreeValue::Tree(_) => "a directory",
TreeValue::GitSubmodule(_) => "a git submodule",
TreeValue::Conflict(_) => "another conflict (you found a bug!)",
}
.to_string(),
"difficult",
);
}

write!(formatter, "{formatted_path} ")?;
formatter.with_label("conflict_description", |formatter| {
let print_pair = |formatter: &mut dyn Formatter, (text, label): &(String, &str)| {
write!(formatter.labeled(label), "{text}")
};
print_pair(
formatter,
&(
format!("{sides}-sided"),
if sides > 2 { "difficult" } else { "normal" },
),
)?;
write!(formatter, " conflict")?;

if !seen_objects.is_empty() {
write!(formatter, " including ")?;
let seen_objects = seen_objects.into_iter().collect_vec();
match &seen_objects[..] {
[] => unreachable!(),
[only] => print_pair(formatter, only)?,
[first, middle @ .., last] => {
print_pair(formatter, first)?;
for pair in middle {
write!(formatter, ", ")?;
print_pair(formatter, pair)?;
}
write!(formatter, " and ")?;
print_pair(formatter, last)?;
}
};
}
Ok(())
})?;
writeln!(formatter)?;
}
Ok(())
}

pub fn print_checkout_stats(
ui: &mut Ui,
stats: CheckoutStats,
Expand Down
121 changes: 15 additions & 106 deletions cli/src/commands/resolve.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,14 @@
// See the License for the specific language governing permissions and
// limitations under the License.

use std::collections::BTreeMap;
use std::io::Write;

use itertools::Itertools;
use jj_lib::backend::TreeValue;
use jj_lib::merge::MergedTreeValue;
use jj_lib::object_id::ObjectId;
use jj_lib::repo_path::RepoPathBuf;
use tracing::instrument;

use crate::cli_util::{CommandHelper, RevisionArg, WorkspaceCommandHelper};
use crate::cli_util::{print_conflicted_paths, CommandHelper, RevisionArg};
use crate::command_error::{cli_error, CommandError};
use crate::formatter::Formatter;
use crate::ui::Ui;

/// Resolve a conflicted file with an external merge tool
Expand Down Expand Up @@ -111,107 +106,21 @@ pub(crate) fn cmd_resolve(
format!("Resolve conflicts in commit {}", commit.id().hex()),
)?;

if let Some(mut formatter) = ui.status_formatter() {
let new_tree = new_commit.tree()?;
let new_conflicts = new_tree.conflicts().collect_vec();
if !new_conflicts.is_empty() {
writeln!(
formatter,
"After this operation, some files at this revision still have conflicts:"
)?;
print_conflicted_paths(&new_conflicts, formatter.as_mut(), &workspace_command)?;
}
}
Ok(())
}

#[instrument(skip_all)]
pub(crate) fn print_conflicted_paths(
conflicts: &[(RepoPathBuf, MergedTreeValue)],
formatter: &mut dyn Formatter,
workspace_command: &WorkspaceCommandHelper,
) -> Result<(), CommandError> {
let formatted_paths = conflicts
.iter()
.map(|(path, _conflict)| workspace_command.format_file_path(path))
.collect_vec();
let max_path_len = formatted_paths.iter().map(|p| p.len()).max().unwrap_or(0);
let formatted_paths = formatted_paths
.into_iter()
.map(|p| format!("{:width$}", p, width = max_path_len.min(32) + 3));

for ((_, conflict), formatted_path) in std::iter::zip(conflicts.iter(), formatted_paths) {
let sides = conflict.num_sides();
let n_adds = conflict.adds().flatten().count();
let deletions = sides - n_adds;

let mut seen_objects = BTreeMap::new(); // Sort for consistency and easier testing
if deletions > 0 {
seen_objects.insert(
format!(
// Starting with a number sorts this first
"{deletions} deletion{}",
if deletions > 1 { "s" } else { "" }
),
"normal", // Deletions don't interfere with `jj resolve` or diff display
);
}
// TODO: We might decide it's OK for `jj resolve` to ignore special files in the
// `removes` of a conflict (see e.g. https://github.com/martinvonz/jj/pull/978). In
// that case, `conflict.removes` should be removed below.
for term in itertools::chain(conflict.removes(), conflict.adds()).flatten() {
seen_objects.insert(
match term {
TreeValue::File {
executable: false, ..
} => continue,
TreeValue::File {
executable: true, ..
} => "an executable",
TreeValue::Symlink(_) => "a symlink",
TreeValue::Tree(_) => "a directory",
TreeValue::GitSubmodule(_) => "a git submodule",
TreeValue::Conflict(_) => "another conflict (you found a bug!)",
}
.to_string(),
"difficult",
);
}

write!(formatter, "{formatted_path} ",)?;
formatter.with_label("conflict_description", |formatter| {
let print_pair = |formatter: &mut dyn Formatter, (text, label): &(String, &str)| {
write!(formatter.labeled(label), "{text}")
};
print_pair(
formatter,
&(
format!("{sides}-sided"),
if sides > 2 { "difficult" } else { "normal" },
),
)?;
write!(formatter, " conflict")?;

if !seen_objects.is_empty() {
write!(formatter, " including ")?;
let seen_objects = seen_objects.into_iter().collect_vec();
match &seen_objects[..] {
[] => unreachable!(),
[only] => print_pair(formatter, only)?,
[first, middle @ .., last] => {
print_pair(formatter, first)?;
for pair in middle {
write!(formatter, ", ")?;
print_pair(formatter, pair)?;
}
write!(formatter, " and ")?;
print_pair(formatter, last)?;
}
};
// Print conflicts that are still present after resolution if the workspace
// working copy is not at the commit. Otherwise, the conflicting paths will
// be printed by the `tx.finish()` instead.
if workspace_command.get_wc_commit_id() != Some(new_commit.id()) {
if let Some(mut formatter) = ui.status_formatter() {
let new_tree = new_commit.tree()?;
let new_conflicts = new_tree.conflicts().collect_vec();
if !new_conflicts.is_empty() {
writeln!(
formatter,
"After this operation, some files at this revision still have conflicts:"
)?;
print_conflicted_paths(&new_conflicts, formatter.as_mut(), &workspace_command)?;
}
Ok(())
})?;
writeln!(formatter)?;
}
}
Ok(())
}
5 changes: 2 additions & 3 deletions cli/src/commands/status.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ use jj_lib::repo::Repo;
use jj_lib::rewrite::merge_commit_trees;
use tracing::instrument;

use super::resolve;
use crate::cli_util::CommandHelper;
use crate::cli_util::{print_conflicted_paths, CommandHelper};
use crate::command_error::CommandError;
use crate::diff_util;
use crate::ui::Ui;
Expand Down Expand Up @@ -72,7 +71,7 @@ pub(crate) fn cmd_status(
formatter.labeled("conflict"),
"There are unresolved conflicts at these paths:"
)?;
resolve::print_conflicted_paths(&conflicts, formatter, &workspace_command)?
print_conflicted_paths(&conflicts, formatter, &workspace_command)?
}

let template = workspace_command.commit_summary_template();
Expand Down
2 changes: 2 additions & 0 deletions cli/tests/test_chmod_command.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,8 @@ fn test_chmod_file_dir_deletion_conflicts() {
Parent commit : zsuskuln c51c9c55 file | file
Parent commit : royxmykx 6b18b3c1 deletion | deletion
Added 0 files, modified 1 files, removed 0 files
There are unresolved conflicts at these paths:
file 2-sided conflict including 1 deletion and an executable
"###);
let stdout = test_env.jj_cmd_success(&repo_path, &["debug", "tree", "-r=file_deletion"]);
insta::assert_snapshot!(stdout,
Expand Down
2 changes: 2 additions & 0 deletions cli/tests/test_diffedit_command.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,8 @@ fn test_diffedit_merge() {
Working copy now at: yqosqzyt 1de824f2 (conflict) (empty) (no description set)
Parent commit : royxmykx b90654a0 (conflict) merge
Added 0 files, modified 0 files, removed 1 files
There are unresolved conflicts at these paths:
file2 2-sided conflict
"###);
let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "-s", "-r", "@-"]);
insta::assert_snapshot!(stdout, @r###"
Expand Down
10 changes: 10 additions & 0 deletions cli/tests/test_repo_change_report.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ fn test_report_conflicts() {
Working copy now at: zsuskuln 7dc9bf15 (conflict) (empty) (no description set)
Parent commit : kkmpptxz 9baab11e (conflict) C
Added 0 files, modified 1 files, removed 0 files
There are unresolved conflicts at these paths:
file 2-sided conflict including 1 deletion
"###);

let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["rebase", "-d=description(A)"]);
Expand Down Expand Up @@ -75,6 +77,8 @@ fn test_report_conflicts() {
Working copy now at: zsuskuln 83074dac (conflict) (empty) (no description set)
Parent commit : kkmpptxz 4f0eeaa6 (conflict) C
Added 0 files, modified 1 files, removed 0 files
There are unresolved conflicts at these paths:
file 2-sided conflict
"###);

// Resolve one of the conflicts by (mostly) following the instructions
Expand All @@ -84,6 +88,8 @@ fn test_report_conflicts() {
Working copy now at: vruxwmqv 2ec0b4c3 (conflict) (empty) (no description set)
Parent commit : rlvkpnrz e93270ab (conflict) B
Added 0 files, modified 1 files, removed 0 files
There are unresolved conflicts at these paths:
file 2-sided conflict including 1 deletion
"###);
std::fs::write(repo_path.join("file"), "resolved\n").unwrap();
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["squash"]);
Expand Down Expand Up @@ -129,6 +135,8 @@ fn test_report_conflicts_with_divergent_commits() {
Working copy now at: zsuskuln?? cdae4322 (conflict) C2
Parent commit : kkmpptxz b76d6a88 (conflict) B
Added 0 files, modified 1 files, removed 0 files
There are unresolved conflicts at these paths:
file 2-sided conflict including 1 deletion
"###);

let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["rebase", "-d=description(A)"]);
Expand Down Expand Up @@ -160,6 +168,8 @@ fn test_report_conflicts_with_divergent_commits() {
Working copy now at: zsuskuln?? 33752e7e (conflict) C2
Parent commit : zzzzzzzz 00000000 (empty) (no description set)
Added 0 files, modified 1 files, removed 0 files
There are unresolved conflicts at these paths:
file 2-sided conflict including 1 deletion
"###);

let (stdout, stderr) =
Expand Down
4 changes: 2 additions & 2 deletions cli/tests/test_resolve_command.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ fn test_resolution() {
Parent commit : zsuskuln aa493daf a | a
Parent commit : royxmykx db6a4daf b | b
Added 0 files, modified 1 files, removed 0 files
After this operation, some files at this revision still have conflicts:
There are unresolved conflicts at these paths:
file 2-sided conflict
"###);
insta::assert_snapshot!(
Expand Down Expand Up @@ -704,7 +704,7 @@ fn test_multiple_conflicts() {
Parent commit : zsuskuln de7553ef a | a
Parent commit : royxmykx f68bc2f0 b | b
Added 0 files, modified 1 files, removed 0 files
After this operation, some files at this revision still have conflicts:
There are unresolved conflicts at these paths:
this_file_has_a_very_long_name_to_test_padding 2-sided conflict
"###);
insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff"]),
Expand Down
Loading

0 comments on commit 579ecb7

Please sign in to comment.