-
Notifications
You must be signed in to change notification settings - Fork 37
Fix bash quoting, add type hints, code cleanup #196
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
Open
pmhahn
wants to merge
20
commits into
iterative:main
Choose a base branch
from
pmhahn:quoting
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks Signed-off-by: Philipp Hahn <[email protected]>
> shtab/__main__.py:8: error: "main" does not return a value (it only ever returns None) Signed-off-by: Philipp Hahn <[email protected]>
ArgumentParser._get_optional_actions() will always return instances of `Action` and never `SUPPRESS`, which is a `str`. Signed-off-by: Philipp Hahn <[email protected]>
Also handle input redirection. Also handle other file descriptors than STDOUT and STDERR.
No backslash at the end of the line is required after && and ||. Signed-off-by: Philipp Hahn <[email protected]>
https://www.shellcheck.net/wiki/SC2004 Signed-off-by: Philipp Hahn <[email protected]>
Declare some variables as integers, so `var+=1` can be used. Signed-off-by: Philipp Hahn <[email protected]>
https://www.shellcheck.net/wiki/SC2086 Signed-off-by: Philipp Hahn <[email protected]>
https://www.shellcheck.net/wiki/SC2207 Signed-off-by: Philipp Hahn <[email protected]>
Signed-off-by: Philipp Hahn <[email protected]>
SC2199 (error): Arrays implicitly concatenate in [[ ]]. Use a loop (or explicit * instead of @). Signed-off-by: Philipp Hahn <[email protected]>
Required for: - `typing.Protocol` - walrus operator Using `collections.abc` instead of `typing` requires 3.9 for type-checking only! Signed-off-by: Philipp Hahn <[email protected]>
typing.List[…] -> list[…] typing.Dict[…] -> dict[…] typing.Union[…] -> … | … typing.Optional[…] -> … | None Signed-off-by: Philipp Hahn <[email protected]>
Signed-off-by: Philipp Hahn <[email protected]>
Signed-off-by: Philipp Hahn <[email protected]>
Also get rid of `cases` changing type from `list[str]` to `str`. Signed-off-by: Philipp Hahn <[email protected]>
Use list-comprehension instead of sum(); it's easier to grasp and faster. Signed-off-by: Philipp Hahn <[email protected]>
Signed-off-by: Philipp Hahn <[email protected]>
Drop Python 3.7 support as the Walrus-operator only available since 3.8. Signed-off-by: Philipp Hahn <[email protected]>
Signed-off-by: Philipp Hahn <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I found your project while looking for how to get bash completion automatically from
ArgumentParser.
Sadly Ubuntu 24.04 ships a broken version, so I had a look at your GitHub repository. Great work. Sadly there are several pending open issues and other PRs, so I hope you will find some time to work on this again soon.list[…]
instead oftyping.Lits[…]
and such.mypy
found some minor issues, which I also fixed. See individual commits at the front.shellcheck
found many issues, mostly related to missing quotes. I fixed the code, which generates those. See individual commits in the middle.__init__.py
is quiet large and contains mixed Python/shell code. Maybe it might be a good idea to move the shell-code to separate files; see last commit.If you prefer I can send you smaller pull requests only doing one thing at a time. But before I spend more time on this, I'd like to get your feedback if such changes are preferred or not.
Footnotes
For complete type annotations
typing.Protocol
from Python 3.8 (EoL last year) is needed. Therefor I bumped the minimum supported version to 3.8 not that 3.14 got released. By usingfrom __future__ import annotations
andif TYPE_CHECKING:
the code still runs with 3.8. Usingfrom collections.abc import …
instead offrom typing import …
works only since Python 3.9 (EoL this month). Raising the minimum version to3.9
thus might be preferred. ↩