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

When printing options in two or more columns, sort them so they read naturally #3211

Closed
wants to merge 1 commit into from

Conversation

scotwk
Copy link

@scotwk scotwk commented Mar 21, 2018

When printing options in the help message the options are sorted so they read row by row. E.g.:

$ aws lambda foo

...

add-permission                           | create-alias                            
create-event-source-mapping              | create-function                         
delete-alias                             | delete-event-source-mapping             
delete-function                          | delete-function-concurrency             
get-account-settings                     | get-alias                               
get-event-source-mapping                 | get-function                            
get-function-configuration               | get-policy                              
invoke                                   | invoke-async                            
list-aliases                             | list-event-source-mappings              
list-functions                           | list-tags                               
list-versions-by-function                | publish-version                         
put-function-concurrency                 | remove-permission                       
tag-resource                             | untag-resource                          
update-alias                             | update-event-source-mapping             
update-function-code                     | update-function-configuration           
help                                    

But that's not how we naturally read columns of text. It should be sorted so we read one column, then the next. E.g.:

add-permission                           | list-aliases                            
create-alias                             | list-event-source-mappings              
create-event-source-mapping              | list-functions                          
create-function                          | list-tags                               
delete-alias                             | list-versions-by-function               
delete-event-source-mapping              | publish-version                         
delete-function                          | put-function-concurrency                
delete-function-concurrency              | remove-permission                       
get-account-settings                     | tag-resource                            
get-alias                                | untag-resource                          
get-event-source-mapping                 | update-alias                            
get-function                             | update-event-source-mapping             
get-function-configuration               | update-function-code                    
get-policy                               | update-function-configuration           
invoke                                   | help                                    
invoke-async                             |                                         

This commit implements the latter sorting behavior. It will work for differing values of ChoicesPerLine in case that gets changed.

@codecov-io
Copy link

codecov-io commented Mar 21, 2018

Codecov Report

Merging #3211 into develop will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #3211      +/-   ##
===========================================
+ Coverage    97.11%   97.11%   +<.01%     
===========================================
  Files          407      407              
  Lines        33530    33539       +9     
===========================================
+ Hits         32563    32572       +9     
  Misses         967      967
Impacted Files Coverage Δ
awscli/argparser.py 95% <100%> (+0.49%) ⬆️

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 a9a0112...9b89a00. Read the comment docs.

@scotwk
Copy link
Author

scotwk commented Apr 25, 2018

Anyone able to review this and comment on it?

@scotwk
Copy link
Author

scotwk commented May 11, 2018

@JordonPhillips - we chatted at pycon about this :)

@kdaily kdaily added enhancement feature-request A feature should be added or improved. needs-review This issue or pull request needs review from a core team member. labels Sep 28, 2021
thoward-godaddy pushed a commit to thoward-godaddy/aws-cli that referenced this pull request Feb 12, 2022
thoward-godaddy pushed a commit to thoward-godaddy/aws-cli that referenced this pull request Feb 12, 2022
* Revert "fix: Dependency and runtime mismatch at init now displays the correct error message (aws#3211)"

This reverts commit 17ef26d.

* Revert "feat: SAM Init interactive flow redesign (aws#2958)"

This reverts commit 307ded6.
@justindho
Copy link
Contributor

Hi @scotwk, thanks for the contribution!

Our team just put out a recent proposal in #6828 detailing improvements to the contribution process. We are working through open PRs and are trying to determine where this issue falls.

For feature requests, to invest the time in reviewing it, we would like to make sure the feature has wider community interest and are looking for 10 👍 votes on the issue #6974. In the meantime, we are going to set this PR as a draft. Once the 👍 threshold is met, we will move this PR to the “Implementation” stage and take a closer look.

@justindho justindho marked this pull request as draft May 19, 2022 22:55
@justindho justindho added community and removed needs-review This issue or pull request needs review from a core team member. labels May 19, 2022
@tim-finnigan
Copy link
Contributor

Thank you for creating this PR. I'm checking in as there hasn't been any activity here for a while. Now that CLI auto-prompt mode is available, you can see a list of services/commands in alphabetical order before encountering the help message.

If folks would still like to see this feature, we can continue tracking the request in #6974. But for the feature to be implemented the PR would have to account for changes in the argparser.py file, in addition to removing the line for supporting Python 2 as only Python 3+ is supported now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community enhancement feature-request A feature should be added or improved.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sort help message options by column instead of by row when an invalid command/subcommand is given
5 participants