8000 CLI: `click` -> `argparse` by tony · Pull Request #830 · tmux-python/tmuxp · GitHub
[go: up one dir, main page]
Skip to content

CLI: click -> argparse#830

Merged
tony merged 9 commits intomasterfrom
vanilla-argparse
Oct 9, 2022
Merged

CLI: click -> argparse#830
tony merged 9 commits intomasterfrom
vanilla-argparse

Conversation

@tony
Copy link
Member
@tony tony commented Oct 2, 2022
  • Remove click and sphinx-click
  • Add sphinx-argparse
  • Use argparse
  • Remove completion, for now

See also: vcs-python/vcspull#400

@tony tony mentioned this pull request Oct 2, 2022
Merged
@tony tony force-pushed the vanilla-argparse branch from a8070e0 to 3f6f951 Compare October 2, 2022 17:19
@codecov
Copy link
8000
codecov bot commented Oct 2, 2022 •
edited
Loading

Codecov Report

Merging #830 (afb7af3) into master (0a6e23a) will decrease coverage by 0.42%.
The diff coverage is 71.77%.

@@            Coverage Diff             @@
##           master     #830      +/-   ##
==========================================
- Coverage   75.45%   75.03%   -0.43%     
==========================================
  Files          19       19              
  Lines        1434     1602     +168     
  Branches      331      359      +28     
==========================================
+ Hits         1082     1202     +120     
- Misses        265      290      +25     
- Partials       87      110      +23
Impacted Files Coverage Δ
docs/conf.py 58.94% <ø> (ø)
src/tmuxp/cli/utils.py 63.50% <58.40%> (-15.67%) ⬇️
src/tmuxp/cli/load.py 67.34% <70.14%> (+1.42%) ⬆️
src/tmuxp/cli/freeze.py 71.27% <72.22%> (-0.16%) ⬇️
src/tmuxp/cli/convert.py 82.35% <77.27%> (+0.21%) ⬆️
src/tmuxp/cli/edit.py 76.92% <80.00%> (+6.92%) ⬆️
src/tmuxp/cli/import_config.py 79.36% <84.21%> (+19.90%) ⬆️
src/tmuxp/cli/shell.py 85.45% <87.80%> (-3.12%) ⬇️
conftest.py 87.50% <100.00%> (ø)
src/tmuxp/cli/debug_info.py 100.00% <100.00%> (ø)
... and 5 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@tony tony force-pushed the vanilla-argparse branch 2 times, most recently from 9e71c62 to 30e5183 Compare October 4, 2022 02:02
@piotr-dobrogost
Copy link
piotr-dobrogost commented Oct 4, 2022

Why are you replacing click with argparse?

@tony
Copy link
Member Author
tony commented Oct 5, 2022

@piotr-dobrogost

  • click:
    • Pros:
      • Decorators
      • Completions
      • Testing is ok
    • Cons:

      • Extra dependency

        • This can cause headaches downstream on linux packages

      • Completion cumbersome in various ways (context sharing, testing, lack of documentation)

      • Passing contexts is cumbersome and sometimes buggy

      • Click's documentation

        Basic patterns and context passing aren't covered

      • Our docs: sphinx-click is hard to work with / control

        It automatically create 10000 s a section that's the exact name of the header that the user would want to write by hand

  • argparse
    • Pros:

      • Well maintained

      • New python versions are getting new features (3.9 and 3.10, when our minimum version is bumped, automatically gets these)

      • Works out of the box

      • Our docs: sphinx-argparse allows very fine-grained control

      • Has third party completion tools

        argcomplete, shtab

      • Easy for downstream package maintainers

      • Easy to test

    • Cons:
      • Very manual (this could also be considered a plus)
      • Parser creation and running happens in a separate function for each command
      • Completion is a separate package - so we end up needing a new package anyway - but it can be optional

@tony tony force-pushed the vanilla-argparse branch 17 times, most recently from b8279e5 to c0c9fdc Compare October 9, 2022 22:39
@tony tony force-pushed the vanilla-argparse branch from c0c9fdc to ed15da8 Compare October 9, 2022 22:53
@tony tony force-pushed the vanilla-argparse branch from ed15da8 to afb7af3 Compare October 9, 2022 22:54
@tony tony merged commit 9813ce2 into master Oct 9, 2022
@tony tony deleted the vanilla-argparse branch October 9, 2022 23:01
tony added a commit that referenced this pull request Oct 9, 2022
@tony
docs(CHANGES): Note #830 argparse change
e44d181
tony added a commit that referenced this pull request Oct 9, 2022
@tony
v1.17.0a0 (click -> argparse, from #830)
< 7119 div class="pr-1 d-md-inline-block d-none">
8bf5b94
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tony @piotr-dobrogost
0