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

Add type annotations for screens.components subpackage #19

Open
wants to merge 61 commits into
base: order-files-by-size-diff-screen
Choose a base branch
from

Conversation

pbugnion
Copy link
Contributor

@pbugnion pbugnion commented Jun 3, 2018

This is an attempt at introducing greater rigour in how we use mypy types. This PR is mostly supposed to be a starting point for discussions, not necessarily useful in itself.

The sml_sync.screens.components subpackage now has full typechecking, including on all private methods. This subpackage heavily dependent on prompt_toolkit, which doesn't have type annotations, so I've written some stubs by just reading the code on GitHub.

This exercise has lead to some tidying of import structures (we were frequently importing classes from prompt_toolkit.layout.containers or prompt_toolkit.layout.controls, instead of directly from prompt_toolkit.layout, for instance), so it's been somewhat beneficial for the codebase. Nevertheless, type annotations do add complexity, so I'm open to deciding we don't want these.

If we do decide to merge this, we should test it in Travis.

To reproduce, run:

mypy sml_sync

Previously, this was just done in a simple tuple. That harmed readability.
This will let us render more than just the path.
This will help structure the file list, sizes and mtimes.
This allows embedding the table container in HSplits and VSplits.
Allows embedding table in {H,V}Splits.
@pbugnion pbugnion changed the title Add mypy annotations for screens.components subpackage Add type annotations for screens.components subpackage Jun 3, 2018
@pbugnion pbugnion mentioned this pull request Jun 4, 2018
@pbugnion pbugnion force-pushed the order-files-by-size-diff-screen branch from 6db9504 to 11bc160 Compare June 5, 2018 05:28
@pbugnion pbugnion force-pushed the order-files-by-size-diff-screen branch from 38eb083 to cbaae62 Compare June 16, 2018 10:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant