awslabs / aws-shell

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

cd command isn't supported #76

Closed donnemartin closed 8 years ago

donnemartin commented 8 years ago

I sometimes switch directories, such as when working with S3.

Try running:

aws> !pwd
aws> !cd ..
aws> !pwd

Here's one potential way to address this, from SAWS:

https://github.com/donnemartin/saws/blob/master/saws/saws.py#L279-L309

jamesls commented 8 years ago

I'll go ahead and take a crack at this. It might be worth having this be a dot command to avoid edge cases such as:

aws> !cd /tmp/otherdir && cat file-in-other-dir.txt

vs.

aws> .cd /tmp/otherdir
aws> !cat file-in-other-dir.txt

This makes it clear to the user that cd'ing is "special" and can't be combined with other commands.

Thoughts?

quiver commented 8 years ago

In vim, !cd just spawns a command and does not affect the parent process(i.e. vim process), and :cd /path/to/somewhere changes vim process' working directory. In aws-shell world, this translates to !cmd and .Cmd respectively.

I'll second @jamesls approach.

donnemartin commented 8 years ago

I think that could work.

I am a little concerned that users might not find it discoverable/intuitive. We seem to have a growing number areas where the aws-shell diverges from aws-cli:

I've added some more specifics on the concerns in the linked tickets.

I think the lowest friction approach for shell commands is to handle the 'magic' stuff in the background without users having to remember they need to use ! or .. From a technical perspective this could be more difficult, especially if you factor in some of the chaining/piping like James' example.

Just my 2 cents :)

jamesls commented 8 years ago

Thanks @quiver. I'd forgotten that vim also does this. Just as another data point, ipython has similar semantics (!cd /tmp doesn't change your directory but %cd /tmp does). I know ipython has been discussed before, but given that ipython's magics (as well as sqlite's dot commands) was a main source of inspiration for aws-shell dot commands, it's worth considering what approach they've taken.

That being said, I do share the same concerns about this not being discoverable. I like @e1ven's suggestion of giving pointers to users to help them learn the differences. Perhaps we could do something similar here: if the user provides a command of the basic form "!cd /single/directory" we would could still execute the command (which would do nothing) but warn them that they might have meant ".cd /single/directory".

Also, as an aside, what if we add a .tutorial dot command? Then we check if it's the user's first time running the shell, we write out a message that tells them they can run .tutorial if they want a tour of the shell. This is where we could tell them about ! and dot commands, etc.

donnemartin commented 8 years ago

I like @e1ven's suggestion of giving pointers to users to help them learn the differences.

Also, as an aside, what if we add a .tutorial dot command?

Thumbs up for each! :+1: :+1:

donnemartin commented 8 years ago

ipython's magics (as well as sqlite's dot commands) was a main source of inspiration for aws-shell dot commands, it's worth considering what approach they've taken.

I agree, those are great reference points.

Here's an interesting thought that might cover all the bases, from the IPython Magics Docs:

If you have ‘automagic’ enabled (as it is by default), you don’t need to type in the single % explicitly for line magics; IPython will scan its internal list of magic functions and call one if it exists...The automagic system has the lowest possible precedence in name searches, so you can freely use variables with the same names as magic commands. If a magic command is ‘shadowed’ by a variable, you will need the explicit % prefix to use it: [Edit]: Added section after the ellipses.

Out of the box this allows me to type:

In [1]: cd foo
foo

Instead of: %cd foo.

jamesls commented 8 years ago

I like that idea. Implementation wise, it might get tricky. In order to have the dot commands have the lowest precedence, we need to be sure that there's nothing else of higher precedence. This likely will require checking the index to ensure there's no CLI services that start with the command name.

donnemartin commented 8 years ago

It does seem trickier implementation-wise.

This likely will require checking the index to ensure there's no CLI services that start with the command name.

I haven't thought this out completely yet so just brainstorming...another alternative (perhaps along the lines of LBYL vs EAFP) is to maybe to use subprocess.check_output. I think if check_output comes back with an exception then there isn't anything that handles the command, so we execute the dot command.

jamesls commented 8 years ago

I've been thinking about this, and I think the only way to get this working is to keep track of what the valid CLI commands are. I don't think we can rely on whether or not Popen gives us an error code. It's possible that Popen could have failed because the CLI command itself failed (syntax error, validation error, error from the service, etc).

This also is complicated if #87 is implemented (which btw is an idea that I think is awesome) because the valid set of CLI commands will change based on what hierarchy you're at.

I still think this is doable, but I'm thinking it's complicated enough that it's probably worth decoupling from the .cd feature. It's also more general than just the .cd dot command.

So I'd propose adding the .cd dot command, and creating an issue to support the automagic functionality that ipython has.

Does that seem reasonable?

donnemartin commented 8 years ago

Great points, works for me! :+1: