Implement fish completion by KiruyaMomochi · Pull Request #29549 · spack/spack

KiruyaMomochi · 2022-03-16T19:28:49Z

This pull request tries to implement completion generation script for fish.

fish._home_kyaru_spack.04-03-17.04-16-22_Trim.mp4

Because fish supports command descriptions, argparsewriter.py is changed to add help message to Command class.

To-do:

Remove 'spack' from _dest_to_fish_complete to avoid duplication.
Check compatibility with old versions of python.
Check compatibility with old versions of fish.
Add comments for new code and scripts.
spack activate should only show installed packages that have extensions.

However, it's still hard to implement more detailed completion for specs.
Further improvements may consider use spack itself to provide complete results.
For example, spack complete "spack -v --color" "a" give results always auto .

KiruyaMomochi · 2022-03-25T17:57:56Z

It finally works...

However, spec completion looks like a mess. If I have more time, I will consider implementing this feature in python instead, it will be not only cleaner but also easier to test.
_dest_to_fish_complete dict in commands.py also looks a bit creepy, but make it a class maybe too completed?
Also, we may need to refactor ArgparseWriter since tuples like optional now have 5 elements.

adamjstewart · 2023-07-10T21:55:53Z

Trying to use the tab completions in my day-to-day routine. Next issue I noticed:

> spack edit [TAB]

suggests packages as expected, but

> spack edit -c [TAB]

should suggest paths instead of packages. Same for all other spack edit flags. Not sure if it's possible to override things to such a specific level, as each flag needs a different base path.

At this point we're probably good enough for a first implementation. I'd rather get this in soon instead of waiting for perfection. Once #38804 is merged, I'll rebase one last time and start bugging people for review.

tgamblin · 2023-07-13T07:08:49Z

@adamjstewart:

However, a small number of Spack commands have very long or stylized help strings.

I think this is an artifact of people making functions for subcommands and using the doc strings as argparse help. I think sometimes the functions evolve and people forget that their doctoring is supposed to help argparse.

So, honestly, I would refactor either the way those commands generate their subcommand help text (i.e., don't use the docstring), or change the docstring. I think you went with the former in #38804 -- I'm fine w/that.

### Rationale While working on #29549, I noticed a lot of inconsistencies in our argparse help messages. This is important for fish where these help messages end up as descriptions in the tab completion menu. See #29549 (comment) for some examples of longer or more stylized help messages. ### Implementation This PR makes the following changes: - [x] help messages start with a lowercase letter. - [x] Help messages do not end with a period - [x] the first line of a help message is short and simple longer text is separated by an empty line - [x] "help messages do not use triple quotes" """(except docstrings)""" - [x] Parentheses not needed for string concatenation inside function call - [x] Remove "..." "..." string concatenation leftover from black reformatting - [x] Remove Sphinx argument docs from help messages The first 2 choices aren't very controversial, and are designed to match the syntax of the `--help` flag automatically added by argparse. The 3rd choice is more up for debate, and is designed to match our package/module docstrings. The 4th choice is designed to avoid excessive newline characters and indentation. We may actually want to go even further and disallow docstrings altogether. ### Alternatives Choice 3 in particular has a lot of alternatives. My goal is solely to ensure that fish tab completion looks reasonable. Alternatives include: 1. Get rid of long help messages, only allow short simple messages 2. Move longer help messages to epilog 3. Separate by 2 newline characters instead of 1 4. Separate by period instead of newline. First sentence goes into tab completion description The number of commands with long help text is actually rather small, and is mostly relegated to `spack ci` and `spack buildcache`. So 1 isn't actually as ridiculous as it sounds. Let me know if there are any other standardizations or alternatives you would like to suggest.

adamjstewart · 2023-07-13T19:20:12Z

I actually went with the latter in #38804. The spack help output is still extremely verbose, but it's on a separate line, so I use line breaks to only show the first line in fish tab completion. But I'm open to other suggestions for how to handle them.

adamjstewart

I think this is good enough for a first implementation, but want to get a second set of eyes on the code to make sure there isn't an obvious better way to do things.

adamjstewart · 2023-07-22T13:55:44Z

Thanks for the hard work on this @KiruyaMomochi!

* commands: provide more information to Command * fish: Add script to generate fish completion * fish: auto prepend `spack` command to avoid duplication * fish: impove completion generation code readability * commands: replace match-case with if-else * fish: fix optspec variable name prefix * fish: fix return value in get_optspecs * fish: fix return value in get_optspecs * format: split long line and trim trailing space * bugfix: replace f-string with interpolation * fish: compete more specs and some fixes * fish: complete hash spec starts with / * fish: improve compatibility * style: trim trailing whitespace * commands: add fish to update args and update tests * commands: add fish completion file * style: merge imports * fish: source completion in setup-env * fish: caret only completes dependencies * fish: make sure we always get same order of output * fish: spack activate only show installed packages that have extensions * fish: update completion file * fish: make dict keys sorted * Blacken code * Fix bad merge * Undo style changes to setup-env.fish * Fix unit tests * Style fix * Compatible with fish_indent * Use list for stability of order * Sort one more place * Sort more things * Sorting unneeded * Unsort * Print difference * Style fix * Help messages need quotes * Arguments to -a must be quoted * Update types * Update types * Update types * Add type hints * Change order of positionals * Always expand help * Remove shared base class * Fix type hints * Remove platform-specific choices * First line of help only * Remove unused maps * Remove suppress * Remove debugging comments * Better quoting * Fish completions have no double dash * Remove test for deleted class * Fix grammar in header file * Use single quotes in most places * Better support for remainder nargs * No magic strings * * and + can also complete multiple * lower case, no period --------- Co-authored-by: Adam J. Stewart <[email protected]>

### Rationale While working on spack#29549, I noticed a lot of inconsistencies in our argparse help messages. This is important for fish where these help messages end up as descriptions in the tab completion menu. See spack#29549 (comment) for some examples of longer or more stylized help messages. ### Implementation This PR makes the following changes: - [x] help messages start with a lowercase letter. - [x] Help messages do not end with a period - [x] the first line of a help message is short and simple longer text is separated by an empty line - [x] "help messages do not use triple quotes" """(except docstrings)""" - [x] Parentheses not needed for string concatenation inside function call - [x] Remove "..." "..." string concatenation leftover from black reformatting - [x] Remove Sphinx argument docs from help messages The first 2 choices aren't very controversial, and are designed to match the syntax of the `--help` flag automatically added by argparse. The 3rd choice is more up for debate, and is designed to match our package/module docstrings. The 4th choice is designed to avoid excessive newline characters and indentation. We may actually want to go even further and disallow docstrings altogether. ### Alternatives Choice 3 in particular has a lot of alternatives. My goal is solely to ensure that fish tab completion looks reasonable. Alternatives include: 1. Get rid of long help messages, only allow short simple messages 2. Move longer help messages to epilog 3. Separate by 2 newline characters instead of 1 4. Separate by period instead of newline. First sentence goes into tab completion description The number of commands with long help text is actually rather small, and is mostly relegated to `spack ci` and `spack buildcache`. So 1 isn't actually as ridiculous as it sounds. Let me know if there are any other standardizations or alternatives you would like to suggest.

* commands: provide more information to Command * fish: Add script to generate fish completion * fish: auto prepend `spack` command to avoid duplication * fish: impove completion generation code readability * commands: replace match-case with if-else * fish: fix optspec variable name prefix * fish: fix return value in get_optspecs * fish: fix return value in get_optspecs * format: split long line and trim trailing space * bugfix: replace f-string with interpolation * fish: compete more specs and some fixes * fish: complete hash spec starts with / * fish: improve compatibility * style: trim trailing whitespace * commands: add fish to update args and update tests * commands: add fish completion file * style: merge imports * fish: source completion in setup-env * fish: caret only completes dependencies * fish: make sure we always get same order of output * fish: spack activate only show installed packages that have extensions * fish: update completion file * fish: make dict keys sorted * Blacken code * Fix bad merge * Undo style changes to setup-env.fish * Fix unit tests * Style fix * Compatible with fish_indent * Use list for stability of order * Sort one more place * Sort more things * Sorting unneeded * Unsort * Print difference * Style fix * Help messages need quotes * Arguments to -a must be quoted * Update types * Update types * Update types * Add type hints * Change order of positionals * Always expand help * Remove shared base class * Fix type hints * Remove platform-specific choices * First line of help only * Remove unused maps * Remove suppress * Remove debugging comments * Better quoting * Fish completions have no double dash * Remove test for deleted class * Fix grammar in header file * Use single quotes in most places * Better support for remainder nargs * No magic strings * * and + can also complete multiple * lower case, no period --------- Co-authored-by: Adam J. Stewart <[email protected]>

adamjstewart added shell-support UI labels Mar 16, 2022

KiruyaMomochi force-pushed the fish-complete branch 4 times, most recently from df1c03c to 3309e3e Compare March 21, 2022 19:01

spackbot-app bot added commands utilities labels Mar 21, 2022

KiruyaMomochi force-pushed the fish-complete branch 4 times, most recently from ef1f246 to cc46fb2 Compare March 25, 2022 17:57

KiruyaMomochi marked this pull request as ready for review March 25, 2022 17:57

KiruyaMomochi force-pushed the fish-complete branch from 85080e0 to 1ce3119 Compare March 26, 2022 23:42

KiruyaMomochi added 15 commits March 28, 2022 02:22

commands: provide more information to Command

c2c4dfb

fish: Add script to generate fish completion

ca57ff5

fish: auto prepend spack command to avoid duplication

9f54dd6

fish: impove completion generation code readability

f973b8f

commands: replace match-case with if-else

ea81c00

fish: fix optspec variable name prefix

fb28e2f

fish: fix return value in get_optspecs

f8d11de

fish: fix return value in get_optspecs

c9ca383

format: split long line and trim trailing space

dc8d01f

bugfix: replace f-string with interpolation

1b1dfec

fish: compete more specs and some fixes

71e1913

fish: complete hash spec starts with /

2ec5781

fish: improve compatibility

c8b2369

style: trim trailing whitespace

c9da97d

commands: add fish to update args and update tests

8ff5544

This comment was marked as resolved.

Sign in to view

Remove platform-specific choices

7614b15

adamjstewart mentioned this pull request Jul 9, 2023

Standardize argparse help messages #38804

Merged

7 tasks

adamjstewart added 9 commits July 9, 2023 16:15

First line of help only

1b75ce2

Remove unused maps

19a42f6

Remove suppress

ba0bc8a

Remove debugging comments

b7f94fd

Better quoting

dd3fe6e

Fish completions have no double dash

7838da5

Remove test for deleted class

ab40c6f

Fix grammar in header file

47fe269

Use single quotes in most places

b8f2e32

This comment was marked as resolved.

Sign in to view

adamjstewart added 3 commits July 9, 2023 22:48

Better support for remainder nargs

aefc290

No magic strings

da8ddfd

* and + can also complete multiple

b221b8b

adamjstewart added 2 commits July 13, 2023 14:20

Merge branch 'develop' into fish-complete

a2b7be7

lower case, no period

c932b88

adamjstewart approved these changes Jul 13, 2023

View reviewed changes

adamjstewart merged commit 90ac0ef into spack:develop Jul 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement fish completion#29549

Implement fish completion#29549
adamjstewart merged 63 commits intospack:developfrom
KiruyaMomochi:fish-complete

KiruyaMomochi commented Mar 16, 2022 •

edited

Loading

Uh oh!

KiruyaMomochi commented Mar 25, 2022 •

edited

Loading

Uh oh!

This comment was marked as resolved.

This comment was marked as resolved.

adamjstewart commented Jul 10, 2023

Uh oh!

tgamblin commented Jul 13, 2023 •

edited

Loading

Uh oh!

adamjstewart commented Jul 13, 2023

Uh oh!

adamjstewart left a comment

Uh oh!

adamjstewart commented Jul 22, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

KiruyaMomochi commented Mar 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

KiruyaMomochi commented Mar 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as resolved.

This comment was marked as resolved.

adamjstewart commented Jul 10, 2023

Uh oh!

tgamblin commented Jul 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adamjstewart commented Jul 13, 2023

Uh oh!

adamjstewart left a comment

Choose a reason for hiding this comment

Uh oh!

adamjstewart commented Jul 22, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

KiruyaMomochi commented Mar 16, 2022 •

edited

Loading

KiruyaMomochi commented Mar 25, 2022 •

edited

Loading

tgamblin commented Jul 13, 2023 •

edited

Loading