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

Support hyperlinks in supported terminals #37

Merged
merged 21 commits into from
Apr 22, 2020
Merged

Support hyperlinks in supported terminals #37

merged 21 commits into from
Apr 22, 2020

Conversation

nicholaschiasson
Copy link
Contributor

@nicholaschiasson nicholaschiasson commented Sep 19, 2019

Fixes #35


IssueHunt Summary

Referenced issues

This pull request has been submitted to:


IssueHunt has been backed by the following sponsors. Become a sponsor

@nicholaschiasson
Copy link
Contributor Author

#35

@sindresorhus
Copy link
Member

// @stroncium @TiagoDanin Would you be able to help review?

package.json Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
test.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
test.js Outdated Show resolved Hide resolved
test.js Outdated Show resolved Hide resolved
@sindresorhus
Copy link
Member

@nicholaschiasson Ping :)

@nicholaschiasson
Copy link
Contributor Author

Just updating, I plan to resolve all of the open discussions here soon! Sorry for the wait 😄

Fixed hyperlink wrap by adding the hyperlink ansi wrap opening and closing on each beginning and ending of line, respectively, where necessary.
Fixed existing bug where the string 'pre' was being incorrectly indexed without considering surrogate pairs, instead deconstructed it into an array.
Added test case to cover code paths where an ansi escape is used which does not match on Select Graphic Rendition or ansi hyperlink patterns.
Made hyperlink test cases much more understandable.
@nicholaschiasson
Copy link
Contributor Author

Addressed all discussions, but apparently support for node 8 is broken. I will look into this soon.

@sindresorhus
Copy link
Member

Support for Node.js 8 doesn’t matter. I plan to target Node.js 10 after this PR.

@nicholaschiasson
Copy link
Contributor Author

Support for Node.js 8 doesn’t matter. I plan to target Node.js 10 after this PR.

Ah whoops, I fixed it nonetheless. Up to you which style is preferable, named regex capture groups or not, but in any case I think this is ready again for review.

@sindresorhus
Copy link
Member

Can you revert 2b6a0b4?

@sindresorhus sindresorhus changed the title Tweaking to support hyperlinks in supporting terminals Support hyperlinks in supported terminals Apr 22, 2020
@sindresorhus sindresorhus merged commit 0e49047 into chalk:master Apr 22, 2020
@sindresorhus
Copy link
Member

Thank you :)

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.

Add support for hyperlink
3 participants