vadimdemedes / ink

🌈 React for interactive command-line apps
https://term.ink
MIT License
26.42k stars 594 forks source link

Fix broken link escapes #623

Closed matteodepalo closed 1 year ago

matteodepalo commented 1 year ago

A bug has been introduced with the work done here and it relates to the tokenization of ansi escape characters. We noticed it with ansiEscapes.link and I added a test case in this repo which now fails.

@AlCalzone I don't know enough about your ansi-tokenize project, but could this be fixed there?

Here's a screenshot of the failing test:

Screenshot 2023-09-07 at 11 25 04

You can see from the screenshot that the link escape is not being closed properly so the link "leaks" in all the text after that.

AlCalzone commented 1 year ago

I agree this needs to be fixed in ansi-tokenize

AlCalzone commented 1 year ago

Fixed in v0.1.3: https://github.com/AlCalzone/ansi-tokenize/releases/tag/v0.1.3

(except that your test has extra " around the string)

matteodepalo commented 1 year ago

@AlCalzone thank you for fixing this so quickly! I've bumped the version in this PR.

matteodepalo commented 1 year ago

@AlCalzone we also saw a difference in the ansi escapes related to colors when upgrading. For example this component

<Box>
  <Text color="red">hello</Text>
  <Text color="green">world</Text>
</Box>

Before it would produce

'helloworld'

Now it produces

'helloworld'

There seems to be a missing [39m after the first word which I think it's the code to reset the color? Not sure if this is a difference in the way chalk works, maybe it has been updated? The end result doesn't seem to change much in our outputs.

AlCalzone commented 1 year ago

That's expected. ansi-tokenize uses the least amount of ansi codes necessary for a style change.

matteodepalo commented 1 year ago

Got it! Make sense, I'll update our tests.

vadimdemedes commented 1 year ago

Thanks @matteodepalo for the PR and thanks @AlCalzone for fixing it so quickly 💛