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 71492d0
Show file tree
Hide file tree
Showing 3 changed files with 52 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
31 changes: 24 additions & 7 deletions cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1093,17 +1093,34 @@ 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).map_err(|mut e| {
if self.parse_file_path_expressions(values).is_ok() {
e.add_hint(
"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:\".",
);
}
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#"
Error: Failed to parse fileset: Syntax error
Caused by: --> 1:5
|
1 | abc~
| ^---
|
= expected `~` or <primary>
Hint: 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:".
"#);

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 71492d0

Please sign in to comment.