search

awslabs / aws-shell

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

Extract dot commands to separate object #46

Closed jamesls closed 8 years ago

jamesls commented 8 years ago

This extracts out the handling of dot commands to a separate object. This makes it easier to add dot commands, and makes testing dot commands easier.

As part of this work, I've also updated the .edit command to support the EDITOR env var, and fall back to platform specific defaults if the var is not defined.

Fixes #43 Fixes #40

Note, I will likely need to rebase this once https://github.com/awslabs/aws-shell/pull/45 is merged.

donnemartin commented 8 years ago

I haven't yet had a chance to more thoroughly review this, but have we looked into whether there's overlap with the built-in prompt_toolkit.buffer.Buffer:open_in_editor?

jamesls commented 8 years ago

Let me take a look at open_in_editor, haven't seen it before. It would be nice if there was something in prompt_toolkit that does this.

jamesls commented 8 years ago

After looking at open_in_editor it looks like it's similar to c-x e, or visual mode in bash where it can open a buffer in an editor, let you edit commands, and then execute those commands. The closest thing I see to what I'd need is _open_file_in_editor, but that doesn't appear to have any windows default editors (e.g notepad). It's looking like for now we'll have to keep this.

jamesls commented 8 years ago

I've gone through and merged the existing pull requests and rebased this PR against the latest master branch. I had to make a small change with the .history attribute to keep the tests passing. Should be good to look at.

cc @kyleknap

kyleknap commented 8 years ago

Looks fine. Just had some docstring/naming convention questions as I went through it. :ship:

jamesls commented 8 years ago

@kyleknap Thanks, I've incorporated your feedback.

kyleknap commented 8 years ago

Thanks!