Add nanomatch.capture to return captured matches #4
Conversation
|
Tests don't appear to be passing on windows already... Not sure about that. If you like this PR, I'll go ahead and add the |
index.js
Outdated
| return null; | ||
| } | ||
|
|
||
| return match.slice(1).filter(Boolean); |
There was a problem hiding this comment.
.filter(Boolean) removes the empty captures/strings, right?
Doesn't that make this API hard to use?
For instance: given a/**/c/**/f.txt then both paths below result in the same capture ["x"]:
a/x/c/f.txta/c/x/f.txt
It's impossible to rebuild the matching path from the capture.
I you kept empty strings, then the first would capture ["x", ""] and the second ["", "x"]. The matching paths are very easy to rebuild.
There was a problem hiding this comment.
Good point. This made sense for my use case (described above), but I can see it being weird for other cases. I'll remove the filter here and move it to my code.
There was a problem hiding this comment.
Thanks for doing this! I like the simplicity of the code, nice work. Great all around PR.
Tests don't appear to be passing on windows already... Not sure about that.
Looks like one specific test if failing. I'm not sure why, but I'll look into it.
I couldn't get the documentation generator to run on my system, so if you could run it that would be great. Thanks for a great library!
No problem. I'll take care of it.
If you like this PR, I'll go ahead and add the capture method to micromatch as well, and update extglob with support for capture groups as well.
Yes, I do and that would be great! thanks!
index.js
Outdated
| * @api public | ||
| */ | ||
|
|
||
| nanomatch.capture = function(pattern, string, options) { |
There was a problem hiding this comment.
this is a little thing, but could you change string to str to be consistent with other methods?
index.js
Outdated
| var unixify = utils.unixify(options); | ||
|
|
||
| function match() { | ||
| return function(str) { |
There was a problem hiding this comment.
and change this to string (this is probably why you did the other as string. noted, and thanks for indulging my OCD)
|
@jonschlinkert updated! |
|
@devongovett great work, perfect PR all around! I sent you an invite to join the org. Thank you! |
|
I accidentally pushed this up as a patch, since I was focused on a bugfix and I forgot I merged this in first. However, there shouldn't be anything breaking, it's really straight forward, and I tested extensively with micromatch and all tests pass. |
|
@jonschlinkert awesome, thanks. I think it should be ok. I tested with micromatch as well, and added all of the nanomatch/extglob tests that I added to micromatch as well: micromatch/micromatch#105. |
This adds a new method
nanomatch.capture, which returns captured matches from dynamic parts of a pattern (e.g. star, globstar). Should partially address micromatch/micromatch#85.For example:
This is very useful for a number of purposes. My usecase is building a nested object with results of a glob. For example, given the path and pattern above:
It works by adding a
captureoption to the compiler, which causes capturing groups to be added to the generated regex for stars and globstars. This option is off by default, so everything continues working exactly the same for other methods.I couldn't get the documentation generator to run on my system, so if you could run it that would be great. Thanks for a great library!