fix: Error when trying to set a breakpoint in index.html#1029
Merged
connor4312 merged 2 commits intomicrosoft:mainfrom Jun 11, 2021
Merged
fix: Error when trying to set a breakpoint in index.html#1029connor4312 merged 2 commits intomicrosoft:mainfrom
connor4312 merged 2 commits intomicrosoft:mainfrom
Conversation
|
|
Contributor
Author
|
It looks like some tests are failing, so I've closed my pull request and will make another when the tests pass |
Two of the tests seemed to be incorrect, but were passing because of a previous bug, so this commit changes those two tests. Another test was failing because the previous commit incorrectly disabled conversion of the whole path to a case insensitive regex pattern on case insensitive platforms, so this commit restores that behavior while still correctly ignoring portions of the path that were already converted to a regex pattern.
Contributor
Author
|
Tests are passing now. Two of the test were failing incorrectly due to bugs in the tests that were hidden by the urlToRegex bug, so I fixed those. One of the tests was failing because I accidentally broke the case insensitive portion of the conversion, so that should be fixed now, too. |
Member
|
Good find! |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This is a fix for the urlToRegex issue referenced in #1028 along with 2 new test cases.
I'm making the assumption that any part of
aPaththat has already been escaped should also not have the url decode/encode step done, which seems to match the previous intended behavior. I am also making the assumption that there is no expectation at that point that the portion of the path that needs to be escaped will be a valid file url, especially if escapeReStart is not zero. This seems like a safe assumption given that the escape param only seems to be used from one place that works under those assumptions, but the behavior isn't super intuitive. I didn't want to change too much though, especially since I'm new to this codebase and don't know the history of that method.