Visitar URL original
regression with tty detection · Issue #1459 · pre-commit/pre-commit · GitHub
Skip to content

regression with tty detection #1459

Description

@ssbarnea

A recent regression was introduced via #1382 which broke detection on various conditions.

For example, on my MacOS host, only stdout and stdin report True on isatty() while stderr reports False, even if in reality there is no problem with sending ANSI to stderr.

Probably we should rely solely on stdout for the test or to assure that we respect it for each tty.

This issue is not happening with lots of people because only few of them use a method to colorize stderr, which apparently is what has a side-effect of marking it as a non tty.

Example of zsh snippet from ~/.zshrc that does this:

    export STDERRED_ESC_CODE="$fg[red]"
    if [[ "$STDERR_COLORIZE" != $$ ]]; then
      if (( $+commands[colorize] )); then
        exec 2>>(colorize $fg[yellow] $reset_color > /dev/tty &)
      else
        exec 2>>(while read line; do
          print ${fg[yellow]}$line$reset_color > /dev/tty; print -n $'\0'; done &)
      fi
      export STDERR_COLORIZE=$$
    fi

Once this runs you no longer gave a tty-enabled stderr.

Vast majority of CLI tools I know are using sys.stdout.isatty(), the only other exception I know is mypy. https://bixense.com/clicolors/ seems to support that approach too.

Regarding the never-ending struggle to control ANSI on/off behavior where each tool invented its own implementation and variables to configure behavior, it seems that someone realized that maybe is time to do something about it https://no-color.org/ :)

Activity

  1. asottile commented on May 15, 2020

    @asottile
    Member

    I cannot reproduce, what is your terminal emulator, shell, etc.

    no-color.org is flawed in that NO_COLOR= does not operate the same as the variable missing so I have rejected implementing it. there's PRE_COMMIT_COLOR which works nearly identical and is documented but that's off topic

  2. asottile commented on May 15, 2020

    @asottile
    Member

    I can't reproduce in zsh either

  3. ssbarnea commented on May 15, 2020

    @ssbarnea
    Author

    The terminal is irrelevant (same with iTerm or native Terminal) but the shell type and configuration is relevant: it will reproduce with zsh only if you add the mentioned script for colorizing the stderr console. That snippet is a subset of https://github.com/cehoffman/dotfiles/blob/master/zsh/config#L171-L193 which I adopted long time ago.

    If the ANSI output is to be produced on stdout, clearly the app needs to check sys.stdout.isatty() and not other stream. Same applies to stderr.

    A simple test of ``pre-commit 1>/dev/null` demonstrates that default stream is stdout, so we cannot really blame stderr stream for breaking stdout output.

    In fact there is an even easier way to reproduce wrong assumption:

    python3 -c "import sys; print(sys.stderr.isatty())" 2>/dev/null
    

    Once you redirect stderr to a file, it looses the tty properly, which is fine. Still, what is not fine is that the app stops producing ANSI only stdout.

  4. asottile commented on May 15, 2020

    @asottile
    Member

    well we can't pick stdout because it breaks windows, stderr works fine without your configuration so I'd say that's a your-configuration problem

  5. ssbarnea commented on May 15, 2020

    @ssbarnea
    Author

    I (effectively) beg to differ as this can easily happen with redirection anywhere. At the same time I do understand that you do no want to break the Windows hack too, so I will try to investigate the subject more in order too see how others sorted the issue.

  6. asottile commented on May 15, 2020

    @asottile
    Member

    could maybe use one on one and one on the other?

  7. asottile commented on Nov 27, 2020

    @asottile
    Member

    I've created an issue on git-for-windows: git-for-windows/git#2914

  8. benlindsay commented on Feb 28, 2022

    @benlindsay

    Have either of you been able to get colorized pre-commit output in VSCode since this issue was raised? On pre-commit fail, I see uncolorized text like this in the editor if I click on "Show Command Output":

    image

  9. asottile commented on Feb 28, 2022

    @asottile
    Member

    that looks like file output which wouldn't be in color anyway

  10. benlindsay commented on Mar 1, 2022

    @benlindsay

    Is there any way to get the output from pre-commit written to the terminal or somewhere that would preserve the coloration? It's a shame that all the colorization of the output gets lost if committing with the GUI rather than on the command line

  11. asottile commented on Mar 1, 2022

    @asottile
    Member

    pre-commit can't control how external tools render things -- perhaps report your bug against them?

  12. benlindsay commented on Mar 1, 2022

    @benlindsay

    Ah shoot, I was just following a chain of github issues and didn't notice I'd slipped out of VSCode repos and into the pre-commit repo. I'm not quite sure where to mention this but I'll find a better place. Thanks for all your work on pre-commit! It's a fantastic tool.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions