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

Enable shell alias expansion #27

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

michaek
Copy link
Contributor

@michaek michaek commented May 7, 2015

I'm not actually sure if this is a good idea, but it works for my purposes.

I say it's optimistic, because I didn't know what flags other shells were looking for, and I was hoping -O would be a consistent flag for shell options or that shell options would be the same across shells. That's not true for zsh, which uses -o http://linux.die.net/man/1/zsh

I'm opening the pull request to start a discussion. If it seems useful, I can see about getting this working with other shells.

Thanks for a gr-eat tool. :)

@michaek
Copy link
Contributor Author

michaek commented Oct 15, 2015

@mixu If this seems like something that could be useful, I'd be happy to detect other shells and set options appropriately. What do you think?

@mixu
Copy link
Owner

mixu commented Oct 16, 2015

Sorry for the long delay on this! I wasn't sure what the impact of this would be on people who use other shells and then didn't have time to test it properly. I think if it works in bash, zsh and maybe a third shell like fish or dash (don't really know since I haven't seen any stats on how popular different shells are??), it's probably good enough.

Rather than enabling this by default, it might be best to look at what process.env.SHELL contains and then only adding the extra flags when we know that that shell supports them. In any case thanks for the patch, I'll cherrypick 8926c950216e9bda14a16fda49f020f9288f0757 and clean it up a bit.

@mixu
Copy link
Owner

mixu commented Oct 16, 2015

closing; cherry-picked the commit

@mixu mixu closed this Oct 16, 2015
@mixu
Copy link
Owner

mixu commented Oct 16, 2015

Actually, after some testing I'm unable to get this to work at all on OSX - could you specify which environment this did work in? Just running the patch in master gets me this:

[gr @master] ./bin/gr gs

in ~/mnt/iv

/bin/bash: gs: command not found

spawn-task: "gs" exited with nonzero exit code: 127
[gr @master] zsh --version
zsh --version
zsh 5.0.7 (x86_64-apple-darwin14.0.0)
[gr @master] bash --version
bash --version
GNU bash, version 3.2.57(1)-release (x86_64-apple-darwin14)
Copyright (C) 2007 Free Software Foundation, Inc.

The closest I got to this working was writing out actual shell scripts in the form:

#!/bin/bash
shopt -s expand_aliases
gs

(ran with bash -i test.sh) and

#!/bin/zsh
source ~/.zshrc
setopt aliases
gs

ran with zsh test.sh.

@mixu mixu reopened this Oct 16, 2015
@mixu
Copy link
Owner

mixu commented Oct 17, 2015

hmm, looks like https://github.com/jdfreder/pur/blob/master/src/dealias.js does it by running a command to sync up the aliases and then does the actual alias replacing via regexp - that might work.

e.g. zsh -i -c 'alias' / bash -l -c 'alias -p' (OSX) / bash -i -c 'alias -p' (Linux). It's a bit hacky but could work.

@michaek
Copy link
Contributor Author

michaek commented Nov 18, 2015

I didn't see your messages until now! The fork is still working for me on OSX (on a different machine, no less). I'll look into what might be going on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants