Skip to content

fix debug console to handle more than one graphics mode in a single escape sequence#38981

Merged
isidorn merged 2 commits intomicrosoft:masterfrom
dyc3:master
Nov 27, 2017
Merged

fix debug console to handle more than one graphics mode in a single escape sequence#38981
isidorn merged 2 commits intomicrosoft:masterfrom
dyc3:master

Conversation

@dyc3
Copy link
Contributor

@dyc3 dyc3 commented Nov 22, 2017

According to ascii-table.com, the spec allows for more than one graphics mode to be specified at a time.

@msftclas
Copy link

msftclas commented Nov 22, 2017

CLA assistant check
All CLA requirements met.

@isidorn isidorn added this to the November 2017 milestone Nov 23, 2017
@isidorn
Copy link
Collaborator

isidorn commented Nov 23, 2017

@dyc3 thanks a lot for your contribution! This looks great to me.
However do you have an example where this contribution clearly helps?
Also does this maybe fix one of #21423 #6845

@dyc3
Copy link
Contributor Author

dyc3 commented Nov 24, 2017

Sure, @isidorn.
Here's before my fix:
Before
and after:
After

This would appear to fix #6845, but not #21423. I think that how the function is currently written does not make #21423 easy to fix, and it'll probably have to be rewritten for that.

@isidorn
Copy link
Collaborator

isidorn commented Nov 24, 2017

@dyc3 I checked and this indeed fixes #6845 - great!
In the after picture I do not see the actual difference since it is cutoff but does not really matter.
The only thing which makes me nervous is the while (chr !== 'm') loop which can spin infinetely. Can we rewrite this in such a way that we are sure it will always finishes?

@dyc3
Copy link
Contributor Author

dyc3 commented Nov 24, 2017

@isidorn Ah, I didn't see that before. I'll see what I can figure out.

@dyc3
Copy link
Contributor Author

dyc3 commented Nov 24, 2017

Unless there is something I'm missing, there is a practical limit to the number of graphic modes specified at one time: bold, underscore, blink, reverse video, concealed, foreground color. and background color. So limiting codes to a length of 7 at most should do the trick.

@isidorn
Copy link
Collaborator

isidorn commented Nov 27, 2017

@dyc3 looks good, thanks a lot. Merging it in

@isidorn isidorn merged commit 0474a98 into microsoft:master Nov 27, 2017
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants