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

Argparser auto-completion and colored help #4267

Closed
wants to merge 6 commits into from

Conversation

jnoortheen
Copy link
Member

@jnoortheen jnoortheen commented May 8, 2021

TODO:

  • update completer to handle options and nargs
  • add coloring

sub PRs:

I think it is better to split this PR

For community

⬇️ Please click the 👍 reaction instead of leaving a +1 or 👍 comment

@jnoortheen
Copy link
Member Author

added motivation here #4284

@daniel-shimon
Copy link
Contributor

Hey @jnoortheen - can you explain a bit which improvements this PR does? It's a bit hard to get from the code

@jnoortheen
Copy link
Member Author

jnoortheen commented May 17, 2021

It generates parser.add_argument parameters from the function signature. I was thinking of using Annotated to pass the metadata that will make it more clear. Is it Ok to add https://pypi.org/project/typing-extensions/ ?

@daniel-shimon
Copy link
Contributor

AFAIK all of xonsh's requirements need to be optional
So maybe it can be used optionally with fallbacking to Any

@jnoortheen
Copy link
Member Author

AFAIK all of xonsh's requirements need to be optional
So maybe it can be used optionally with fallbacking to Any

Yes I was also hesitant adding another dependency. But this comes from Python core repo. Also I dont find any other way to pass the metadata that passes type-checker.

@daniel-shimon
Copy link
Contributor

Since the type checker only runs in the CI, you can use this package and add it to the dev requirements file with a fallback to Any on import error. We don't really need the annotations in production anyway

@jnoortheen
Copy link
Member Author

Since the type checker only runs in the CI, you can use this package and add it to the dev requirements file with a fallback to Any on import error. We don't really need the annotations in production anyway

Ok that is better.

@jnoortheen jnoortheen changed the title Argparser improvement Argparser from function signature May 17, 2021
@jnoortheen jnoortheen force-pushed the argparser-improvement branch 10 times, most recently from dad2efd to 32fd260 Compare May 22, 2021 09:57
@codecov-commenter
Copy link

codecov-commenter commented May 22, 2021

Codecov Report

Merging #4267 (a864467) into main (db99b64) will increase coverage by 0.31%.
The diff coverage is 66.58%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4267      +/-   ##
==========================================
+ Coverage   57.13%   57.44%   +0.31%     
==========================================
  Files         138      136       -2     
  Lines       22178    22243      +65     
  Branches     4068     4096      +28     
==========================================
+ Hits        12671    12778     +107     
+ Misses       8454     8394      -60     
- Partials     1053     1071      +18     
Impacted Files Coverage Δ
xonsh/dirstack.py 30.35% <30.00%> (-3.83%) ⬇️
xontrib/voxapi.py 26.06% <50.00%> (+4.84%) ⬆️
xonsh/xontribs.py 52.67% <62.50%> (+5.15%) ⬆️
xonsh/completers/completer.py 58.47% <67.18%> (+11.63%) ⬆️
xonsh/cli_utils.py 71.50% <67.30%> (-21.23%) ⬇️
xonsh/xonfig.py 35.35% <73.68%> (-2.48%) ⬇️
xontrib/vox.py 44.30% <74.28%> (+44.30%) ⬆️
xonsh/completers/tools.py 86.99% <81.57%> (-2.42%) ⬇️
xonsh/aliases.py 54.56% <100.00%> (+0.62%) ⬆️
xonsh/completers/init.py 100.00% <100.00%> (ø)
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update db99b64...a864467. Read the comment docs.

@jnoortheen jnoortheen marked this pull request as draft May 27, 2021 10:15
@jnoortheen
Copy link
Member Author

after coloring it looks like this
image

