-
-
Notifications
You must be signed in to change notification settings - Fork 57
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Do not use previous value for the slash token #52
Do not use previous value for the slash token #52
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Small nitpicks, would you mind making those changes?
Also, how did you find this bug? I appreciate you looking into it. What was the glob pattern you used for your example?
lib/parse.js
Outdated
@@ -839,6 +839,7 @@ const parse = (input, options) => { | |||
|
|||
state.output += prior.output + prev.output; | |||
consume(value + advance()); | |||
value = '/'; | |||
|
|||
push({ type: 'slash', value, output: '' }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch!
Instead of reassigning value
, let's move that to the token:
push({ type: 'slash', value: '/', output: '' });
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JFYI: I use reassigning value
only for local consistency within this module (in most cases we use { value }
without property value assigning).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing that out, and I appreciate you paying attention to those details!
5a7c2a2
to
c3c192d
Compare
c3c192d
to
5e4cd68
Compare
I'm trying to split pattern into segments. Now these are just experiments:
One of the test patterns in this repository:
|
Impressive! Thank you!
…Sent from my iPhone
On Dec 24, 2019, at 3:22 AM, Denis Malinochkin ***@***.***> wrote:
Also, how did you find this bug?
I'm trying to split pattern into segments. Now these are just experiments:
mrmlnc/fast-glob#156 (comment)
What was the glob pattern you used for your example?
One of the test patterns in this repository:
https://github.com/micromatch/picomatch/blob/606e35fb2037fc1c000f6528820fb4df28bf2dac/test/braces.js#L124
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
For some patterns the
.parse
method returns tokens with correct type but wrong value.I don't know where I must write tests for it :)