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

Hyperlinks #1544

Merged
merged 14 commits into from
Feb 18, 2022
Merged

Hyperlinks #1544

merged 14 commits into from
Feb 18, 2022

Conversation

romainfrancois
Copy link
Contributor

@romainfrancois romainfrancois commented Jan 7, 2022

Proof of concept to display link in the test results.

image

After clicking the link, this opens the file at the requested line/col:

image

currently needs rstudio/rstudio#10299

@romainfrancois romainfrancois marked this pull request as draft January 7, 2022 11:12
R/expectation.R Outdated Show resolved Hide resolved
@romainfrancois
Copy link
Contributor Author

🚀

Enregistrement.de.l.ecran.2022-01-07.a.12.27.04.mov

@romainfrancois
Copy link
Contributor Author

This no longer needs dev version of cli.

R/expectation.R Outdated Show resolved Hide resolved
@romainfrancois romainfrancois marked this pull request as ready for review January 11, 2022 11:39
@romainfrancois

This comment has been minimized.

kevinushey added a commit to rstudio/rstudio that referenced this pull request Feb 8, 2022
* identify ansi hyperlinks

* unused

* HYPERLINK_REGEX only matching one anchor code (rather than both)

* attempt to render <a> in the console output

* only match at start

* handle urls in ClassRange

* Adding style xtermUnderline to link <span>

* retain both url and params in Hyperlink class

* use Event.setEventListener() instead of onclick attribute

* add consoleFollowHyperlink() rpc method

* prepare to use consoleFollowHyperlink()

* unused

* Update src/cpp/session/modules/SessionRCompletions.R

* add text to console_follow_hyperlink rpc method

* invoke the console_follow_hyperlink rpc

* handle links with type=help:topic=<fun>

* type=file -> .rs.api.navigateToFile()

* type=viewer

* define R_CLI_HYPERLINKS=true in build sessions

* rather define R_CLI_HYPERLINKS in the main session and let it be copied on build sessions

* Add and use Hyperlink.getTitle()

* no longer user [params] for dispatch to action, just use url

* unused import

* fix file handler when no location is given

* use the uri scheme to dispatch. use `[params] = target=viewer` to open in viewer

* using file:(line):(col) syntax

* parse name=value: params

* remove local variable hyperlink_code, simplify.

* Apply suggestions from code review

Co-authored-by: Kevin Ushey <kevin@rstudio.com>

* describe params= and rm the unused funs

* - obsolete comment

* rm _ suffix for public members

* rm _ suffix

* just rely on browseURL() to follow help links

* handle rstudio:help scheme, and rstudio:print

* shelf the serialization idea, use params for `rstudio:help`

* only needs first thing

* Add style xtermHyperlink to themes

* Revert "Add style xtermHyperlink to themes"

This reverts commit ad55c22.

* no longer add xtermUderline style automatically

* rstudio:viewer as a prefix

* typo

* update comment about `file://` handler

* apply style xtermHyperlink to links, currently doing nothing

* default style dor xtermHyperlink in the themes. Cancelled if other cli style is applied

* add color to xtermHyperlink class

* Improve link title

* special url `rstudio:run` to send code to the console

* allow the form `rstudio:run:code`

* fix file:// handling

* hyperlink artifically add underline, italic and magenta styles

* Apply suggestions from code review

Co-authored-by: Kevin Ushey <kevin@rstudio.com>

* Not adding italic to links by default

* Us params for file:// links, because :suffix don't work in e.g. iTerm. see r-lib/testthat#1544 (comment)

* make <a> instead of <span>

* remove unintentionally added import

* col was not correctly inialized

* no longer set the href ad params attributes

* no longer set the href and params attrinutes

* specify cursor: pointer in .xtermHyperlink css class.

* unused import

* squeeze in a VirtualConsoleServerOperations interface

* import

* inject VirtualConsoleServerOperations in VirtualConsole ctor

* Fake VirtualConsoleServerOperations in tests

* inject VirtualConsoleServerOperations in the constructor

* + unit test

* news bullet

* Apply suggestions from code review

Co-authored-by: Kevin Ushey <kevin@rstudio.com>

* Remove ... in NEWS

* missing ,

* missing argument

Co-authored-by: Kevin Ushey <kevin@rstudio.com>
@hadley
Copy link
Member

hadley commented Feb 10, 2022

@romainfrancois is this ok to merge, do you think?

@romainfrancois
Copy link
Contributor Author

Yes I think so. Probably not worth advertising it in the NEWS yet though, as this will need a spotted-wakerobin (2022-06) build of rstudio.

@gaborcsardi
Copy link
Member

It will also happen in terminals that cli detects hyperlink support for, e.g. iTerm2.

code_accept <- paste0("snapshot_accept('", name, "')")
code_review <- paste0("snapshot_review('", name, "')")
link <- function(code) {
cli::style_hyperlink(code, paste0("rstudio:run:testthat::", code))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gaborcsardi should we be doing something so that this only appear as a link in the ide ?

Copy link
Member

Choose a reason for hiding this comment

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

I don't mind.

@hadley
Copy link
Member

hadley commented Feb 13, 2022

I think it’s worth a news bullet even if it only works in an upcoming release.

@lionel- we should definitely include this in the next release.

@lionel-
Copy link
Member

lionel- commented Feb 15, 2022

Is this ready to be merged?

@romainfrancois
Copy link
Contributor Author

Yes, we might follow up in r-lib/cli#416 to only display the rstudio:run: links in the ide, but as far as testthat is concerned, this is ready.

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.

4 participants