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

Deal with less common file hyperlinks on Windows #2416

Merged
merged 2 commits into from
Mar 12, 2024
Merged

Conversation

jennybc
Copy link
Member

@jennybc jennybc commented Mar 8, 2024

Addresses #2413

I give more context in #2413 but the TL;DR is that Positron does need to make a special allowance for the current file hyperlinks produced by the cli R package on Windows.

There is a path towards having cli emit more conventional hyperlinks, which Positron will opt in to once that's possible. Changes in the cli package are constrained by the need to also not break file hyperlinks on Windows in RStudio.

@@ -56,6 +57,11 @@ export const OutputRun = (props: OutputRunProps) => {
return url;
}

// a hack that, at least, pinpoints the problem / solution
if (platform.isWindows) {
url = url.replace(/\\/g, '/').replace(/file:\/\/(?!\/)/g, 'file:///');
Copy link
Member Author

Choose a reason for hiding this comment

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

Example of a broken before link and a working after link:

file://D:\\Users\\jenny\\source\\repos\\glue\\tests\\testthat\\test-glue.R
file:///D:/Users/jenny/source/repos/glue/tests/testthat/test-glue.R

Copy link
Contributor

@softwarenerd softwarenerd left a comment

Choose a reason for hiding this comment

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

I'm approving this, but we are going to continue the investigation and make fixes elsewhere one day.

@jennybc jennybc changed the title Hack that fixes file hyperlinks Deal with less common file hyperlinks on Windows Mar 12, 2024
@jennybc jennybc marked this pull request as ready for review March 12, 2024 21:20
Copy link
Contributor

@softwarenerd softwarenerd left a comment

Choose a reason for hiding this comment

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

Again, I like that this takes us from "broken" to "working" until such time as we can get everything fixed in the right layers (in this case, Positron informing R to emit different file URLs).

@jennybc jennybc merged commit 8a7ff31 into main Mar 12, 2024
1 check passed
@jennybc jennybc deleted the file-hyperlink-fixup branch March 12, 2024 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants