expr: Fix assorted test errors in tests/expr/expr.pl #6134
Conversation
sargas
force-pushed
the
expr-error-messages
branch
from
85f838b to
613ab2a
Compare
sylvestre
force-pushed
the
expr-error-messages
branch
from
f4d7a06 to
c2d17a1
Compare
|
GNU testsuite comparison: |
sargas
force-pushed
the
expr-error-messages
branch
2 times, most recently
from
419ee88 to
036e428
Compare
sargas
changed the title
|
GNU testsuite comparison: |
src/uu/expr/src/syntax_tree.rs
Outdated
| /// differently from the oniguruma library used by the regex crate. | ||
| /// This method attempts to do these checks manually in one pass | ||
| /// through the regular expression. | ||
| fn validate_regex(pattern: &str) -> ExprResult<()> { |
There was a problem hiding this comment.
there isn't any crate doing it already ?
There was a problem hiding this comment.
I don't think so? Regex (and related) crates don't use posix syntax. tests/expr/expr.pl does check that \{ and { are treated opposite of how they would be in the regex crate
There was a problem hiding this comment.
maybe rename this function to make it more specific ?
like
validate_regex_posix?
sargas
force-pushed
the
expr-error-messages
branch
from
764a06c to
6e38843
Compare
|
GNU testsuite comparison: |
sargas
force-pushed
the
expr-error-messages
branch
from
6e38843 to
7de320c
Compare
|
GNU testsuite comparison: |
sargas
force-pushed
the
expr-error-messages
branch
from
7de320c to
e166030
Compare
|
GNU testsuite comparison: |
sargas
force-pushed
the
expr-error-messages
branch
from
e166030 to
abaf93d
Compare
src/uu/expr/src/syntax_tree.rs
Outdated
| let mut repetition = repeating_pattern_text[..repeating_pattern_text.len() - 1] | ||
| .splitn(2, |x| x == ','); | ||
| match (repetition.next(), repetition.next()) { | ||
| (None, None) => { |
There was a problem hiding this comment.
i see 3 paths that the code coverage can't reach, is it possible to add tests to cover it? thanks
sargas
force-pushed
the
expr-error-messages
branch
from
abaf93d to
72ba10c
Compare
|
GNU testsuite comparison: |
sylvestre
force-pushed
the
expr-error-messages
branch
from
72ba10c to
6dfb9ac
Compare
|
GNU testsuite comparison: |
This situation now has two errors - one if we parse a character that isn't a closing parenthesis, and the other if we don't parse anything.
GNU expr appears to treat this as a literal asterisk
sargas
force-pushed
the
expr-error-messages
branch
from
6b7e932 to
8c27c7f
Compare
|
GNU testsuite comparison: |
|
crap, i forgot about this PR when I worked on this: github.com//pull/7119 |
|
@sylvestre hah, I should have checked if there was any expr PRs in progress before updating this too! There are at least two test failures that should be fixed with the version of Oniguruma released a few weeks ago FWIW |
|
@sargas no worries, I wrote enough code on this project :) |
This PR makes several changes to
expr:?operator on a call to.next()), this PR gives an error indicating that we were looking for a missing closing parenthesis. We also now say when a parenthesis should have been seen if that's not the end of the arguments.?or*), then the regex does match but there's no captured groups; causing a panic in the current logic. This PR changes the behavior to treat an empty match as the empty string.These changes fix many of the test failures in GNU's
tests/expr/expr.pl.There's other test failures due to the regular expressions not matching correctly, but the oniguruma library should have fixes for that in the next release[1][2].