Fix specs on Windows...again #99
Conversation
| if options.detail? and packageName = getPackageName(options.detail) | ||
| return packageName | ||
| if options.detail? | ||
| file = /(\((.+?):\d+|\((.+)\)|(.+))/.exec(options.detail)[1] |
There was a problem hiding this comment.
Should this stuff go into getPackageName?
There was a problem hiding this comment.
Do you mean the filePath version (L185)?
There was a problem hiding this comment.
It just seems like there is filtering being done before it hits the getPackageName method. Should that filtering logic go in the getPackageName method?
| return packageName | ||
| if options.detail? | ||
| packageName = getPackageName(options.detail) | ||
| return packageName if packageName? |
There was a problem hiding this comment.
Any reason for this change? cc @maxbrunsfeld style 👀?
There was a problem hiding this comment.
Oops, I forgot to fully revert it after I moved the regex out of this if. Fixing now...
|
Seems good to me. I defer to @maxbrunsfeld |
| @@ -183,6 +183,12 @@ class NotificationIssue | |||
| packagePaths[packageName] = fs.realpathSync(packagePath) | |||
|
|
|||
| getPackageName = (filePath) => | |||
| filePath = /(\((.+?):\d+|\((.+)\)|(.+))/.exec(filePath)[1] | |||
There was a problem hiding this comment.
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 item 0?
|
🚢 |
Specs were passing locally when I created #94...I swear! :P
Anyway, it seems like stack traces are sometimes returning file URIs for me which totally broke package name parsing. That has been fixed.
/cc @benogle
This also might help with #66.