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

Bash alias command substitution window resize bug #5417

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

lap1nou
Copy link

@lap1nou lap1nou commented Dec 21, 2024

Please review the following checklist.

  • Docstrings on all new or modified functions / classes
  • Updated documentation
  • Updated CHANGELOG.md (where appropriate)

Greetings,

I'm making this PR following a conversation with @xavierog (thanks to him) on Discord. I asked him for help regarding my TUI application.

My application was not resized correctly, the display was bugged and for example the Footer was disappearing, the thing is that I was running the Python application from a bash alias using command substitution (alias cmd='$(python3 tui-app.py)'), @xavierog discovered that the Textual driver was using shutil.get_terminal_size() to get the window size, but in that specific case, the function is failling because the command is ran from a bash command substitution.

This PR aim to fix that specific (and probably uncommon) situation, I ran all tests and here is the results:

================================== short test summary info ==================================
FAILED tests/snapshot_tests/test_snapshots.py::test_text_area_themes[css] - AssertionError: assert False
FAILED tests/snapshot_tests/test_snapshots.py::test_text_area_themes[monokai] - AssertionError: assert False
FAILED tests/snapshot_tests/test_snapshots.py::test_text_area_themes[dracula] - AssertionError: assert False
FAILED tests/snapshot_tests/test_snapshots.py::test_text_area_themes[vscode_dark] - AssertionError: assert False
FAILED tests/snapshot_tests/test_snapshots.py::test_text_area_themes[github_light] - AssertionError: assert False
FAILED tests/snapshot_tests/test_snapshots.py::test_text_area_wrapping_and_folding - AssertionError: assert False
FAILED tests/text_area/test_languages.py::test_setting_unknown_language - Failed: DID NOT RAISE <class 'textual.widgets._text_area.LanguageDoesNotExist'>
FAILED tests/text_area/test_languages.py::test_register_language - ImportError: cannot import name 'get_language' from 'textual._tree_sitter' (/root/textua...
FAILED tests/text_area/test_languages.py::test_update_highlight_query - AssertionError: assert 0 > 0
======== 9 failed, 3104 passed, 2 skipped, 4 xfailed, 1 warning in 259.55s (0:04:19) ========

I had the same results with a fresh Textual clone so I don't think my changes broke any tests.

CHANGELOG.md Outdated Show resolved Hide resolved
src/textual/drivers/linux_driver.py Outdated Show resolved Hide resolved
src/textual/drivers/linux_driver.py Outdated Show resolved Hide resolved
src/textual/drivers/linux_driver.py Outdated Show resolved Hide resolved
src/textual/drivers/linux_inline_driver.py Outdated Show resolved Hide resolved
@xavierog
Copy link
Contributor

@lap1nou: Looks good to me.

Questions for @willmcgugan and @darrenburns:

  1. What do you think re: the issue itself and the suggested fix?
  2. Beyond that fix: here, we have clear code duplication, calling for refactoring: either introducing an AbstractLinuxDriver class or turning that method into a standalone function with an fd argument that would go into e.g. drivers/_tty.py -- is this worth changing here?
  3. What about HeadlessDriver? I think it could be stripped down to:
    def _get_terminal_size(self) -> tuple[int, int]:
        return (80, 25) if self._size is None else self._size

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

Successfully merging this pull request may close these issues.

2 participants