GNOME Bugzilla – Bug 567444
[PATCH] add support for OSC 10
Last modified: 2014-04-06 18:18:18 UTC
OSC 10 allows you to change the foreground color. The attached patch implements this functionality.
Created attachment 126243 [details] [review] 0001-add-support-for-OSC-10.patch
Humm... I wonder if it should call that function or modify the default cells. Same about the cursor color changing. I mean, the public API is used by the user to set/get colors and that shouldn't change by the input stream. Not sure. ChPe, what do you think?
It may seem a bit convuluted to use the public API to execute the change, but doing so ensures that any additional checks that are placed there in the future would also apply to the changes performed internally.
I agree with Behdad; I don't think it should change the colour set with the API, it should just use the new colour instead of the colour from the palette.
In order to keep the palette consistent, it would require a duplicate palette, which takes precedence over the API specified palette. Is that really the best option? If we go with that, perhaps a new set of APIs to access the input stream based palette would be warranted as well?
No, the user doesn't really need to be able to query the terminal state.
What about the following case: VTE is used as an embedded terminal. The user sets the foreground color using the OSC 10 command. The embedding application provides a configuration interface for the colors. Because there are no APIs to get the modified palette, and we dont modify the palette, the wrong colors would be passed back.
(In reply to comment #7) > What about the following case: VTE is used as an embedded terminal. The user > sets the foreground color using the OSC 10 command. The embedding application > provides a configuration interface for the colors. Because there are no APIs > to get the modified palette, and we dont modify the palette, the wrong colors > would be passed back. That's exactly why the palette should not be modified, in fact.
Setting patch status to need-work based on previous comments.
Can anyone expand on what the right way to do this is? I'm not really following the reasoning. If there is a "host" application (which embeds VTE, and can change colors using VTE APIs) and an "guest" application (a terminal app running inside VTE which can change colors using escape sequences), it may make sense to say that the guest should not be able to change the settings managed by the host. But I'm not entirely clear on what the alternative is. Should the host's settings just be the defaults? Meaning the guest can override them with escape sequences, but when the terminal is reset the host's settings are loaded? And by the same token, OSC 110 would revert OSC 10 by restoring the host's color for the foreground? I *think* this makes sense. Is this what you guys have in mind?
Saleem's proposal in comment 5 is the same as mine in bug 705985 comment 2. I'll give it a try, see that other bug for progress.
I'm looking at this now. Thanks for the patch Saleem. Setting/querying the foreground should go with its counterpart of resetting (OSC 110) as well as the same stuff for background. Maybe highlight color too. The querying part is too much code duplication, cries for being factored out to some helper function. Sibling issues: bug 151260, bug 640040, bug 722446.
I just found out that in Konsole you can have a different random bright background for each of your tabs. I think it's a really useful feature. (I remember a colleague of mine having different foreground for each of her terminals.) It helps you unconsciously locate the tab you're looking for. ("I had that thing open that I need now. Was it in the 6th tab or the 7th? Maybe the 8th. Don't know for sure. Let's find it as fast as possible. I recall it had a yellowish background...") This could be implemented in g-t, but given Gnome's direction in simplifying the UI and providing just the basic necessary alternatives, I don't think such a feature would be accepted mainstream. With OSC 10/11 support everyone can hack up their favorite custom randomness in their shell startup scripts.
Created attachment 266693 [details] [review] OSC 10, 11, 17 This patch adds support for OSC 10, 11 and 17 (background, foreground, and highlight background) colors, and their reset (110, 111, 117) counterparts. Goes on top of bug 640040's patches. Contains tons of copy-pasted spaghetti code, I'll factor them out.
Created attachment 266823 [details] [review] OSC 10, 11, 17 Updated to current git.
Created attachment 266931 [details] [review] OSC 10, 11, 17 (v2) Refactored to avoid code duplication. Please review, thanks.
Fixed in vte-0-36. Keeping the bug open for vte-next.