Skip to content

Handle normalized absolute file paths on markdown preview link click #116649

Merged
mjbvz merged 2 commits into
microsoft:mainfrom
habibkarim:habibkarim/115812_markdown_links_no_longer_work
Feb 19, 2021
Merged

Handle normalized absolute file paths on markdown preview link click #116649
mjbvz merged 2 commits into
microsoft:mainfrom
habibkarim:habibkarim/115812_markdown_links_no_longer_work

Conversation

@habibkarim

Copy link
Copy Markdown
Contributor

This PR fixes #115812

hrefPath = path.join(path.dirname(this.resource.fsPath), hrefPath);
} else {
// Handle any normalized file paths
hrefPath = hrefPath.replace('file///', '');

@habibkarim habibkarim Feb 14, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After investigation, I noticed that this issue was caused inadvertently by https://github.com/microsoft/vscode/pull/114553/files where the links were normalised to display images. This fix handles both absolute links and file:/// links

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On windows, I think the file path will look like file://c/path/to/file. Try parsing the file path using vscode.Uri.parse and then getting the .fsPath from it instead. I think that should be safer but haven't tested

@habibkarim habibkarim Feb 18, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion. I've made the change The latest update now handles file:/ links as well. I've tested this change both on Windows and Mac for multiple paths.

Windows:
Windows

Mac:
Mac

@joaomoreno joaomoreno changed the base branch from master to main February 15, 2021 08:51
hrefPath = path.join(path.dirname(this.resource.fsPath), hrefPath);
} else {
// Handle any normalized file paths
hrefPath = hrefPath.replace('file///', '');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On windows, I think the file path will look like file://c/path/to/file. Try parsing the file path using vscode.Uri.parse and then getting the .fsPath from it instead. I think that should be safer but haven't tested

@mjbvz mjbvz merged commit 9f08368 into microsoft:main Feb 19, 2021
@mjbvz

mjbvz commented Feb 19, 2021

Copy link
Copy Markdown
Contributor

Thanks! Will be in the next insiders build and is scheduled for VS Code 1.54

@mjbvz mjbvz added this to the February 2021 milestone Feb 19, 2021
@habibkarim habibkarim deleted the habibkarim/115812_markdown_links_no_longer_work branch February 19, 2021 17:33
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 5, 2021
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.

links in Markdown preview no longer work

2 participants