python / cpython

The Python programming language
https://www.python.org
Other
63.12k stars 30.22k forks source link

os.get_terminal_size() should check stdin as a fallback #59046

Open 80036ac5-bb84-4d39-8416-02cd8e51707d opened 12 years ago

80036ac5-bb84-4d39-8416-02cd8e51707d commented 12 years ago
BPO 14841
Nosy @vstinner, @blueyed, @eryksun, @grantjenks
PRs
  • python/cpython#12697
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields: ```python assignee = None closed_at = None created_at = labels = ['type-bug', '3.8', '3.9', '3.10', 'extension-modules', 'library'] title = 'os.get_terminal_size() should check stdin as a fallback' updated_at = user = 'https://bugs.python.org/Arfrever' ``` bugs.python.org fields: ```python activity = actor = 'eryksun' assignee = 'none' closed = False closed_date = None closer = None components = ['Extension Modules', 'Library (Lib)'] creation = creator = 'Arfrever' dependencies = [] files = [] hgrepos = [] issue_num = 14841 keywords = ['patch'] message_count = 7.0 messages = ['160988', '222266', '222272', '222527', '323836', '323847', '339495'] nosy_count = 7.0 nosy_names = ['vstinner', 'blueyed', 'Arfrever', 'zbysz', 'denilsonsa', 'eryksun', 'grantjenks'] pr_nums = ['12697'] priority = 'normal' resolution = None stage = 'patch review' status = 'open' superseder = None type = 'behavior' url = 'https://bugs.python.org/issue14841' versions = ['Python 3.8', 'Python 3.9', 'Python 3.10'] ```

    80036ac5-bb84-4d39-8416-02cd8e51707d commented 12 years ago
    $ stty size | cat
    46 157
    $ python3.3 -c 'import os; print(os.get_terminal_size())' | cat
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
    OSError: [Errno 22] Invalid argument

    Redirection to less are often used.

    A proposed patch was attached by Victor Stinner in issue bpo-13609.

    83d2e70e-e599-4a04-b820-3814bbdb9bef commented 10 years ago

    bpo-13609 is closed fixed so can this also be closed? I've tried to reproduce this on Windows with the help of unxutils but it didn't want to know, sorry :(

    80036ac5-bb84-4d39-8416-02cd8e51707d commented 10 years ago

    This bug is still reproducible in Python 3.4 and 3.5.

    232d7ad7-7660-4d4e-a54f-74929b9d96b9 commented 10 years ago

    FYI, "tput" tool can find out the dimensions even when both stdin and stdout are redirected. It can't, however, if all stdin, stdout and stderr are redirected. Python should probably implement a similar logic.

    $ echo | stty size
    stty: standard input: Inappropriate ioctl for device
    $ echo | tput cols | cat
    223
    $ echo | tput cols 2>&1 | cat 
    80

    (tested under Linux)

    c8b49e28-25bb-49e2-b713-2af027a19b3f commented 6 years ago

    I asked on the ncurses maintainers email list about their logic and was pointed to tty_settings.c which checks:

    1. stderr
    2. stdout
    3. stdin
    4. open('/dev/tty', 'r+')

    I don't know a cross-platform way to check #4 but I think #1-3 are a reasonable change to shutil.get_terminal_size().

    The current logic checks only stdout. I'd like to amend that to try stderr, stdout, and stdin after checking the COLUMNS and LINES env vars. So the new logic would be:

    1. Check COLUMNS and LINES env vars (for overrides)
    2. Check os.get_terminal_size(stderr)
    3. Check os.get_terminal_size(stdout)
    4. Check os.get_terminal_size(stdin)
    eryksun commented 6 years ago

    The Windows implementation shouldn't map file-descriptor values 0, 1, and 2 to the process standard handles. fileno(stdout) may not even be 1 in some cases. It should use handle = _get_osfhandle(fd), which is more generally correct and not limited to hard-coded defaults.

    Regarding this issue, the docs should clarify that the Windows console's input buffer cannot be used to get the screen buffer size. Typically the input buffer is STDIN_FILENO (0) or opened via "CONIN$" or "CON" (read mode). Thus there is no point to checking stdin in Windows since it will always fail.

    I agree that the default behavior should be extended to check a platform-dependent tty/console device. In Windows this is fd = _open("CONOUT$", _O_RDWR).

    1f7c169c-a6ad-45ae-b614-0d40d962c776 commented 5 years ago

    Created a PR at https://github.com/python/cpython/pull/12697.

    It prefers stdin and stderr over stdout.

    I think stdin is most likely connected to a terminal, and wondered why ncurses uses stderr/stdout first, and only then stdin. stdin handling was added there in https://github.com/mirror/ncurses/commit/aa70bf3c#diff-10ad6ea20599ac9258757354665b80cbR1295, and it looks just like an oversight maybe - it is also not that critical in C I guess.