@jnoortheen jnoortheen force-pushed the argparser-improvement branch 3 times, most recently from 09ce721 to 1810b1d Compare May 29, 2021 20:36
@jnoortheen jnoortheen marked this pull request as ready for review May 29, 2021 20:37
jnoortheen added a commit to jnoortheen/xonsh that referenced this pull request Aug 4, 2021
jnoortheen added a commit to jnoortheen/xonsh that referenced this pull request Aug 9, 2021
jnoortheen added a commit to jnoortheen/xonsh that referenced this pull request Aug 9, 2021
jnoortheen added a commit to jnoortheen/xonsh that referenced this pull request Aug 10, 2021
jnoortheen added a commit to jnoortheen/xonsh that referenced this pull request Aug 10, 2021
jnoortheen added a commit to jnoortheen/xonsh that referenced this pull request Aug 11, 2021
jnoortheen added a commit to jnoortheen/xonsh that referenced this pull request Aug 11, 2021
jnoortheen added a commit to jnoortheen/xonsh that referenced this pull request Aug 16, 2021
jnoortheen added a commit to jnoortheen/xonsh that referenced this pull request Aug 16, 2021
jnoortheen added a commit to jnoortheen/xonsh that referenced this pull request Aug 19, 2021
jnoortheen added a commit to jnoortheen/xonsh that referenced this pull request Aug 19, 2021
jnoortheen added a commit to jnoortheen/xonsh that referenced this pull request Aug 24, 2021
jnoortheen added a commit to jnoortheen/xonsh that referenced this pull request Aug 24, 2021
@jnoortheen
Copy link
Member Author

@daniel-shimon are there any other things to check?

daniel-shimon pushed a commit that referenced this pull request Aug 25, 2021
* feat: add colored help formatter and alias builder

closes #4284

* feat: add auto-completion support to argparser

* test: update test for completer

* fix: getting doc from params that have annotation

* refactor: use filter-function for checking alias completions

* doc: add discussion abount check for alias having parser

see discussion on
#4267

* type fix

* refactor: use function based completer

* test: fix failing argparser test

* docs: update news item

* update completion for argparser sub-commands to append_space

from comment on
#4267 (comment)

* docs: update docstring typo

* refactor: move inspect import to top

* feat: support option strings before positionals

and add env setting for showing completions for options by default

* test: update tests after adding new $ALIAS_COMPLETIONS_OPTIONS_BY_DEFAULT

* add suggested completion_context_parse fixture

* docs: add suggested doc for dispatch function

* refactor: use try/except for import of typing.annotated

* refactor: move complete_argparser_aliases to completers/aliases.py

* refactor: move argparser completer to its own module

* style:

* refactor: rename completer to not clash with argparse

* fix: expand option's descriptions

* fix: add completer/argparser to amalgam
@anki-code
Copy link
Member

The coloring is working for me:
image

@jnoortheen
Copy link
Member Author

@anki-code this PR is outdated. I will close it after the related PRs get closed.

please check the one in here #4437

@jnoortheen jnoortheen closed this Sep 3, 2021
@jnoortheen jnoortheen deleted the argparser-improvement branch September 3, 2021 03:49
jnoortheen added a commit to jnoortheen/xonsh that referenced this pull request Sep 18, 2021
jnoortheen added a commit to jnoortheen/xonsh that referenced this pull request Sep 18, 2021
jnoortheen added a commit to jnoortheen/xonsh that referenced this pull request Sep 26, 2021
this will resolve checking parser or alias. Now the alias can define how
it completes.

will solve
xonsh#4267 (comment)
@jnoortheen jnoortheen mentioned this pull request Sep 26, 2021
jnoortheen added a commit to jnoortheen/xonsh that referenced this pull request Oct 13, 2021
this will resolve checking parser or alias. Now the alias can define how
it completes.

will solve
xonsh#4267 (comment)
jnoortheen added a commit to jnoortheen/xonsh that referenced this pull request Oct 15, 2021
this will resolve checking parser or alias. Now the alias can define how
it completes.

will solve
xonsh#4267 (comment)
jnoortheen added a commit to jnoortheen/xonsh that referenced this pull request Oct 16, 2021
this will resolve checking parser or alias. Now the alias can define how
it completes.

will solve
xonsh#4267 (comment)
jnoortheen added a commit to jnoortheen/xonsh that referenced this pull request Nov 17, 2021
this will resolve checking parser or alias. Now the alias can define how
it completes.

will solve
xonsh#4267 (comment)
jnoortheen added a commit to jnoortheen/xonsh that referenced this pull request Nov 20, 2021
this will resolve checking parser or alias. Now the alias can define how
it completes.

will solve
xonsh#4267 (comment)
gforsyth pushed a commit that referenced this pull request Nov 23, 2021
* feat: create field to define alias-completer

this will resolve checking parser or alias. Now the alias can define how
it completes.

will solve
#4267 (comment)

* docs:

* fix: mypy error

* fix: rst qa

* style:
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.

None yet

5 participants