Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modify regex for scheme detection #7742

Merged
merged 1 commit into from May 16, 2019

Conversation

@jamesoliverh
Copy link

commented May 15, 2019

Modified regex expression for scheme detection from
if(baseString.matches(".*:/.*")){
to
if(baseString.matches(".{2,}:/.*")){
i.e. there must be at least 2 characters before the ":/".

This prevents the string
"C:/path/"
from being matched (contrary to before), whilst matching e.g.
"file:/C:/path" and "file:/path".

Additionally, with the proposed modification the string ":/path" which matched previously no longer matches (which I assume is desired behaviour).

Modify regex for scheme detection
Modified regex expression for scheme detection from
`if(baseString.matches(".*:/.*")){`
to
`if(baseString.matches(".{2,}:/.*")){`
i.e. there must be at least 2 characters before the ":/".

This prevents the string
"C:/path/" 
from being matched (contrary to before), whilst matching e.g.
"file:/C:/path" and "file:/path".

Additionally, with the proposed modification the string ":/path" which matched previously no longer matches (which I assume is desired behaviour).
@jamesoliverh

This comment has been minimized.

Copy link
Author

commented May 15, 2019

@AlexDBlack Any pointers on why some stuff might be failing in the continuous integration tests? Apologies, I am quite new to this.

@AlexDBlack
Copy link
Contributor

left a comment

Thanks for the PR. Don't worry about CI failure, there's a few things we're in the process of fixing there.
I've reviewed and tested this locally, LGTM.

@AlexDBlack AlexDBlack merged commit 61b4a4d into eclipse:master May 16, 2019

1 of 2 checks passed

continuous-integration/jenkins/pr-head This commit cannot be built
Details
Codacy/PR Quality Review Up to standards. A positive pull request.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.