awslabs / aws-shell

An integrated shell for working with the AWS CLI.
Apache License 2.0
7.17k stars 772 forks source link

Implement #26: Add toolbar menu. #45

Closed donnemartin closed 8 years ago

donnemartin commented 8 years ago

Changes

Includes automated tests.

Also tested manually on:

Note: The base of this branch is from feature/28-auto-suggestions.

Unfortunately, you will also see changes from feature/28-auto-suggestions (https://github.com/awslabs/aws-shell/pull/44), although the changes are relatively minor.

jamesls commented 8 years ago

Awesome, I'm taking a look at this now.

jamesls commented 8 years ago

:shipit: Looks good to me. I just had one question about adding back the line between the docs and the input container.

donnemartin commented 8 years ago

Thanks for the review and feedback James.

About the separator line, I'll take a closer look either later this evening or tomorrow morning.

Early on I had noticed cosmetic issues when adding the toolbar. I believe the problem was the help and toolbar were overlapping, perhaps not enough vertical space was being allocated. If I recall correctly the change in question fixed that issue, although it seems I was confused when I added the comment and I also failed to notice the missing line above help pane.

I did upgrade prompt_toolkit to 0.52 which might have fixed the cosmetic issue by itself.

I'll try to sort this out and add the line back before merging in the changes.

donnemartin commented 8 years ago

I placed the original line back above the help and confirmed there are no cosmetic issues on both Mac and Windows.

I think it would be nice to add an additional line below the help text, otherwise the spacing is a little tight.

No additional line below help:

Imgur

Additional line below help:

Imgur

donnemartin commented 8 years ago

If no other issues, I'll try to resolve the conflicts and merge tomorrow morning.

I'll add the other feedback on my TODO.

jamesls commented 8 years ago

:+1: Sounds good to me.