Changed basename extraction regex to support Windows paths. #816
Conversation
There was a problem hiding this comment.
@johnkord thanks a lot for taking the time to address this issues. 💐 I'm more than happy to merge your fix once you've addressed the minor comment regarding the used regular expression.
src/js/stores/MesosStore.js
Outdated
| (Windows) path. The basename will be found in index 0 if it is | ||
| a forward slash path. A backslash path will also match in index 0, | ||
| but it will not contain the basename. */ | ||
| var basename_match = /[a-zA-Z]:.*\\([^\\]+\\?$)|[^\/]+\/?$/.exec( |
There was a problem hiding this comment.
The pattern and condition looks unnecessary complex. The following pattern should be sufficient to also extract the basename from Windows path.
/[^/\\]+[/\\]{0,2}$/Side Note: I wonder why decided to include ending slashes. It might makes sense to add a capture group to only get the name. /([^/\\]+)[/\\]{0,2}$/.exec(file.path)[1]
|
Thanks, great advice! I've submitted the requested change and have tested and verified the associated tests pass and the stderr/stdout links work with both Linux and Windows agents. @orlandohohmeier |
|
Pinging @orlandohohmeier to let you know that this is ready for re-review, when you get a chance :) |
|
LGTM 💯 ✨ |
No description provided.