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

Yaspin pipesafety issues #31

Open
sebageek opened this issue Mar 19, 2019 · 6 comments
Open

Yaspin pipesafety issues #31

sebageek opened this issue Mar 19, 2019 · 6 comments

Comments

@sebageek
Copy link
Contributor

Pipesafety of yaspin does not work for me (or I misunderstood the feature).

When I run yaspin and pipe the output into another program or into a file I'd expect to only see the output of write() calls and maybe of ok()/fail() or other functions with persistant text. Nevertheless I can also see the spinnerstate in programms like less or xxd.

import time
import yaspin

with yaspin.yaspin() as sp:
    sp.text = 'Test'
    time.sleep(0.25)
    sp.write('Hello world')

and when running this script

$ python pipesafe.py |xxd
00000000: 0de2 a08b 1b5b 306d 201b 5b4b 080d e2a0  .....[0m .[K....
00000010: 991b 5b30 6d20 5465 7374 1b5b 4b08 0de2  ..[0m Test.[K...
00000020: a0b9 1b5b 306d 2054 6573 741b 5b4b 080d  ...[0m Test.[K..
00000030: e2a0 b81b 5b30 6d20 5465 7374 1b5b 4b0d  ....[0m Test.[K.
00000040: 1b5b 4b48 656c 6c6f 2077 6f72 6c64 0a08  .[KHello world..
00000050: 0d1b 5b4b                                ..[K

This feels especially weird when using grep. In the case grep matches the spinner text, only the text from write() is displayed:

$ python pipesafe.py | grep Test
Hello world

$ python pipesafe.py | grep Test | xxd
00000000: 0de2 a08b 1b5b 306d 201b 5b4b 080d e2a0  .....[0m .[K....
00000010: 991b 5b30 6d20 5465 7374 1b5b 4b08 0de2  ..[0m Test.[K...
00000020: a0b9 1b5b 306d 2054 6573 741b 5b4b 080d  ...[0m Test.[K..
00000030: e2a0 b81b 5b30 6d20 5465 7374 1b5b 4b0d  ....[0m Test.[K.
00000040: 1b5b 4b48 656c 6c6f 2077 6f72 6c64 0a    .[KHello world.

In an attempt to fix this in an application I'm writing I created a yaspin-like interface, that maps write() to print() and ignores .text plus all other calls to the class. I then replace yaspin with that class if stdout is not a tty, but I'm not sure if this is the way to go for upstream.

@pavdmyt
Copy link
Owner

pavdmyt commented Mar 19, 2019

Hi @sebageek

Thanks for reporting this!

Can you please share python version and locale of the environment you're running at:

$ python -V
$ locale -a

Which operating system is used?

@sebageek
Copy link
Contributor Author

Sure thing! I'm using python from within a virtualenv for this. Operating System is Linux.

$ python -V
Python 3.5.2

$ locale -a
C
C.UTF-8
de_DE.utf8
en_AG
en_AG.utf8
en_AU.utf8
en_BW.utf8
en_CA.utf8
en_DK.utf8
en_GB.utf8
en_HK.utf8
en_IE.utf8
en_IN
en_IN.utf8
en_NG
en_NG.utf8
en_NZ.utf8
en_PH.utf8
en_SG.utf8
en_US.utf8
en_ZA.utf8
en_ZM
en_ZM.utf8
en_ZW.utf8
he_IL.utf8
POSIX

$ env|grep ^LC_
LC_PAPER=de_DE.UTF-8
LC_ADDRESS=en_US.UTF-8
LC_MONETARY=en_US.UTF-8
LC_NUMERIC=en_US.UTF-8
LC_ALL=en_US.UTF-8
LC_TELEPHONE=en_US.UTF-8
LC_IDENTIFICATION=en_US.UTF-8
LC_MEASUREMENT=de_DE.UTF-8
LC_TIME=de_DE.UTF-8
LC_NAME=de_DE.UTF-8

$ lsb_release -d
Description:	Ubuntu 16.04.5 LTS

$ echo $BASH_VERSION
4.3.48(1)-release

@pavdmyt
Copy link
Owner

pavdmyt commented Mar 23, 2019

I was able to look at this issue more thoroughly, especially at this part:

In an attempt to fix this in an application I'm writing I created a yaspin-like interface, that maps write() to print() and ignores .text plus all other calls to the class. I then replace yaspin with that class if stdout is not a tty, but I'm not sure if this is the way to go for upstream.

Initially, the idea behind "pipe safety" was the ability to direct the output of the program which uses yaspin into a file or another program in a way that doesn't crash the first one and somehow persist generated output (to be able to use it for logging, etc). Such ability was not the case for the most spinner libs at that time.

As I see, the main concern outlined by this issue is that output generated by yaspin is not cleared from various terminal codes in a case when stdout is not a tty.

The way to fix this probably is to add more checks like sys.stdout.isatty() when producing the output in Yaspin._compose_out and sanitize both terminal codes and spinner frames from the output.

@sebageek what do you think? I believe that, as of now, you have a closer familiarity with this issue, since you're actively utilizing yaspin.

@sebageek
Copy link
Contributor Author

So I think this depends on what the user expects from yaspin in case their program is used with a pipe (aka stdout is not a tty). I see the following options:

  1. Status information gets discarded, only output from write() is interesting (and maybe okay() etc.). This can be compared to the behaviour of ls from coreutils, where colors are only used if stdout is a terminal
  2. Status information gets written to stderr (if stderr is a tty), write() and other calls still write to stdout (so that the output can be still used in the pipe)
  3. The user is responsible themselves for taking appropriate action and disable yaspin themselves.

In my programs using yaspin I currently replace the Spinner class with a DummySpinner somewhat compatible to yaspin. This would be one approach but would need some tuning. In my code this looks something like this:

class DummySpinner:
    def __init__(self, *args, **kwargs):
        self.text = ''

    def write(self, *args, **kwargs):
        print(*args, **kwargs)

    def __getattr__(self, name):
        return self

    def __call__(self):
        pass

    def __enter__(self):
        return self

    def __exit__(self, *args, **kwargs):
        return False

With this I somewhat emulate Option 1, but it is a rather hacky approach. @pavdmyt do you think this is something that yaspin should handle? Or should it just stay with option 3? If the latter I'd at least change the documentation to reflect what pipesaftey means to yaspin. :)

@gaudenz
Copy link

gaudenz commented Aug 17, 2020

This seems to be still an issue. I would prefer option 1 from the comment above.

@woodruffw
Copy link

I'm seeing this same issue, similar environmental setup as #31 (comment): Python 3 (3.10), Linux (Ubuntu 20.04), running in a virtual environment.

I'm only familiar with a handful of terminal drawing libraries/toolkits, but here's the rough order of precedence that I've seen elsewhere:

  1. If stderr is open and isatty(stderr), use it
  2. If stdout is open and isatty(stdout), use it
  3. Do not emit any escape characters whatsoever if neither standard stream is a TTY

This is (IMO) the most reasonable set of defaults -- stderr is semantically reserved for this kind of output, and should nearly always be preserved over stdout for things like spinners. Similarly, there should absolutely never be any escape codes on a non-TTY stream: the absence of isatty is an explicit marker not to write escape codes.

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

4 participants