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

File hyperlinks are broken in the R Console on Windows #2413

Closed
jennybc opened this issue Mar 8, 2024 · 8 comments
Closed

File hyperlinks are broken in the R Console on Windows #2413

jennybc opened this issue Mar 8, 2024 · 8 comments
Assignees
Labels
area: console Issues related to Console category. bug Something isn't working lang: r os-windows Windows issue

Comments

@jennybc
Copy link
Member

jennybc commented Mar 8, 2024

Here I have clicked on the file-hyperlink contained in this line:

• Global (user-level) gitignore file: D:/Users/jenny/.gitignore_global

Interestingly, copy and paste actually yields the link:

• Global (user-level) gitignore file: [D:/Users/jenny/.gitignore_global](vscode-file://vscode-app/d:/Users/jenny/source/repos/positron/out/vs/code/electron-sandbox/workbench/workbench-dev.html#)

leading to this error (also seen in screenshot):

The editor could not be opened due to an unexpected error: Unable to read file '\\D:\Users\jenny\.gitignore_global' (Unknown (FileSystemError): Error: UNKNOWN: unknown error, stat '\\D:\Users\jenny\.gitignore_global')
Screenshot 2024-03-08 at 10 14 37 AM

I have seen this (and actually discovered it) with several other file links (and emanating from other packages, such as testthat), so it's not specific to this file or to usethis.

@jennybc jennybc added bug Something isn't working os-windows Windows issue area: console Issues related to Console category. lang: r labels Mar 8, 2024
@jennybc jennybc added this to the Public Beta 2024 Q2 milestone Mar 8, 2024
@jennybc

This comment was marked as off-topic.

@jennybc
Copy link
Member Author

jennybc commented Mar 8, 2024

Filehyperlinks with line/column information seem to be perceived as a directory.

I'm clicking on test-glue.R:314:3 in this testthat output in the R Console:

══ Testing test-glue.R ════════════════════════════════════════════════════════════════════════════════════════════════════════════════
[ FAIL 0 | WARN 1 | SKIP 0 | PASS 145 ]

── Warning (test-glue.R:314:3): glue always returns UTF-8 encoded strings regardless of input encodings ──
using locale code page other than 65001 ("UTF-8") may cause problems
Backtrace:
    ▆
 1. └─withr::with_locale(...) at test-glue.R:314:3
 2.   └─withr::local_locale(new)
 3.     └─withr:::set_locale(cats)
 4.       └─base::mapply(Sys.setlocale, names(cats), cats)
 5.         └─base (local) `<fn>`(dots[[1L]][[1L]], dots[[2L]][[1L]])
[ FAIL 0 | WARN 1 | SKIP 0 | PASS 145 ]

Here's what copy/paste reveals the hyperlink to be (I'm beginning to suspect there's no information in this?):

[test-glue.R:314:3](vscode-file://vscode-app/d:/Users/jenny/source/repos/positron/out/vs/code/electron-sandbox/workbench/workbench-dev.html#)

The error:

The file is not displayed in the text editor because it is a directory.
Screenshot 2024-03-08 at 10 34 51 AM

@jennybc
Copy link
Member Author

jennybc commented Mar 8, 2024

In my examples so far, the url returned by props.outputRun.hyperlink.url seems to start with file:// (2 slashes) instead of file:/// (3 slashes).

And, sometimes, but not always, there is also a problem with the (some of) the path separators, being \ instead of / (in the path part to the right of file://.

A crude fixup along those lines does make the links work again, although I doubt that is the correct long-term solution.

url = url.replace(/\\/g, '/').replace(/file:\/\/(?!\/)/g, 'file:///');

The test-glue.R:314:3 example from above exhibits both problems:

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

which works once fixed up to:

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

@gaborcsardi
Copy link

gaborcsardi commented Mar 12, 2024

This is how the file link looks on Windows currently:

> cli::format_inline("{.file DESCRIPTION}")
[1] "\033]8;;file://C:/Users/csard/works/cli/DESCRIPTION\a\033[34mDESCRIPTION\033[39m\033]8;;\a"

This looks correct to me, C:/Users... is the actual file, and there are three slashes, like on Unix, but the third one is preceded by the drive letter.

This works on RStudio and also in other tools, e.g. Windows Terminal.

Having three slashes before the drive letter, i.e. file:///C:/ seems to still work in Windows Terminal, but not any more in RStudio.

WRT backward slashes, yes, they need to be converted to forward slashes. This could be done by Positron, or by cli. I am not sure why cli is not doing it, I know I had resrvations about normalizePath(), but it seems to work for non-existing files, so I think we could do it in cli.

@jennybc
Copy link
Member Author

jennybc commented Mar 12, 2024

@gaborcsardi and I discussed this, so I'll give an update. Here's the TL;DR as I see it:

  • In Positron, embrace Postel's Law ("be conservative in what you send, be liberal in what you accept") and be able to cope with a file hyperlink like file://D:\\Users\\jenny\\source\\repos\\glue\\tests\\testthat\\test-glue.R. For example, Windows Terminal can work with these links, i.e. cli's current file hyperlinks work there. So some version of my fixup should stay, but I will probably relocate it.
  • Signal to cli that we want POSIX compatible ANSI hyperlinks (see below), which should soon deliver the most canonical file hyperlink, e.g. file:///D:/Users/jenny/source/repos/glue/tests/testthat/test-glue.R. We'll do this in the startup.R script where we already have cli-related setup.

Basically, we'll work the problem from both sides.

More context: any change to cli has to also be done in a way that doesn't break existing functionality in RStudio, which expects the links currently produced by cli.


cli will start normalizing path separators to be / instead of \. This will presumably be implemented unconditionally.


cli has, today, an environment variable R_CLI_HYPERLINK_MODE:

R_CLI_HYPERLINK_MODE

Set to posix to force generating POSIX compatible ANSI hyperlinks. If not set, then RStudio compatible links
are generated. This is a temporary crutch until RStudio handles POSIX hyperlinks correctly, and after that it
will be removed.

R_CLI_HYPERLINK_MODE was originally introduced to address the opening and closing escape sequence for hyperlinks, i.e. to allow opting in to a more modern approach (https://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda#the-escape-sequence).

We're going to expand on R_CLI_HYPERLINK_MODE to make some changes on the cli side:

  • R_CLI_HYPERLINK_MODE is going to gain an equivalent R option, so that it's easier to set in, say, our startup.R code.
  • In addition to controlling the opening and closing sequence, this will opt the consumer in to a strategy where the typical file hyperlink has 3 slashes instead of just 2, e.g. file:///c:/some/path.txt.

@jennybc
Copy link
Member Author

jennybc commented Mar 12, 2024

Recoding some of my research here, so I can close these tabs!

RFC 8089 The "file" URI Scheme:
https://datatracker.ietf.org/doc/html/rfc8089#appendix-E.2

Excerpt from Appendix B. Example URIs:

The syntax in Section 2 is intended to support file URIs that take
the following forms:

Local files:

o A traditional file URI for a local file with an empty authority.
This is the most common format in use today. For example:

  *  "file:///path/to/file"

Excerpt from Appendix E.2 DOS and Windows Drive Letters:

URIs of the form "file:///c:/path/to/file" are already supported by
the "path-absolute" rule.


Excerpt from Appendix E.4 Backslash as Separator:

Historically, some usages have copied entire file paths into the path
components of file URIs. Where DOS or Windows file paths were thus
copied, the resulting URI strings contained unencoded backslash "\"
characters, which are forbidden by both [RFC1738] and [RFC3986].

It may be possible to translate or update such an invalid file URI by
replacing all backslashes "\" with slashes "/"
if it can be
determined with reasonable certainty that the backslashes are
intended as path separators.

@kevinushey
Copy link
Contributor

In case it's relevant... in Edge and Chrome, if you try to navigate to a file like:

file://C:\Windows

it will automatically "repair" the URI for you, e.g. you instead get:

file:///C:/Windows/

with three leading slashes, and backslashes translated into forward slashes. So there's already some existing precedence for this sort of file URI repair.

@jonvanausdeln jonvanausdeln self-assigned this Mar 13, 2024
@jonvanausdeln
Copy link
Contributor

Verified with 2024.03.0-1061
Windows 11

I tried several different style and all the links seem to be working.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: console Issues related to Console category. bug Something isn't working lang: r os-windows Windows issue
Projects
None yet
Development

No branches or pull requests

4 participants