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

Fix: on Windows 8.1, ESC[7m (REVERSE) does not work. #51

Merged
merged 1 commit into from May 4, 2020

Conversation

@zetamatta
Copy link
Contributor

@zetamatta zetamatta commented May 4, 2020

Sorry, I mistaked on #50 . On Windows 8.1, the flag COMMON_LVB_REVERSE_VIDEO is ignored by OS's default terminal.

Please look at the rectangled area. You will find ESC[7m does not work on Windows 8.1.

The current code on Windows 8.1 (This case has the problem that ESC[7m does not work.)
image

Same test on Windows 10, (This case has no problems.)
image

So, I made the patch again which reverts the change for ESC[7m and ESC[27m.
But, it does not solute all problems. Please see these two images. ESC[27m loses the compatiblity with Windows10's native ANSI-Escape sequence.

Patch applied code on Windows8.1
image

Patch applied code on Windows10.
image

  • When it is merged, ESC[27m loses the compatibility with Windows10's native sequence.
  • When it is rejected, Windows 8.1 's ESC[7m does not work.

The idea exists to make the boolean-variable indicating ESC[7m is used or not, but I am afraid that it causes unexpected bugs.

I have added the undesired behavior to your code, so I have the duty to make the patch to revert it even if it may be rejected. I will respect your judgement whether the patch is merged or rejected.

@mattn mattn merged commit f1b5a0e into mattn:master May 4, 2020
1 check passed
@mattn
Copy link
Owner

@mattn mattn commented May 4, 2020

Thank you this too.

@zetamatta zetamatta deleted the fork2 branch May 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants