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

Please finish support for OSC 4 and friends #317

Closed
Jaykul opened this issue Dec 6, 2018 · 16 comments · Fixed by #891
Closed

Please finish support for OSC 4 and friends #317

Jaykul opened this issue Dec 6, 2018 · 16 comments · Fixed by #891
Assignees
Labels
Area-VT Virtual Terminal sequence support Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Conhost For issues in the Console codebase Resolution-Fix-Available It's available in an Insiders build or a release

Comments

@Jaykul
Copy link
Contributor

Jaykul commented Dec 6, 2018

I was really happy to see support for OSC 4 to set the 16 palette colors (and I recently helped a friend add it to the docs, the way it is now) ...

However, I have some problems. I'm hoping you can address them all as one request, but I'm happy to split them out. First, as background the xterm control sequences docs. Obviously these are not sacred, but they do represent a sort-of standard in that other terminals and tmux have implemented these same sequences the same way...

  1. OSC 4 should support multiple colors. XTerm and tmux (and others) support multiple pairs of index;spec without having to repeat the ESC]4; part, so you can do: ESC]4;0;rgb:00/00/00;1;rgb:00/00/70;...\\ and list out the whole set of colors as one giant escape sequence.
  2. It doesn't allow setting the extended xterm 88 or 256 color palette (see Unable to set colors above index 15 with OSC 4 #313).
  3. It doesn't support querying. I should be able to put a "?" in place of the rgb spec and get back a control sequence (which is supposed to be the exact same format), such that it could be used to set the color to the same RGB value it is already. This would allow me to check if I'm dealing with PowerShell blue instead of magenta by writing ESC]4;5;?\\ and parsing the output

Additionally ... without support for OSC 10 and 11 I cannot change the default foreground and background colors, so there's no way to fix (even temporarily) the fact that PowerShell is using 5 as it's default background color. NOTE that these should also support querying with a "?"

AND EVEN WORSE ...

We really also need support for OSC 104 (and 110 and 111) which would reset the colors changed by OSC 4, 10, and 11 to their defaults. Note that if done right, they support resetting individual colors, but as a first draft, I'd settle for the parameterless reset of the entire table -- and for my own purposes, if you implement 110 and 111 as meaning force WHITE text on a BLACK background, without implementing 10 and 11, I wouldn't cry 😉

@zadjii-msft
Copy link
Member

Okay, this is a great writeup.

However, there's a LOT of work to be done here - more than I can likely justify getting into the next (19H1) release of Windows. That being said, I have filed a pile of internal tasks on me to make sure these scenarios are all covered.

As you called out, #313 is fortunately on my tasklist for this month, so hopefully that one should be out soontm.

This is deliverable MSFT:19859141

@zadjii-msft zadjii-msft self-assigned this Dec 7, 2018
@zadjii-msft zadjii-msft added this to the Backlog milestone Dec 7, 2018
@zadjii-msft zadjii-msft added Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Conhost For issues in the Console codebase labels Dec 7, 2018
@Jaykul
Copy link
Contributor Author

Jaykul commented Dec 7, 2018

For what it's worth, @zadjii-msft I would say OSC 110 and 111 (if implemented such that they return it to white on black) would be way more useful than any of the rest of it.

Honestly, the way it is now (or with #313 fixed) it's not very useful. Without knowing/controlling which color is the actual (default) background and foreground, setting the color palette is almost pointless -- did you see that screenshot on #81 with the pink background and bright yellow text?

@zadjii-msft
Copy link
Member

Ah, so I'd say the main problem you're having is that powershell has hard-coded it's default colors to be yellow on magenta, then re-defined what yellow and magenta are (to a light grey and Powershell Blue). We've tried to get them to change this in the past, but it's never happened.

Currently, o of the big differences between conhost and other terminals is that we don't support the notion of a default color that's not a part of the color table.

I'd have to investigate, but presumably, using those OSC's to reset the "default color" would revert the default color to whatever conhost had when it was launched, but it wouldn't change which indices were set as the defaults.

Also stay tuned in this area, I think we'll have something bubbling up the source tree that I can't really talk about quite yet :)

@oising
Copy link
Collaborator

oising commented Dec 7, 2018

  1. OSC 4 should support multiple colors. XTerm and tmux (and others) support multiple pairs of index;spec without having to repeat the ESC]4; part, so you can do: ESC]4;0;rgb:00/00/00;1;rgb:00/00/70;...\\ and list out the whole set of colors as one giant escape sequence.

There is one thing though I'd be careful about - it's that (CSI) control sequences may have multiple parameters, but to be strictly compatible there's a limit of sixteen parameters as anything beyond that is silently ignored. I'm not sure if the same limits apply to OSC sequences, but it's likely. But if tmux/xterm/screen are doing it, I guess it's probably okay?

Source: https://vt100.net/emu/dec_ansi_parser

@Jaykul
Copy link
Contributor Author

Jaykul commented Dec 7, 2018

I don't think that applies @oising. CSI parameters are also limited to be numbers, because the thing is building a buffer of parameters to pass to the command. For OSC, there's no buffer: OSC strings have to be terminated by ST, CAN, SUB or ESC -- any other characters are supposedly just "passed" from the control string to the handler as they arrive...

