Skip to content

Commit

Permalink
cli: migrate "chmod" to matcher API, warn unmatched paths
Browse files Browse the repository at this point in the history
  • Loading branch information
yuja committed Apr 10, 2024
1 parent ae70db8 commit 1bfacea
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 34 deletions.
22 changes: 8 additions & 14 deletions cli/src/commands/chmod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,12 @@
// See the License for the specific language governing permissions and
// limitations under the License.

use itertools::Itertools;
use jj_lib::backend::TreeValue;
use jj_lib::merged_tree::MergedTreeBuilder;
use jj_lib::object_id::ObjectId;
use tracing::instrument;

use crate::cli_util::{CommandHelper, RevisionArg};
use crate::cli_util::{print_unmatched_explicit_paths, CommandHelper, RevisionArg};
use crate::command_error::{user_error, CommandError};
use crate::ui::Ui;

Expand Down Expand Up @@ -60,30 +59,25 @@ pub(crate) fn cmd_chmod(
};

let mut workspace_command = command.workspace_helper(ui)?;
// TODO: migrate to .parse_file_patterns()?.to_matcher()
let repo_paths: Vec<_> = args
.paths
.iter()
.map(|path| workspace_command.parse_file_path(path))
.try_collect()?;
let commit = workspace_command.resolve_single_rev(&args.revision)?;
workspace_command.check_rewritable([commit.id()])?;
let tree = commit.tree()?;
// TODO: No need to add special case for empty paths when switching to
// parse_union_filesets(). paths = [] should be "none()" if supported.
let fileset_expression = workspace_command.parse_file_patterns(&args.paths)?;
let matcher = fileset_expression.to_matcher();
print_unmatched_explicit_paths(ui, &workspace_command, &fileset_expression, [&tree])?;

let mut tx = workspace_command.start_transaction();
let tree = commit.tree()?;
let store = tree.store();
let mut tree_builder = MergedTreeBuilder::new(commit.tree_id().clone());
for repo_path in repo_paths {
for (repo_path, tree_value) in tree.entries_matching(matcher.as_ref()) {
let user_error_with_path = |msg: &str| {
user_error(format!(
"{msg} at '{}'.",
tx.base_workspace_helper().format_file_path(&repo_path)
))
};
let tree_value = tree.path_value(&repo_path);
if tree_value.is_absent() {
return Err(user_error_with_path("No such path"));
}
let all_files = tree_value
.adds()
.flatten()
Expand Down
29 changes: 9 additions & 20 deletions cli/tests/test_chmod_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,27 +116,16 @@ fn test_chmod_regular_conflict() {
>>>>>>>
"###);

// An error prevents `chmod` from making any changes.
// In this case, the failure with `nonexistent` prevents any changes to `file`.
let stderr = test_env.jj_cmd_failure(&repo_path, &["chmod", "x", "nonexistent", "file"]);
// Unmatched paths should generate warnings
let (_stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["chmod", "x", "nonexistent", "file"]);
insta::assert_snapshot!(stderr, @r###"
Error: No such path at 'nonexistent'.
"###);
let stdout = test_env.jj_cmd_success(&repo_path, &["debug", "tree"]);
insta::assert_snapshot!(stdout,
@r###"
file: Conflicted([Some(File { id: FileId("587be6b4c3f93f93c489c0111bba5596147a26cb"), executable: false }), Some(File { id: FileId("df967b96a579e45a18b8251732d16804b2e56a55"), executable: false }), Some(File { id: FileId("8ba3a16384aacc37d01564b28401755ce8053f51"), executable: false })])
"###);
let stdout = test_env.jj_cmd_success(&repo_path, &["cat", "file"]);
insta::assert_snapshot!(stdout,
@r###"
<<<<<<<
%%%%%%%
-base
+x
+++++++
n
>>>>>>>
Warning: No matching entries for paths: nonexistent
Working copy now at: yostqsxw cbc43289 conflict | (conflict) conflict
Parent commit : royxmykx 427fbd2f x | x
Parent commit : zsuskuln 3f83a26d n | n
Added 0 files, modified 1 files, removed 0 files
There are unresolved conflicts at these paths:
file 2-sided conflict including an executable
"###);
}

Expand Down

0 comments on commit 1bfacea

Please sign in to comment.