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

Revert "Transform file paths into hyperlinks" #9294

Closed
wants to merge 1 commit into from

Conversation

thymikee
Copy link
Collaborator

@thymikee thymikee commented Dec 11, 2019

Reverts #8980.

After #8980 (comment):

Some observations after using this in iTerm:

  • Jest UI flickers and jumps 😨 (may be related to using iTerm beta channel 🤔)
  • iTerm noticeably slows down when scrolling through the lines with hyperlinks (also overall execution time of seems to be slower by 2s in Jest's own test suite – running jest packages)
  • it sometimes gets wild and doesn't clear the next line:
    image
  • I still think the view with hyperlinks looks a bit cluttered:
with hyperlinks no hyperlinks
Screen Shot 2019-12-11 at 12 44 18 Screen Shot 2019-12-11 at 12 44 39

This feature definitely needs some polish and better support from terminal emulators. Happy to accept future improvements, but for now, let's revert.

@jeysal
Copy link
Contributor

jeysal commented Dec 11, 2019

Can't really comment on this, never used a terminal that supports this

@thymikee thymikee force-pushed the revert-8980-default-reporter-hyperlinks branch from 8bf3694 to 0909877 Compare January 23, 2020 03:09
@thymikee thymikee changed the title Revert "feat: transform file paths into hyperlinks" Revert "Transform file paths into hyperlinks" Jan 23, 2020
@SimenB
Copy link
Member

SimenB commented Jan 23, 2020

CI is unhappy. WRT the revert I have no opinion - they aren't rendered as links for me, so I have no frame of reference of its impact

@pedrottimark
Copy link
Contributor

@thymikee Like Simen and Tim, this does not affect me. That suggests the benefit-to-risk is not favorable, especially because you found a problem in a widely-used terminal. I trust your judgment.

@elstgav
Copy link

elstgav commented Apr 19, 2020

I would definitely like to see this PR merged; let me know if I can help!

I agree with #8980 (comment) that the jest output looks more cluttered with dashed underlines (and harder to read).

For those waiting, here’s a workaround to disable hyperlink underlines in iTerm2:

Go to Prefs > Advanced > Underline OSC 8 Hyperlinks and set to “No”

⚠️ Note that this will globally disable hyperlink highlighting, which could be useful in other contexts.

@SimenB
Copy link
Member

SimenB commented Apr 19, 2020

This is failing CI, happy to merge and release once fixed 😀

@thymikee thymikee force-pushed the revert-8980-default-reporter-hyperlinks branch 4 times, most recently from b21fd80 to 58d5126 Compare May 2, 2020 12:29
@thymikee thymikee force-pushed the revert-8980-default-reporter-hyperlinks branch from 58d5126 to 1e73b56 Compare May 2, 2020 12:50
@SimenB
Copy link
Member

SimenB commented Jul 30, 2020

@thymikee what's the status here?

@thymikee
Copy link
Collaborator Author

It's not bothering me this much recently, and it seems almost no-one else noticed. I'll close it for now, but happy to review any contributions in this area :)

@thymikee thymikee closed this Jul 30, 2020
@SimenB SimenB deleted the revert-8980-default-reporter-hyperlinks branch July 30, 2020 10:00
@thymikee thymikee mentioned this pull request Nov 23, 2020
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants