-
Notifications
You must be signed in to change notification settings - Fork 36
Fix specs on Windows...again #99
Changes from 3 commits
6a0783d
c54ddcc
035ba16
22f69bd
5c5522c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -183,15 +183,22 @@ class NotificationIssue | |
packagePaths[packageName] = fs.realpathSync(packagePath) | ||
|
||
getPackageName = (filePath) => | ||
filePath = /(\((.+?):\d+|\((.+)\)|(.+))/.exec(filePath)[1] | ||
|
||
# Stack traces may be a file URI | ||
if /file:\/\/(\w*)\/(.*)/.test(filePath) | ||
filePath = /file:\/\/(\w*)\/(.*)/.exec(filePath)[2] | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like the same regex is created and executed twice here, and the first capture group is unused. Also, sometimes when regexps have multiple # somewhere at the top of the file
FileURLRegExp = new RegExp('file://\w*/(.*)')
# ...
if match = FileURLRegExp.exec(filePath)
filePath = match[1] |
||
if path.isAbsolute(filePath) | ||
for packName, packagePath of packagePaths | ||
continue if filePath is 'node.js' | ||
relativePath = path.relative(packagePath, filePath) | ||
return packName unless /^\.\./.test(relativePath) | ||
@getPackageNameFromFilePath(filePath) | ||
|
||
if options.detail? and packageName = getPackageName(options.detail) | ||
return packageName | ||
if options.detail? | ||
packageName = getPackageName(options.detail) | ||
return packageName if packageName? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any reason for this change? cc @maxbrunsfeld style 👀? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oops, I forgot to fully revert it after I moved the regex out of this if. Fixing now... |
||
|
||
if options.stack? | ||
stack = StackTraceParser.parse(options.stack) | ||
|
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.
I'm a little confused by the nesting of capture groups here. Does item
1
in the match array represent the entire match, because the entire thing is wrapped in parens? If so, maybe you could just remove the outer parens and use item0
?