progressbar: fix regression with hidden bars

[CounterPillow]

The documentation of the progress bar states that the progress bar will output only the label of the bar if it is outputting to a file that is not a tty. However, this was broken a few versions ago, without the documentation being adjusted, and even a test added.

Restore this behaviour so we follow the documentation again, and adjust the test to match. Fixes #1138.

[davidism]

When/where was it removed? Why was it removed?

[CounterPillow]

It was removed somewhere between versions 6.7 and 7.0, as it works in 6.7 but doesn't in 7.0.

It appears to not have been removed on purpose, as git log 6.7...7.0 click/_termui_impl.py does not show any commits that mention removing this. It was likely a regression introduced by one of the refactorings.

I'd also argue that even if it was removed on purpose, it should still be kept, since:

1. the docs outline this behaviour
2. if you have a script running in the background that hangs during something that has a progress bar, it would probably be nice to know that it entered that progress bar at some point.

[CounterPillow]

git bisect points to 6126661 as the first bad commit.
nevermind, 61abbef is the first bad commit. Doesn't explain at all why it was removed.

Most peculiar observation: This + a test for it were that contributor's only commits to the repository, and that during October of last year, so this seems like a well-intentioned but ultimately wrong Hacktoberfest drive-by.

[sirosen]

I agree with the rationale for changing the behavior back to match the docs.

However, I'd really appreciate a second reviewer stepping in and agreeing before we merge. It's a behavior change between 7.0 and 7.1 . I think it's fair to call it a bugfix.

[davidism]

The PR was #863.

[sirosen]

Looking over the discussion again, it's less obvious that this should move forward as stated.
The use case, as I understand it, is having cmd-with-progressbar | parse-output-cmd work nicely without the progressbar interfering.

One way or another, the docs don't currently match the behavior.
If we change the behavior back and call this a "bug" in 7.0, things cohere well. If we change the documentation to reflect the new behavior and say this was an intentional behavior change between 6.x and 7.x, that holds together too.

The original change proposed in #863, writing to stderr instead of stdout, seems best.
That satisfies both use cases nicely. If you're running some background process, you should be collecting stderr, and if you pipe to another command, you only want to read stdout.
Command authors can easily enough add curl-style -s/--silent options -- the only thing at stake is what is the best "sane default" for the built-in progressbar.
But now I'm proposing even more changes in behavior, different from what it did in 6.x or 7.0, and I'm really really not sure that's a good idea.

[davidism]

I think restoring the behavior makes sense, this PR seems fine. I think the stderr default also makes sense, but I'm hesitant to change it at this point, especially since I'm trying to stop making changes to our progress bar and point users to tqdm for anything beyond basic tasks.