awslabs / aws-shell

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

Update aws-shell to work with prompt_toolkit==0.54.0 #78

Closed jamesls closed 8 years ago

jamesls commented 8 years ago

I also needed to update one of the unit tests for the F10 keypress. On 0.54.0, an IOError is no longer being raised so I'm just testing the is_exiting flag directly.

Also did a bit of manual testing to ensure the UI looked correct. All seems well.

Closes #60.

cc @donnemartin

donnemartin commented 8 years ago

I'm seeing cosmetic issues when you use a toolbar shortcut that forces a cli redraw:

Imgur Imgur

The cosmetic issues go away once you hit enter or type over it.

I've tried several calls to cli redraws, cli reset, buffer reset, renderer reset, with no luck.

The problem goes away when you hit return, so that might be a [heavy handed] way to solve this? That is, as long as we don't pass a blank command to aws-cli, or you get the following:

aws>
usage: aws [options] <command> <subcommand> [<subcommand> ...] [parameters]
To see help text, you can run:

  aws help
  aws <command> help
  aws <command> <subcommand> help
aws: error: the following arguments are required: command

Would be cool to find a better solution. With this bug, I'm not sure forcing the update to 0.54 provides a lot of benefit.

Something else that might help is if we add instructions to use virtualenv like what we have in the SAWS docs.

jamesls commented 8 years ago

Good catch. I agree that there's not a whole lot of benefit in upgrading to 0.54 right now, especially with these cosmetic issues.

I'm going to go ahead and close out this pull request. I'll reopen once we want to upgrade to 0.54.0. I also update the readme on instructions for using a virtualenv. That will help with people that are seeing issues with the latest 0.54.0 being installed.

donnemartin commented 8 years ago

Sounds good :+1:

jonathanslenders commented 8 years ago

Hi @donnemartin @jamesls,

The redraw issues have been solved in this commit: https://github.com/jonathanslenders/python-prompt-toolkit/commit/6be54d6c140bfe2ce5d626de94860daa97172132 (not released yet.)

It was caused by the exception that's raised in stop_input_and_refresh_cli which was not handled correctly. However, actually, we don't need to raise that exception. We also don't need to quit the self.cli.run() loop in order to switch between completion types or key bindings.

KeyBindingManager for instance, takes an enable_vi_mode parameter. This can be a boolean, like it is right now, but it can be an expression that is completely dynamic. In prompt_toolkit, that's what we call a Filter. It should be something like this:

from prompt_toolkit.filters import Condition
...
enable_vi_mode = Condition(lambda cli: get_enable_vi_bindings())

For the completions, it's similar. If I find time after the next release, I'll open a pull request with these changes.

Cheers, Jonathan

donnemartin commented 8 years ago

@jonathanslenders awesome thank you for the help! I'll try to free up in the next week or two and see if I can clean this up by not exiting the run loop. Curious, do you have a rough idea when the next release of prompt_toolkit might go out with https://github.com/jonathanslenders/python-prompt-toolkit/commit/6be54d6c140bfe2ce5d626de94860daa97172132?

jonathanslenders commented 8 years ago

HI @donnemartin,

I really hope to push a new release before the end of this month, or early next month. There are still a few small bugs I'll have to fix. But I'm almost there. I'll keep you posted.