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

bug: Importing colors from plumbum adds a trailing line to the output #659

Open
pawamoy opened this issue Oct 22, 2023 · 4 comments
Open

Comments

@pawamoy
Copy link

pawamoy commented Oct 22, 2023

It seems like merely importing colors from plumbum while setting FORCE_COLOR to supported values 1, 2, 3 and 4 adds a new line to a Python script output. Any other value (0, 5, 6, -1, "", "hello") do not trigger this behavior.

cd $(mktemp -d)
python -m venv .venv
. .venv/bin/activate
pip install plumbum rich

cat <<EOF >colors_without_plumbum.py
from rich.console import Console
console = Console()
console.print("[bold green]Hello[/]")
EOF

cat <<EOF >colors_with_plumbum.py
from plumbum import colors
from rich.console import Console
console = Console()
console.print("[bold green]Hello[/]")
EOF

cat <<EOF >colors.sh
echo "NOT forcing colors without Plumbum import"
python colors_without_plumbum.py | cat -n
echo "NOT forcing colors with Plumbum import"
python colors_with_plumbum.py | cat -n
echo "Forcing colors without Plumbum imports"
FORCE_COLOR=1 python colors_without_plumbum.py | cat -n
echo "Forcing colors with Plumbum imports"
FORCE_COLOR=1 python colors_with_plumbum.py | cat -n
EOF

bash colors.sh

plumbum

The % comes from my Zsh prompt. If we redirect the output to a file and open it, here are the contents:

�[1;32mHello�[0m
�[0m

Without the Plumbum import, contents are:

�[1;32mHello�[0m

So actually, rather than a newline, it seems to append a reset sequence. Now that the line is not empty, cat displays it. It has no newline character at the end so my prompt is directly printed after it.

  • System: Linux
  • Python: 3.11.5
  • Plumbum: 1.8.2
  • Rich: 13.6.0
@pawamoy
Copy link
Author

pawamoy commented Oct 22, 2023

Testing all values, and printing with cat -v:

for value in -1 0 1 2 3 4 5 "" yes no; do
    FORCE_COLOR="${value}" python colors_with_plumbum.py > fc$value.txt
done
% for fc in fc*; do echo $fc:; cat -v $fc; echo; done
fc0.txt:
^[[1;32mHello^[[0m

fc-1.txt:
^[[1;32mHello^[[0m

fc1.txt:
^[[1;32mHello^[[0m
^[[0m
fc2.txt:
^[[1;32mHello^[[0m
^[[0m
fc3.txt:
^[[1;32mHello^[[0m
^[[0m
fc4.txt:
^[[1;32mHello^[[0m
^[[0m
fc5.txt:
^[[1;32mHello^[[0m

fcno.txt:
^[[1;32mHello^[[0m

fc.txt:
^[[1;32mHello^[[0m

fcyes.txt:
^[[1;32mHello^[[0m

@pawamoy
Copy link
Author

pawamoy commented Oct 22, 2023

Hmmm, this is caused by atexit.register(_reset) in plumbum.colors. IIUC this is a guard rail to always reset everything (colors, styles, etc.) before giving back control to the parent process (the shell usually). Could maybe this guardrail not be added at import time, but deferred later, if Plumbum color functionality is actually used? Or could we maybe have a "trust" option (in the form of an environment variable) to disable the atexit registration? I'd prefer the first solution though, as it wouldn't require setting yet another environment variable.

@henryiii
Copy link
Collaborator

I was really worried about breaking prompts, but I think that's not as bad these days. In 3.13 Python will even fix the REPL for you if color leaks. Maybe we can remove it and have an opt-in function that adds the protection instead.

@pawamoy
Copy link
Author

pawamoy commented May 21, 2024

That would be great @henryiii, thanks 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants