-
Notifications
You must be signed in to change notification settings - Fork 70
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
Configurable file hyperlinks #744
base: main
Are you sure you want to change the base?
Conversation
@@ -1,6 +1,6 @@ | |||
Package: cli | |||
Title: Helpers for Developing Command Line Interfaces | |||
Version: 3.6.3.9001 | |||
Version: 3.6.3.9002 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to need to detect this version in Positron. We only want to enable hyperlinks in the relevant terminals if we know they're going work exactly as we want.
paste0(abs_path(params$path), params$suffix), | ||
params = params$params | ||
link$url, | ||
params = link$params | ||
) | ||
}) | ||
ret | ||
} | ||
|
||
parse_file_link_params <- function(txt) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This became narrower, i.e. just about parsing.
} else { | ||
list(path = path, params = c(line = num, col = "1")) | ||
} | ||
construct_file_link <- function(params) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This turns the parsed filepath into the data style_hyperlink()
needs.
list(path = path, params = c(line = num, col = "1")) | ||
} | ||
construct_file_link <- function(params) { | ||
fmt <- get_config_chr("hyperlink_file_url_format") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As it stands, only an explicitly configured hyperlink format has any effect.
But you could imagine detecting that we're in, e.g. a Positron or VS Code terminal, and setting this configuration from the cli side. Food for thought. But definitely something that could be added later.
I took inspiration from other tools, such as ripgrep. Here's how ripgrep can be configured to construct hyperlinks for various environments:
Documentation of the https://code.visualstudio.com/docs/editor/command-line#_opening-vs-code-with-urls delta is another tool that has a similar configuration point: https://dandavison.github.io/delta/tips-and-tricks/using-delta-with-vscode.html |
In actual use this is looking pretty good now. For my own convenience, I want to record how to test drive this even without the necessary changes in Positron. In a terminal inside VS Code, set up some environment variables and then start R:
In that R session, you should be able to create cli file hyperlinks and Ctrl/Cmd+click on them to open in VS Code, e.g.: cli::cli_text("{.file ~/.gitconfig}") Likewise, you could do some in a terminal inside a released version of Positron but changing the file hyperlink format:
I'll be back to soon finalize this and remove the draft status. |
This PR makes it possible for Positron (or any execution context) to inform cli how file hyperlinks need to be constructed in order to work in Positron's terminals. What I mean by "work" is that file hyperlinks open in this product and don't delegate to the operating system. This is beneficial for Positron's R package development tasks, most especially
devtools::test()
, which will emit links likepath/to/test-foo:10:3
when tests fail. Companion to the already-merged #739. Here's a video of runningdevtools::test()
and taking advantage of clickable filepaths and runnable code (for reviewing or accepting snapshots):Screen.Recording.2024-12-16.at.4.10.39.PM.mov
Note the screen recording is made with a local branch of Positron, which also needs another PR.
Still a draft, because I need to add tests and docs, but I'll start adding some comments to get a discussion going.