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

Reset is added/removed in various cases #2

Open
ehuss opened this issue Nov 19, 2019 · 0 comments
Open

Reset is added/removed in various cases #2

ehuss opened this issue Nov 19, 2019 · 0 comments

Comments

@ehuss
Copy link

ehuss commented Nov 19, 2019

After d228a29, there are a number of cases where the reset code is not passed through, or added. In some cases, it doesn't matter too much, others it causes incorrect rendering.

The first case (that doesn't matter too much, but broke Cargo's CI), is two resets in a row, which for some reason rustc likes to generate. These get squashed to one reset.

Input \u{1b}[0m\u{1b}[0m
Output \u{1b}[0m

The next is when a string starts with a color without a reset, a reset gets added:

Input \u{1b}[1mbold\u{1b}[31mred\u{1b}[0m
Output \u{1b}[0m\u{1b}[1mbold\u{1b}[0m\u{1b}[31mred\u{1b}[0m

This causes rendering changes:
image

Another case is when reset codes are in the middle of a series of codes:

Input \u{1b}[42m\u{1b}[0m\u{1b}[31mred?\u{1b}[0m
Output \u{1b}[0m\u{1b}[31m\u{1b}[42mred?\u{1b}[0m

image

This is somewhat due to how termcolor works. ANSI is stateful, but termcolor does not track the current state, and so each color change resets everything. But fwdansi is now partially stateful, and doesn't quite match what I would expect.

It would be ideal if fwdansi was completely idempotent when emitting ANSI. It would also be ideal if it handled colors on Win 7/8 in such a way to simulate ANSI stateful behavior. These two issues might be in conflict with one another, since termcolor works fundamentally differently from ANSI. ☹️

bors added a commit to rust-lang/cargo that referenced this issue Nov 19, 2019
Add hack for fwdansi change.

This is a hack to fix a test failing on CI.

The issue is that fwdansi 1.1 was published with a change on how it processes reset codes. I have filed kennytm/fwdansi#2 with some details on some of the issues.

This is just a workaround to get the test to pass.  I'm not too happy with it, but other choices seemed less than ideal.  Am happy to consider them, though:

- Remove fwdansi, and give up supporting color on Win 7/8, old 10.
- Change `Shell` so that it knows whether or not the console supports ANSI, and only use fwdansi when necessary.  This means Win 10 would stop using fwdansi, but 7/8 would continue to use it. It is a little awkward – I think it would have to call wincolor::set_virtual_terminal_processing and see if it succeeds.
- Wait for a change to fwdansi to make resets more transparent.

I am very tempted by the first choice.  rustc's color output has been broken on win 7/8 for a long time, and only a very small number of people have complained.  I have spent way too much time dealing with something I think nobody really uses.

cc @kennytm
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

No branches or pull requests

1 participant