-
-
Notifications
You must be signed in to change notification settings - Fork 276
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
Fix path check when folder name has spaces #836
Conversation
Sounds sensible to me! Do you mind adding an E2E test to cover the new behaviour? |
Sure thing. When debugging this, is there a way to attach vscode or something so you can set breakpoints and step through the code? I tracked this down using a lot of |
Sorry for the late response, I was off sick. I can imagine how painful debugging with console statements is. In JavaScript ther is keyword called |
or are you referring to E2E tests? |
I was trying to debug in the browser with devtools open. I forgot about |
@stefanbuck I adjusted this so it only applies to the .net file paths, otherwise it was breaking other areas like JS imports. |
I agree, it's best to limit this change to .net files to avoid side effects. |
@stefanbuck I found that the extensions https://github.com/glebm/render-whitespace-on-github and https://github.com/sindresorhus/refined-github both break linking for files/folders with spaces in them due to showing the whitespace (which is an inserted This is what our test file looks like. And the html with the visible whitespace. <tr>
<td id="L62" class="blob-num js-line-number" data-line-number="62"></td>
<td id="LC62" class="blob-code blob-code-inner js-file-line rgh-observing-whitespace highlighted"><span data-rgh-whitespace="····"> </span><<span class="pl-ent">Content</span><span data-rgh-whitespace="·"> </span><span class="pl-e">Include</span>=<span class="pl-s"><span class="pl-pds">"</span>../foo<span data-rgh-whitespace="·"> </span>bar/test<span data-rgh-whitespace="·"> </span>file.cs<span class="pl-pds">"</span></span><span data-rgh-whitespace="·"> </span>/></td>
</tr> |
For me this feels like an edge case. I rarely, almost never saw whitespaces in file and path names. Is this a .net thing? |
I noticed it in some older projects I've been working on. I don't think it's super common, but it's definitely possible and I've seen it many times over the years. I'm fine leaving this as-is. |
Do you mind creating an issue for this just in case we want to fix it at some point? |
While working on some more .net support I came across a file that wouldn't resolve. The folder it's in has a space in it and the
captureQuotedWord
regex excludes spaces so I removed that but it still wouldn't resolve.After digging into it more I found that this check was returning
false
because the file paths ingithubTree
have spaces while the value used to compare against contains%20
.I'm not sure if this is the correct spot to fix this, but it's a good way to see the issue.
While running this version of the code go to this url and add/remove the replace fix.
https://github.com/microsoft/uprove-csharp-sdk/blob/master/UProveUnitTest/UProveUnitTest.csproj#L78