Do not let the GC close a borrowed stdout

[kdeldycke]

Complete the partial fix proposed in #3482 for #3449.

This PR reproduce the case highlighted in #3449 where a stream that is not managed by the pager machinery is closed by it unconditionally.

As extra bonuses:

• replace _SkipClose with utils.KeepOpenFile
• document KeepOpenFile
• eliminate a dead elif isinstance(stream, t.BinaryIO) branch as non-buffered text streams can be yielded as-is

This is a follow up of #3482 which is a follow up of #3405 which is a follow up of #1572.

CC @ChrisPappalardo @AndreasBackx @craigds

[davidism]

I realized there's already utils.KeepOpenFile which does all the dunder stuff. Let's get rid of _SkipClose, it's redundant. (Again noticing just how many wrappers we have.)

[davidism]

There also seems to be way too many context managers and levels of indirection for the pager machinery at this point. I'm not sure how we got here, but I'd sure love to refactor and simplify it somehow.

[kdeldycke]

I realized there's already utils.KeepOpenFile which does all the dunder stuff. Let's get rid of _SkipClose, it's redundant. (Again noticing just how many wrappers we have.)

TIL. Let me play a bit with utils.KeepOpenFile to see how it works...

[kdeldycke]

I realized there's already utils.KeepOpenFile which does all the dunder stuff. Let's get rid of _SkipClose, it's redundant. (Again noticing just how many wrappers we have.)

Seems to do the trick! Thanks for the hint! :)

[kdeldycke]

There also seems to be way too many context managers and levels of indirection for the pager machinery at this point. I'm not sure how we got here, but I'd sure love to refactor and simplify it somehow.

This part of Click is insane. The only thing that gives me confidence is the amount of collected cases from users that we transformed into unittests.

[kdeldycke]

I squashed the commits, this is ready to be reviewed. I'll go down the pager rabbit hole in other PRs.