Add absolute option for match event - closes #292
#293
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
If there is anything else you'd like me to tackle as part of this, just let me know. |
This was referenced Sep 19, 2016
isaacs
reviewed
Sep 20, 2016
| @@ -273,6 +273,9 @@ the filesystem. | |||
| In the case of a symlink that cannot be resolved, the full absolute | |||
| path to the matched entry is returned (though it will usually be a | |||
| broken symlink) | |||
| * `absolute` Set to true to always receive absolute paths for the | |||
| `'match'` event. Will not pass absolute paths to the callback | |||
| or returned from `glob.sync`. | |||
There was a problem hiding this comment.
I think it's a little bit surprising for the match events to be absolute, but for the results to still be relative. I'll check out the code to see how hard this is, probably I can at least point you in the right direction.
| @@ -80,6 +80,7 @@ function setopts (self, pattern, options) { | |||
| self.nocase = !!options.nocase | |||
| self.stat = !!options.stat | |||
| self.noprocess = !!options.noprocess | |||
| self._absolute = !!options.absolute | |||
There was a problem hiding this comment.
In my suggestion, I'd forgotten the pattern that this module uses. Probably it should be self.absolute, to match the other settings.
isaacs
added a commit
that referenced
this pull request
Sep 20, 2016
This involes a refactor of the GlobSync._emitMatch method, which it turns out, was not being used at all. Close #293
|
Published as 7.1.0. Thanks! |
|
Awesome! Glad I could help find issues in |
iarna
added a commit
to npm/npm
that referenced
this pull request
Oct 7, 2016
Add absolute option for match event. Credit: @phated PR-URL: isaacs/node-glob#293
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@isaacs Implemented as per guidance on #292 - I added a note about the caveat that this is only applied to
'match'events and not to the final results in the callback. I am not familiar enough with the code to understand the scope of making the final results absolute. Do you think it's a worth including in this feature?