@zadjii-msft I'll retract what I said earlier, if that's how it is. In that case, OSC 110 and 111 are not what I want. I obviously need OSC 10 and 11 instead. 😬 I guess that really, one way or another, I need to be able to force the background to a known value.

I have been assuming that the windows console wouldn't suddenly learn how to have a different default bg/fg and would be limited to one of the 16 palette values ... I'm used to that by now. But I was thinking the defaults would be defaults in the global sense, or at least in the sense that's used in the window's menu...

image

So yeah. Anyway. From my perspective, I don't really care if the background is black, blue, magenta -- but I do need know for certain which color index the background is.

The goal is to avoid that awkward situation where I write text in the background color, when I thought I was writing highly visible pink or white on black ...

And yes, the number one offender is the PowerShell blue background -- but people can (and do) change their defaults. I have had several co-workers with vision impairments and they all tend to force their consoles to black on a white background.

@oising
Copy link
Collaborator

oising commented Dec 10, 2018

This probably belongs here as a reference for others finding this thread:

https://blogs.msdn.microsoft.com/commandline/2018/12/10/new-experimental-console-features/

@Jaykul
Copy link
Contributor Author

Jaykul commented Dec 11, 2018

Very exciting -- and makes OSC 10 and 11 even more critical 🤔 is there even a way to set those without the dialog right now? 🤨 And in particular querying.

How could an application update it's own knowledge of BackgroundColor or ForegroundColor to what the user has set in the dialog? I mean, PowerShell exposes those on the $Host.UI.RawUI -- but how could it read them? Is there any way it could know they'd been changed?

@zadjii-msft
Copy link
Member

Those aren't modifiable or queryable at the moment using VT. It's a pretty early experimental feature at the moment, so I'll make sure to bump up the priority of changing and querying those values, but that will all probably fall under the scope of the aforementioned deliverable.

@oising
Copy link
Collaborator

oising commented Mar 2, 2019

Any progress here, @zadjii-msft ?

@Jaykul
Copy link
Contributor Author

Jaykul commented May 14, 2019

OK, since we have source ... I'm going to try doing this myself, but I need a little help.

Over on Jaykul/SettableDefaultColors I've got a working version of OSC 10 and 11 for conhost, but although they clearly have some effect, I feel like they're not working properly in Terminal, and I'm not sure what I missed.

Can anyone either:

  • Give me a pointer on how to get C++ debugging to work when I launch the Terminal app (it works fine if I launch host.exe, but not Cascadia).
  • Take a look at the list of changes in 0f934b and tell me where I missed implementing it (there are a lot of places to implement this).

@mKay00
Copy link
Contributor

mKay00 commented May 14, 2019

You have to set to Native only in the properties for debugging: https://github.com/microsoft/Terminal/blob/master/doc/Debugging.md

@zadjii-msft
Copy link
Member

@Jaykul great work with that - that's pretty much everything that I'd think you'd need.

If it doesn't work for the terminal, I'd presume that for some reason, the color change isn't making it's way through conpty.

I'd think that the code you correctly grabbed to return false in conpty mode (near this line) should make sure the sequence is re-emitted through conpty. I'd set a breakpoint in TerminalCore/TerminalDispatch::SetDefaultForeground to make sure that the SetDefaultForeground handler is actually being called. It's possible that returning false doesn't actually bubble VT sequences up as I think they do

@Jaykul
Copy link
Contributor Author

Jaykul commented May 14, 2019

@zadjii-msft in a related note ... in OutputStateMachineEngine::ActionOscDispatch (and the other Action*Dispatch handlers) there's this pattern of a double switch: first pass to parse, second pass to actually dispatch ...

Is there a reason it needs to be like that?

It prevents supporting arrays like xterm does: multiple sets of semicolon-delimited index;colorspec pairs after a single OSC 4 (basically, we need to dispatch multiple times, as we're parsing).

It also complicates querying (when the value is ? instead of an rgb value). Actually -- I guess we're going to need a whole other set of API calls for queries so which call to make will depend on the parse results! Actually, I'm not sure how to inject output from inside the state machine, is that even possible?

@zadjii-msft
Copy link
Member

There's definitely no reason that the parser needs to behave like that. You're right that it probably shouldn't, I just haven't gotten the time to go back and update that yet :P

There definitely is a way to respond back to the caller from the adapter. I can't remember the name of the call off the top of my head, something like Device Status Request or something, where we reply with the cursor position.

@Jaykul
Copy link
Contributor Author

Jaykul commented May 18, 2019

I figured out why the terminal wasn't working. In order to change the background color of the Windows Terminal, you have to reset all the Acrylic stuff. I've got it working!

@ghost ghost added the Needs-Tag-Fix Doesn't match tag requirements label May 24, 2019
@miniksa miniksa added the Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. label May 24, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label May 24, 2019
@DHowett-MSFT
Copy link
Contributor

Hey hey, this is now available in the inbox windows console as of insider build 18932! Congrats!

@DHowett-MSFT DHowett-MSFT added Resolution-Fix-Available It's available in an Insiders build or a release and removed Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. labels Jul 3, 2019
@WSLUser WSLUser mentioned this issue Jun 2, 2020
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-VT Virtual Terminal sequence support Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Conhost For issues in the Console codebase Resolution-Fix-Available It's available in an Insiders build or a release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants