Skip to content

Commit

Permalink
cli: warn explicit paths not exist in either of diff trees
Browse files Browse the repository at this point in the history
Maybe we can optimize it to check paths during diffing, but I think it's okay
to add extra lookup cost at the end. The size of the path arguments is usually
small.

Closes #505
  • Loading branch information
yuja committed Apr 9, 2024
1 parent 44ca676 commit ec45ed6
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 3 deletions.
20 changes: 17 additions & 3 deletions cli/src/commands/diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

use itertools::Itertools as _;
use jj_lib::rewrite::merge_commit_trees;
use tracing::instrument;

Expand Down Expand Up @@ -76,9 +77,8 @@ pub(crate) fn cmd_diff(
from_tree = merge_commit_trees(workspace_command.repo().as_ref(), &parents)?;
to_tree = commit.tree()?
}
let matcher = workspace_command
.parse_file_patterns(&args.paths)?
.to_matcher();
let fileset_expression = workspace_command.parse_file_patterns(&args.paths)?;
let matcher = fileset_expression.to_matcher();
let diff_formats = diff_formats_for(command.settings(), &args.format)?;
ui.request_pager();
show_diff(
Expand All @@ -90,5 +90,19 @@ pub(crate) fn cmd_diff(
matcher.as_ref(),
&diff_formats,
)?;

let unmatched_explicit_paths = fileset_expression
.explicit_paths()
.filter(|path| from_tree.path_value(path).is_absent())
.filter(|path| to_tree.path_value(path).is_absent())
.map(|path| workspace_command.format_file_path(path))
.collect_vec();
if !unmatched_explicit_paths.is_empty() {
writeln!(
ui.warning_default(),
"No matching entries for paths: {}",
unmatched_explicit_paths.join(", ")
)?;
}
Ok(())
}
27 changes: 27 additions & 0 deletions cli/tests/test_diff_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,33 @@ fn test_diff_basic() {
file3 | 1 +
3 files changed, 2 insertions(+), 1 deletion(-)
"###);

// Unmatched paths should be warned
let (stdout, stderr) = test_env.jj_cmd_ok(
test_env.env_root(),
&[
"diff",
"-Rrepo",
"-s",
"repo", // matches directory
"repo/file1", // deleted in to_tree, but exists in from_tree
"repo/x",
"repo/y/z",
],
);
insta::assert_snapshot!(stdout.replace('\\', "/"), @r###"
D repo/file1
M repo/file2
A repo/file3
"###);
insta::assert_snapshot!(stderr.replace('\\', "/"), @r###"
Warning: No matching entries for paths: repo/x, repo/y/z
"###);

// Unmodified paths shouldn't be warned
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["diff", "-s", "--from=@", "file2"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @"");
}

#[test]
Expand Down

0 comments on commit ec45ed6

Please sign in to comment.