Skip to content

Commit

Permalink
cli: warn when invalid fileset expressions look like file paths
Browse files Browse the repository at this point in the history
  • Loading branch information
samueltardieu committed Oct 2, 2024
1 parent f8e30fb commit a186e7d
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 7 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

* Color author and committer names yellow

* Commands accepting filesets suggest to use escaping if invalid fileset expressions
look like file paths.

### Fixed bugs

* Update working copy before reporting changes. This prevents errors during reporting
Expand Down
34 changes: 27 additions & 7 deletions cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1093,17 +1093,37 @@ impl WorkspaceCommandHelper {
if values.is_empty() {
Ok(FilesetExpression::all())
} else if self.settings().config().get_bool("ui.allow-filesets")? {
self.parse_union_filesets(ui, values)
self.parse_union_filesets(ui, values).or_else(|e| {
// If the arguments could not be parsed as valid filesets but could still be
// valid paths, warn the user about it.
if self.parse_file_path_expressions(values).is_ok() {
writeln!(
ui.warning_default(),
"Filesets are enabled by default. If you want to match file paths \
containing characters used in fileset expressions (':', or '~'), escape \
them, or prefix them with \"glob:\"."
)?;
}
Err(e)
})
} else {
let expressions = values
.iter()
.map(|v| self.parse_file_path(v))
.map_ok(FilesetExpression::prefix_path)
.try_collect()?;
Ok(FilesetExpression::union_all(expressions))
self.parse_file_path_expressions(values)
.map(FilesetExpression::union_all)
}
}

fn parse_file_path_expressions(
&self,
values: &[String],
) -> Result<Vec<FilesetExpression>, CommandError> {
let expressions = values
.iter()
.map(|v| self.parse_file_path(v))
.map_ok(FilesetExpression::prefix_path)
.try_collect()?;
Ok(expressions)
}

/// Parses the given fileset expressions and concatenates them all.
pub fn parse_union_filesets(
&self,
Expand Down
25 changes: 25 additions & 0 deletions cli/tests/test_global_opts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,31 @@ fn test_bad_path() {
"###);
}

#[test]
fn test_invalid_filesets_looking_like_filepaths() {
let test_env = TestEnvironment::default();
test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
let repo_path = test_env.env_root().join("repo");

let stderr = test_env.jj_cmd_failure(&repo_path, &["file", "show", "abc~"]);
insta::assert_snapshot!(stderr, @r#"
Warning: Filesets are enabled by default. If you want to match file paths containing characters used in fileset expressions (':', or '~'), escape them, or prefix them with "glob:".
Error: Failed to parse fileset: Syntax error
Caused by: --> 1:5
|
1 | abc~
| ^---
|
= expected `~` or <primary>
"#);

test_env.add_config(r#"ui.allow-filesets=false"#);
let stderr = test_env.jj_cmd_failure(&repo_path, &["file", "show", "abc~"]);
insta::assert_snapshot!(stderr, @r#"
Error: No such path: abc~
"#);
}

#[test]
fn test_broken_repo_structure() {
let test_env = TestEnvironment::default();
Expand Down

0 comments on commit a186e7d

Please sign in to comment.