(Start to) handle cli hyperlinks in the terminal used for pkg dev tasks #5231

jennybc · 2024-10-31T03:55:51Z

Addresses #5218

This PR makes suggested code hints like testthat::snapshot_accept() or testthat::snapshot_review() clickable and runnable from the integrated terminal where positron-r sends package development tasks, such as devtools::test(). "Runnable" in the sense that the code is run in the user's R console. This gets routed into the same handlers as clickable code entered directly in the console, so the existing safety guards are in place.

(Note that this PR does not change behaviour for file hyperlinks, which I've crossed out in the screenshot above. Those hyperlinks currently get delegated to the operating system, so for many R users, a .R file will open in RStudio. Changing that is a separate problem #5409.)

QA Notes

To see the new behaviour, you need a development version of the cli R package. I would install that via pak::pak("r-lib/cli"). (Context: r-lib/cli#739.)

Then you need to run tests or R CMD check on a package with a failing snapshot test, in order to get a clickable invitation to accept a new snapshot or to review the proposed snapshot in a Shiny app. I have made a toy package that could be obtained via usethis::create_from_github("jennybc/clilinks"). It has a couple of snapshot tests that will always fail 😄 because they attempt to snapshot a random number.

Install dev cli: pak::pak("r-lib/cli")
Identify a package that tickles this feature and open it in Positron, perhaps via: usethis::create_from_github("jennybc/clilinks")
Use our gesture for running package tests: Ctrl/Cmd + Shift + T or select R: Test R Package from the command palette.
Click on a "runnable code" hyperlink:
- Note this requires Ctrl/Cmd click! This is a VS Code convention, not specific to us.
- Note that you might have to click twice. This is a bug (or 2 bugs?) for which fixes are making their way through the system (Sometimes links don't work the first time you click microsoft/vscode#230010). Again, not us.
- You will need to grant the extension permission to handle such URIs, either as a one-off or more persistently. This is a VS Code feature.

If you click testthat::snapshot_review(), you should see the code execute in the R console and this Shiny app appears. Click on "Accept" (the fact that "Skip" seems broken is r-lib/testthat#2025 and unrelated to this PR).

jennybc · 2024-10-31T03:59:33Z

extensions/positron-r/package.json

@@ -3,7 +3,7 @@
  "displayName": "%displayName%",
  "description": "%description%",
  "version": "0.0.2",
-  "publisher": "vscode",
+  "publisher": "positron",


I suspect this should be the case for all extensions we (Positron) have introduced. Harder to say what should happen to VS Code built-in extensions that we modify, but worth thinking about.

The relevance to this PR is that the publisher affects the form of URI that the associated extention (positron-r, in this case) is allowed to handle. The general form is:

{product-name}://{publisher}.{extension-name}

which means that positron-r can handle URIs that begin like so:

positron://positron.positron-r

From #5268, we're going to use "positron" as publisher here.

extensions/positron-r/package.json

extensions/positron-r/package.nls.json

extensions/positron-r/src/commands.ts

extensions/positron-r/src/extension.ts

extensions/positron-r/src/tasks.ts

Addresses #1957 Requires posit-dev/ark#625 What's the payoff? * Sometimes you want to get info about a package version without necessarily challenging the user to install/upgrade. (I need this to finish #5231, for example). * If you do need to invite the user to install or upgrade a package, this extra info allows us to build a better message. ### QA Notes To experience that `checkInstalled()` still works and to see the user-facing message in the absence of a minimum version requirement, here's one idea: * Remove the styler package: `remove.packages("styler")` * Trigger the *Format Document* command You should see this: <img width="404" alt="Screenshot 2024-11-13 at 3 46 39 PM" src="https://github.com/user-attachments/assets/2231ec70-d335-4f99-b5f9-598cd1c19301"> To experience that `checkInstalled()` still works and to see the user-facing message in the presence of a minimum version requirement, here's one idea: * Install renv at a version below positron-r's minimum version, which is currently 1.0.9. I would do this with `pak::pak("[email protected]")`. * Walk through *File > New Project > R Project* and click the box to use renv: <img width="701" alt="Screenshot 2024-11-13 at 3 35 45 PM" src="https://github.com/user-attachments/assets/430908cb-9fce-43fe-8d39-a6fec56f9a70"> As the new project is being stood up, you should see this: <img width="401" alt="Screenshot 2024-11-13 at 3 37 12 PM" src="https://github.com/user-attachments/assets/ee3d82a1-5479-47e6-82bd-a693583d2f4a">

juliasilge

This is working well for me! 🎉

My main question is about the file hyperlinks. I understand that this PR is not meant to address those but it leaves us in a state where both kinds of hyperlinks look like you should click them but clicking one kind results in something nice and clicking another kind results in something QUITE UNPLEASANT. Is it possible not to make file hyperlinks clickable, since we're not yet handling them? If not, can we put this new feature behind a feature flag until #5409 is resolved? Right now we have landed a combo of "looks just like what I should click" plus "both surprising and awful result".

jennybc · 2024-11-20T01:38:49Z

Unfortunately it's not (yet?) possible to have the run/help/vignette hyperlinks but not have file hyperlinks. At least, AFAIK, it's not possible and I'm pretty up to speed on this part of cli right now.

I feel like we can safely merge and I will tackle #5409 in the near term. This is behind an implicit feature flag of sorts, in that we only activate the hyperlinks in the presence of truly bleeding edge cli (version 3.6.3.9001). It's not just any dev version, but a dev version I bumped to specifically for this (r-lib/cli@9d6e9db). I think the few people who might install this version of cli can probably cope for the couple weeks until file hyperlinks fully catch up.

Plus, as an experiment, I just changed the macOS file affiliation for .R from RStudio to Positron and this also "solves" the problem right away.

jennybc · 2024-11-20T03:01:08Z

extensions/positron-r/src/uri-handler.ts

+	// cli 3.6.3.9001 gained support for configuring the URL format for hyperlinks.
+	const cliPkg = await session.packageVersion('cli', '3.6.3.9001');
+	const cliSupportsHyperlinks = cliPkg?.compatible ?? false;
+
+	if (!cliSupportsHyperlinks) {
+		// eslint-disable-next-line @typescript-eslint/naming-convention
+		return { R_CLI_HYPERLINKS: 'FALSE' };
+	}


Here's where we exit early, without activating hyperlinks in the integrated terminal, in the absence of bleeding edge cli.

juliasilge

This is behind an implicit feature flag of sorts, in that we only activate the hyperlinks in the presence of truly bleeding edge cli (version 3.6.3.9001).

I am not a fan of this getting merged into main as is, but this is a fair point. Can you confirm with Gabor and take on making sure that we do not get a surprise cli release before the file hyperlinks are fixed up?

Alternatively, you could throw a quick IsDevelopmentContext around this to gate it from being exposed to users as is?

jennybc · 2024-11-21T20:23:30Z

Yes @gaborcsardi has confirmed there will be no surprise cli release.

I don't love the idea of the "dev build only" gate, because I am hoping to use this in my package dev work, where I generally use a an Positron release build (albeit a daily). If if we think it needs to be gated, I could add a setting for it.

jennybc · 2024-11-24T20:49:52Z

Feature flag is in.

jennybc commented Oct 31, 2024

View reviewed changes

This was referenced Oct 31, 2024

Make x-r-whatever-style hyperlinks configurable r-lib/cli#739

Merged

Decide how to set the publisher field for extensions #5268

Closed

jennybc mentioned this pull request Nov 13, 2024

New function to expose more info about an installed package #5365

Merged

jennybc added 6 commits November 18, 2024 11:05

(Start to) handle cli hyperlinks in the terminal used for pkg dev tasks

abc76af

Move URI handling into its own file

09a3ad1

Remove chattiness that was useful in dev

6c9d1e0

Handle the env vars more crisply

e8b9fdb

Temporarily emit all supported hyperlinks

2694a20

Remove temporary command/task to tickle help and vignette links

d78dff0

jennybc force-pushed the handle-r-osc-hyperlinks-in-terminal branch from a64b778 to d78dff0 Compare November 18, 2024 20:56

jennybc added 2 commits November 18, 2024 13:24

Put this comma back, the way I found it

40f3e9e

Update "publisher" in mentions of the extension

ee4b359

jennybc marked this pull request as ready for review November 18, 2024 21:29

jennybc requested a review from juliasilge November 18, 2024 21:29

juliasilge requested changes Nov 19, 2024

View reviewed changes

jennybc commented Nov 20, 2024

View reviewed changes

juliasilge previously approved these changes Nov 20, 2024

View reviewed changes

Put behind a feature flag

01ab77a

jennybc dismissed juliasilge’s stale review via 01ab77a November 24, 2024 20:48

juliasilge approved these changes Nov 24, 2024

View reviewed changes

jennybc merged commit 9084df0 into main Nov 25, 2024
4 checks passed

jennybc deleted the handle-r-osc-hyperlinks-in-terminal branch November 25, 2024 05:50

github-actions bot locked and limited conversation to collaborators Nov 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(Start to) handle cli hyperlinks in the terminal used for pkg dev tasks #5231

(Start to) handle cli hyperlinks in the terminal used for pkg dev tasks #5231

jennybc commented Oct 31, 2024 •

edited

Loading

jennybc Oct 31, 2024 •

edited

Loading

jennybc Nov 18, 2024

juliasilge left a comment

jennybc commented Nov 20, 2024

jennybc Nov 20, 2024

juliasilge left a comment

jennybc commented Nov 21, 2024 •

edited

Loading

jennybc commented Nov 24, 2024

(Start to) handle cli hyperlinks in the terminal used for pkg dev tasks #5231

(Start to) handle cli hyperlinks in the terminal used for pkg dev tasks #5231

Conversation

jennybc commented Oct 31, 2024 • edited Loading

QA Notes

jennybc Oct 31, 2024 • edited Loading

Choose a reason for hiding this comment

jennybc Nov 18, 2024

Choose a reason for hiding this comment

juliasilge left a comment

Choose a reason for hiding this comment

jennybc commented Nov 20, 2024

jennybc Nov 20, 2024

Choose a reason for hiding this comment

juliasilge left a comment

Choose a reason for hiding this comment

jennybc commented Nov 21, 2024 • edited Loading

jennybc commented Nov 24, 2024

jennybc commented Oct 31, 2024 •

edited

Loading

jennybc Oct 31, 2024 •

edited

Loading

jennybc commented Nov 21, 2024 •

edited

Loading