Skip to content

Commit

Permalink
fileset: parse glob characters as identifier
Browse files Browse the repository at this point in the history
It's inconvenient that we have to quote glob patterns as 'glob:"*.rs"'. Suppose
filesets are usually specified in shell, it's better to allow unquoted strings
if possible. This change also means we'll probably abandon #2101 "make the
parsing of string arguments stricter."

Note that we can no longer introduce ? operator or [] subscript syntax in
filesets.

Closes #4053
  • Loading branch information
yuja committed Jul 17, 2024
1 parent f2bfb90 commit 3e5a2cd
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 5 deletions.
2 changes: 1 addition & 1 deletion cli/tests/test_diff_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ fn test_diff_basic() {
"diff",
"--config-toml=ui.allow-filesets=true",
"-s",
r#"glob:"file[12]""#,
"glob:file[12]",
],
);
insta::assert_snapshot!(stdout, @r###"
Expand Down
5 changes: 5 additions & 0 deletions docs/filesets.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ if the expression has no operators nor function calls. For example:
* `jj diff '~"Foo Bar"'` (both shell and inner quotes are required)
* `jj diff '"Foo(1)"'` (both shell and inner quotes are required)

Glob characters aren't considered meta characters, but shell quotes are still
required:

* `jj diff '~glob:**/*.rs'`

[string-literals]: templates.md#string-literals

## File patterns
Expand Down
7 changes: 3 additions & 4 deletions lib/src/fileset.pest
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,21 @@ whitespace = _{ " " | "\t" | "\r" | "\n" | "\x0c" }

// XID_CONTINUE: https://www.unicode.org/reports/tr31/#Default_Identifier_Syntax
// +, -, ., @, _: commonly used in file name including "." and ".."
// *, ?, [, ]: glob characters (not extended glob)
// /: path separator
// \: path separator (Windows)
// TODO: accept glob characters as identifier?
identifier = @{
(XID_CONTINUE | "+" | "-" | "." | "@" | "_" | "/" | "\\")+
(XID_CONTINUE | "+" | "-" | "." | "@" | "_" | "*" | "?" | "[" | "]" | "/" | "\\")+
}
strict_identifier_part = @{ (ASCII_ALPHANUMERIC | "_")+ }
strict_identifier = @{
strict_identifier_part ~ ("-" ~ strict_identifier_part)*
}

// TODO: accept glob characters?
// TODO: accept more ASCII meta characters such as "#" and ","?
bare_string = @{
( ASCII_ALPHANUMERIC
| " " | "+" | "-" | "." | "@" | "_" | "/" | "\\"
| " " | "+" | "-" | "." | "@" | "_" | "*" | "?" | "[" | "]" | "/" | "\\"
| '\u{80}'..'\u{10ffff}' )+
}

Expand Down
18 changes: 18 additions & 0 deletions lib/src/fileset_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,10 @@ mod tests {
parse_into_kind(r#"Windows\Path"#),
Ok(ExpressionKind::Identifier(r#"Windows\Path"#))
);
assert_eq!(
parse_into_kind("glob*[chars]?"),
Ok(ExpressionKind::Identifier("glob*[chars]?"))
);
}

#[test]
Expand Down Expand Up @@ -502,6 +506,13 @@ mod tests {
value: "bar".to_owned()
})
);
assert_eq!(
parse_into_kind(" foo:glob*[chars]? "),
Ok(ExpressionKind::StringPattern {
kind: "foo",
value: "glob*[chars]?".to_owned()
})
);
assert_eq!(
parse_into_kind(r#" foo:"bar" "#),
Ok(ExpressionKind::StringPattern {
Expand Down Expand Up @@ -645,6 +656,13 @@ mod tests {
value: " bar baz".to_owned()
})
);
assert_eq!(
parse_maybe_bare_into_kind("foo:glob * [chars]?"),
Ok(ExpressionKind::StringPattern {
kind: "foo",
value: "glob * [chars]?".to_owned()
})
);
assert_eq!(
parse_maybe_bare_into_kind("foo:bar:baz"),
Err(FilesetParseErrorKind::SyntaxError)
Expand Down

0 comments on commit 3e5a2cd

Please sign in to comment.