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
Make URLs in confirmation panels clickable, and underline them #3446
Conversation
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferencesYou may notice some variations in coverage metrics with the latest Coverage engine update. For more details, visit the documentation |
if strings.HasSuffix(underlinedLink, "\x1b[0m") { | ||
// Replace the "all styles off" code with "underline off" code | ||
underlinedLink = underlinedLink[:len(underlinedLink)-2] + "24m" | ||
} |
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 is pretty hacky, but we need it so that we don't cancel the bold text. I don't see a better way of doing this except by making changes to gookit/color
.
Thoughts on using tcell's terminal hyperlinks ability? gdamore/tcell#300 |
I had no idea this existed. I'll play with this to see if it's useful for our case. |
So I'm not very thrilled by tcell's hyperlinks. I couldn't get it to work in Terminal.app, only in iTerm2 (but I may well have done something wrong). In general it's a bit unclear to me how widely supported the feature is yet. Also, you have to hold down Command while clicking them (ctrl on Windows), which I find inconvenient, and hard to discover. I would prefer them to behave more like links on a web page; just click them. Finally, it would take some effort to make this available in gocui, it's actually not quite clear to me how to design the APIs. There's plenty of code that just passes around fg and bg Attributes to functions (and those are enums); but we need to pass attached data for this one. It would take a while to add this to the current design of how attributes work in gocui. To sum it up, I'd prefer to stick with the more pedestrian approach I'm taking here. |
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.
Makes sense, LGTM
Changing globals in the init() function of a test file is a bad idea, as it affects all other tests that run after it. Do it explicitly in each test function that needs it, and take care of restoring the previous value afterwards.
Make it recognize URLs wrapped in angle brackets, and followed by punktuation. We don't need this for the status panel, but we will need it for confirmation panels.
This is not opt-in, we do it always. I can't imagine a situation where we wouldn't want it.
5795f23
to
5d509ef
Compare
This is especially helpful for the breaking changes popup, which has a link to the release notes, but it could also be useful for other panels that display some warning or error with a link to more information.
go generate ./...
)docs/Config.md
) have been updated if necessary