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

Markdown preview support the UNC path files. #26710

Merged
merged 1 commit into from
May 22, 2017
Merged

Markdown preview support the UNC path files. #26710

merged 1 commit into from
May 22, 2017

Conversation

tomoki1207
Copy link
Contributor

@tomoki1207 tomoki1207 commented May 16, 2017

Related to #26464.

  • VSCode Version: 1.12.2
  • OS Version: Windows 8.1 Pro

The built-in markdown extensions cannot preview the network files.
In my case, the network file path is UNC path(starts with\\).

This tiny changes affected to be able to preview even the UNC path files.

@mjbvz mjbvz self-assigned this May 16, 2017
@mjbvz mjbvz self-requested a review May 16, 2017 05:31
@mjbvz
Copy link
Collaborator

mjbvz commented May 16, 2017

I believe we need fsPath in this case because it normalize path on windows.

If the preview is not working for unc paths, first try debugging into provideTextDocumentContent in previewContentProvider.ts to see if the document is loading properly or not

@tomoki1207
Copy link
Contributor Author

tomoki1207 commented May 16, 2017

Why we need the normalize path? For the markdown extensions? Or for the Uri internal behavior?

provideTextDocumentContent is not called. Because Error occurred at URI._validate in process of getMarkdownUri before it.
The Error is caused by UTC path is regarded as wrong URI. So, this error has been raised on other TextDocument events too.

@mjbvz mjbvz added this to the May 2017 milestone May 22, 2017
@mjbvz mjbvz merged commit 3e65e25 into microsoft:master May 22, 2017
@mjbvz
Copy link
Collaborator

mjbvz commented May 22, 2017

Thanks @tomoki1207

We need to normalize the path for scroll synchronization. I believe this normalization already happens downstream in provideTextDocumentContent, so let's see if this change causes any regressions. It should be in the next insiders build

@detlefm
Copy link

detlefm commented May 23, 2017

Works fine on 1.13.0-insider.
Thank you.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants