Urwid does not restore the user's palette

br0ns commented 3 years ago

Description:

Urwid restores the terminal palette to the default Xterm palette on exit (according to comments in raw_display.py and display_common.py).

Affected versions (if applicable)

[x] Latest stable version from pypi

Steps to reproduce (if applicable)

Configure a non-default palette in Xterm, then run example palette_test.py. Terminal colors are now the Xterm defaults.

Expected/actual outcome

Urwid should restore the user's palette on exit.

I have only tested this in Xterm, but the current palette can be read back using control sequence ^]4;0;?;1;?...^\ (i.e. use "?" instead of a color spec). Urwid could at least try to do that, then if that doesn't work default to the, eh well, default.

luchr commented 2 years ago

Or one can try/use OCS 104 (supported by xterm and VTE, I think): screen.write("\x1b]104\x07")

danschwarz commented 1 year ago

I'm having trouble reproducing the behavior described above. Tested using Xterm(372) on Ubuntu 22.04 and the current master.

I wrote a quick script to output the current palette values (in practice, it only seems to output Xterm "dynamic" palette slots 10-17, so it's limited to checking those)

#!/bin/bash

for i in {10..17}
do
    echo -en "\033]$i;?\007"
done

Starting up a fresh xterm instance, I see the following palette values:

XTERM BASELINE

^[]10;rgb:d8d8/d8d8/d8d8^[\^[]11;rgb:1818/1818/1818^[\^[]12;rgb:bebe/bebe/bebe^[\^[]13;rgb:bebe/bebe/bebe^[\^[]14;rgb:0000/0000/0000^[\^[]15;rgb:bebe/bebe/bebe^[\^[]16;rgb:0000/0000/0000^[\^[]17;rgb:bebe/bebe/bebe

catting this file in the shell will switch the dynamic palette values as per the base16-default-light theme: base16-light-theme.txt

BASE16-LIGHT

^[]10;rgb:5353/6767/6d6d^[\^[]11;rgb:fbfb/f3f3/dbdb^[\^[]12;rgb:5353/6767/6d6d^[\^[]13;rgb:bebe/bebe/bebe^[\15;rgb:bebe/bebe/bebe16;rgb:0000/0000/000017;rgb:bebe/bebe/bebed6d13;rgb:bebe/bebe/bebe14;rgb:0000/0000/0000

now launch palette-test.py, and then quit the program.

AFTER palette-test.py

^[]10;rgb:5353/6767/6d6d^[\^[]11;rgb:fbfb/f3f3/dbdb^[\^[]12;rgb:5353/6767/6d6d^[\^[]13;rgb:bebe/bebe/bebe^[\^[]14;rgb:0000/0000/0000^[\^[]15;rgb:bebe/bebe/bebe^[\^[]16;rgb:0000/0000/0000^[\^[]17;rgb:bebe/bebe/bebe

As you can see the values before and after launching palette-test are identical.

mfncooper commented 9 months ago

Let me add a few more data points here.

Using xfce4-terminal (v0.8.9.1) on Linux with a preset color scheme other than the default:

If I do nothing in my code, my Urwid app uses the terminal's color scheme, and not what I want my Urwid app to use. (That is, 'dark blue' in my app could be all sorts of colors other than dark blue, depending on the terminal's color scheme.)
If I configure 256 colors in my Urwid app like this:

    loop.screen.set_terminal_properties(256)
    loop.screen.reset_default_terminal_palette()

I still get the terminal's color scheme in my Urwid app.

If I configure 16 colors in my Urwid app like this:

    loop.screen.set_terminal_properties(16)
    loop.screen.reset_default_terminal_palette()

now I get the color scheme I want in my Urwid app, but the user's palette is messed up when my app exits and the user is returned to their terminal. I believe this is what @br0ns is seeing.

Using iTerm2 (v3.4.23) on Mac with a preset color scheme other than the default:

If I do nothing in my code, the behavior is the same as with doing nothing with xfce4-terminal, as described above.
If I call set_terminal_properties() with either 256 or 16 colors, I get the same behavior as for 16 colors with xfce4-terminal. That is, the user's terminal palette is messed up when my Urwid app exits.

It's more than a little strange that the two behave differently when setting 256 colors, but the same otherwise.

Since allowing my Urwid app to use whatever terminal palette the user has set up can often render my app illegible, or hard to read at best, I really need to be able to use the palette I want in Urwid. But it's kinda rude to have messed up the user's palette when they exit my Urwid app, so I also really need to be able to reset the palette when I exit my app.

Since both terminals set up with TERM=xterm, I tried using the XTPUSHCOLORS and XTPOPCOLORS CSI sequences, like this:

    loop.screen.write("\x1b[#P")
    loop.screen.set_terminal_properties(256)
    loop.screen.reset_default_terminal_palette()
    loop.run()
    loop.screen.write("\x1b[#Q")

but that made no difference. (It also didn't work if I used them in a bash script around running my app.)

I have also tried using OSC 104, as suggested by @luchr, with both ST and BEL finalizers, but those did not work either.

For now, I have to live with configuring 16 colors in my Urwid app and messing up the user's terminal on exit. This, though, really isn't a nice solution, so it would be much better if at least Urwid had a way for me to restore the user's palette before I exit my app.

danschwarz commented 9 months ago

Probably related to the way urwid cleans up on exit. I'll look into this when I can.

(It also didn't work if I used them in a bash script around running my app.)

This is interesting. I'm not sure why that approach would fail. Seems foolproof.

danschwarz commented 9 months ago

OK first thing I see- I find no evidence that iterm2 supports XTPUSHCOLORS/XTPOPCOLORS. Can you try this with a recent version of kitty or xterm and see if your bash script works with those terminals?

mfncooper commented 8 months ago

It doesn't seem to be documented, but it does look like iTerm2 implements this. I found mention of it somewhere on the web, but of course now I can't find that reference, so I looked in the code. You'll see it here:

https://gitlab.com/gnachman/iterm2/-/blob/master/sources/VT100CSIParser.m?ref_type=heads#L437 https://gitlab.com/gnachman/iterm2/-/blob/master/sources/VT100Terminal.m?ref_type=heads#L2382

(By the way, it seems the VT100 prefix on the filenames is historical - those files implement both XTerm and VT100 support.)

Using XTPUSHCOLORS and XTPOPCOLORS was just one experiment I tried in looking for a solution to the problem of the user's terminal being messed up on exiting my Urwid app. I don't need that particular solution to work. Using OSC 104 was another experiment that didn't work. By trying these, I was trying to preempt the most obvious "try this" suggestions. :-) What I really need is for Urwid to restore the original palette when an Urwid app exits. It seems like a reasonable thing to expect from the framework.

I can try Kitty as an experiment, but that wouldn't really solve anything for me, I'm afraid. My application is intended for distribution to end users. Since iTerm2 is the most commonly used Terminal replacement on Mac, I'd really like my app to work out of the box with that. With the kind of users that will be running my app, I can't really tell them that they should use a different terminal.

danschwarz commented 8 months ago

Note that there are two escape sequences that implement this functionality; kitty was first and has theirs documented on their website. Xterm was second and used their own codes. You are using the xterm codes. If iterm2 really does implement this feature, it's possible they use the original kitty sequence. You can try switching to that.

mfncooper commented 8 months ago

Yes, I'm aware of the two different sets of sequences. However, at the first iTerm2 code link I posted earlier, you'll see this code:

        case PACKED_CSI_COMMAND(0, '#', 'P'):
            result->type = XTERMCC_XTPUSHCOLORS;
            break;

        case PACKED_CSI_COMMAND(0, '#', 'Q'):
            result->type = XTERMCC_XTPOPCOLORS;
            break;

Even without looking at the macro being used, it's clear that the #P and #Q refer to the XTerm CSI and codes.

The Kitty sequences are quite different:

\<ESC>]30001\<ESC>\ # push onto stack \<ESC>]30101\<ESC>\ # pop from stack

Nevertheless, I gave them a try. Made no difference, though.

danschwarz commented 8 months ago

So I looked at the display cleanup code and it makes no attempt to restore palette colors at all. This makes sense, because the XTPUSHCOLORS/XTPOPCOLORS functionality didn't even exist before 2020, and is not widely implemented across terminals at this time.

So I have some questions:

What should urwid do?

We can implement XTPUSHCOLORS/XTPOPCOLORS support internally, assuming it has no negative side effects on terminals that don't support the commands which would be our best effort support for palette saving and restoration. I expect this will only work well on kitty and xterm for now, any support on other terminals will be a bonus.
We can call reset_default_terminal_palette before shutdown, which will reset the terminal palette to defaultt xterm colors. Not what you are looking for, but perhaps "good enough?" But this might be unexpected and unwanted behavior for other urwid app users. In that case, we'd need to gate that behavior behind a flag.
We can do nothing and leave it to app developers to write shell scripts like the one you've provided to save/restore the palette.

Why doesn't your script work?

Are you running tmux or screen multiplexers? Either of these apps might eat the XTPUSHCOLORS/XTPOPCOLORS command sequences and leave you with no functionalty.
Despite the code you pointed me to - and I reviewed - it might be buggy or disabled? Unlikely but possible, I suppose. Check with @gnachman?
Gremlins.

Note, you could call reset_default_terminal_palette in your own application. If that's good enough, we need not update urwid at all.

gnachman commented 8 months ago

@danschwarz iTerm2 supports XTPUSH/POPCOLORS in the beta version

mfncooper commented 8 months ago

@gnachman Aha! I see it in the Changelog for 3.5.0beta3. Thanks. I'll download beta 20 and give it a try.

mfncooper commented 8 months ago

Bingo! Using iTerm2 3.5.0beta20 with the following code works perfectly:

    loop.screen.write("\x1b[#P")
    loop.screen.set_terminal_properties(256)
    loop.screen.reset_default_terminal_palette()
    loop.run()
    loop.screen.write("\x1b[#Q")

Thanks @gnachman!

@danschwarz, might I suggest adding these two code sequences to escape.py for others' convenience? That would allow other people to easily do what I'm doing, without committing urwid to doing this kind of save / restore "magically" behind the scenes (in case, as you suggest, it might cause negative side-effects in some terminals). Functions could be added to the screen object too, but since it's only a one-liner to use the code sequences, that could easily wait until there's evidence of no harm being done.

danschwarz commented 8 months ago

If I add the sequence, I might just build the functions and hook them into the start/stop logic (perhaps conditionally)

mfncooper commented 8 months ago

I've just discovered that Windows (both Command Prompt and PowerShell) isn't keen on them. When it gets XTPUSHCOLORS, it echoes the command sequence and then the entire palette (all 256 colors, in my case); for XTPOPCOLORS, it echoes the command sequence. Weird. Conditionally would be good - at least don't issue them on Windows.

mfncooper commented 8 months ago

Okay, that's not quite right. It is actually reset_default_terminal_palette() that is causing the palette itself to be spit out on Windows. I don't know why I wasn't seeing that before, unless it's a side-effect of today's new Urwid 2.5.3 release. [Update: Not related to 2.5.3. I just hadn't noticed it before.] It happens with or without pushing / popping. Writing XTPUSHCOLORS and XTPOPCOLORS each cause their own code sequence to be spit out, but only that. The good thing is that on Windows, it appears I don't need to do any of this stuff (push / pop colors, set terminal properties, or reset the terminal palette) for things to work correctly, at least as far as I've found so far.

danschwarz commented 8 months ago

That's good. In any event, if I implement support, it's going to be in the posix branch not the windows branch of the display code.

wardi commented 8 months ago

clearly the best solution is to display all the colors and take a screenshot so we can restore them correctly :nauseated_face:

penguinolog commented 8 months ago

The good thing is that on Windows, it appears I don't need to do any of this stuff (push / pop colors, set terminal properties, or reset the terminal palette) for things to work correctly, at least as far as I've found so far.

Windows terminals is only partially compatible :-(. Part of sequences are the same and part are fully different.

urwid / urwid