Skip to content

Treat a leading "!" in a bracket expression as negation by default - #188

Open
MildlyMeticulous wants to merge 1 commit into
micromatch:masterfrom
MildlyMeticulous:fix/bracket-negation-default
Open

Treat a leading "!" in a bracket expression as negation by default#188
MildlyMeticulous wants to merge 1 commit into
micromatch:masterfrom
MildlyMeticulous:fix/bracket-negation-default

Conversation

@MildlyMeticulous

Copy link
Copy Markdown

Fixes #187.

The [! to [^ conversion in lib/parse.js was gated on options.posix, so by default ! was emitted as an ordinary class member and [!abc] matched exactly the characters it is meant to exclude:

pm.isMatch('a', '[!abc]')  // was true
pm.isMatch('d', '[!abc]')  // was false
-      if (opts.posix === true && value === '!' && prev.value === '[') {
+      if (value === '!' && prev.value === '[') {

Three reasons the gate looks wrong rather than deliberate:

  • [!...] is Bash pattern negation, not a POSIX bracket expression like [[:alpha:]]
  • [[:alpha:]] already works with posix off, so the flag was not gating the feature it is named for
  • [^...] negation was already unconditional, so the two negation forms disagreed with each other by default

makeRe('[!abc]') now produces ^(?:[^abc/]\/?)$, the same shape as [^abc].

Compatibility

This changes default behaviour, so I measured it rather than assuming.

Existing suite: 1977 passing before, 1977 passing after, with no test edited. Nothing pinned the old behaviour; every existing [! test is in posix-classes.js and already runs with posix: true.

Against minimatch, over a 462-case matrix of 21 bracket patterns by 22 inputs:

agree disagree
before 376 86
after 441 21

65 disagreements fixed, none introduced. The 21 that remain are pre-existing picomatch behaviours unrelated to this change: leading-dot handling and / inside classes. Both also apply to [^...], which this does not touch, and they are unchanged by the patch.

Tests

Six cases added to test/brackets.js: plain negation, ranges, parity with [^...], negation inside a larger pattern, ! as a literal when not leading ([a!]), and a negated literal ! ([!!]). Reverting lib/parse.js and keeping the tests fails five of them.

The conversion of "[!" to "[^" was gated on options.posix, so with the
default options "[!abc]" emitted "!" as an ordinary class member and the
expression matched exactly the characters it should exclude.

"[!...]" is Bash pattern negation rather than a POSIX bracket expression,
and "[[:alpha:]]" already works without the option, so the flag was not
gating what its name suggests. "[^...]" negation was already unconditional,
so the two negation forms disagreed with each other by default.

Removing the gate leaves all 1977 existing tests passing and takes agreement
with minimatch on a 462-case bracket matrix from 376 to 441.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[!abc] matches a, b and c: POSIX-style bracket negation is inverted unless options.posix is set

1 participant