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

Noti doesn't expand alias arguments #11

Closed
simioes opened this issue Jan 19, 2016 · 8 comments
Closed

Noti doesn't expand alias arguments #11

simioes opened this issue Jan 19, 2016 · 8 comments
Milestone

Comments

@simioes
Copy link

simioes commented Jan 19, 2016

After installed, i tried with some created alias and it fails. If i run the command assigned to the alias it works as expected.

@variadico
Copy link
Owner

Could you post more information about this? I wasn't able to reproduce this.

I set this in my ~/.zshrc.

alias n="noti"

Then I ran source ~/.zshrc.

I was able to run n -V alex successfully.

@variadico variadico added the bug label Jan 26, 2016
@simioes
Copy link
Author

simioes commented Jan 26, 2016

I am not talking about set "noti" as an alias. For example, in osx:

alias speak_hello=' say "hello"'

noti speak_hello give an error. No detailed information, but the command in alias does not run.
Try that.

@variadico
Copy link
Owner

Ah, yes.

In v2, you now see more information.

image

Currently, noti assumes utility is an executable in your $PATH. It didn't evaluate your alias. Interestingly, the time command does evaluate aliases, so, yes: this is a bug.

@variadico
Copy link
Owner

Wait. I just realized time isn't /usr/bin/time in my shell. Running /usr/bin/time speak_hello also results in the same behavior as noti. I'm guessing the reason it worked before was because Zsh knew how to expand the alias when it ran its version of time.

Getting this behavior right seems tricky—and doing it in a way that works across different shells seems even more difficult.

Technically, you can get this to work in bash, version 3.2.57, like this.

shopt -s expand_aliases
alias speak_hello='say "hola"'
noti $(speak_hello)

I'd recommend to avoid doing this though. It seems like most apps can't handle expanding alias arguments.

@variadico variadico changed the title Does not work with alias Noti doesn't expand alias arguments Feb 6, 2016
@variadico variadico added enhancement and removed bug labels Feb 6, 2016
@variadico variadico mentioned this issue Sep 1, 2016
@variadico
Copy link
Owner

Hi, @vyasgiridhar, @chmaynard, and @simioes.

I think I might have finally found a solution to this, that's not too crazy. (No regexp.)

cd github.com/variadico/noti/cmd/noti
git fetch --all
git checkout expand-alias
go install

Then try running noti on some shell alias.

noti speak_hello

Can anyone else verify this works? I tried this on macOS Sierra with ZSH. It would be awesome if someone could test on other platforms and shells. ✌️

Thanks!!

@variadico variadico added this to the v2.3 milestone Sep 2, 2016
@vyasgiridhar
Copy link

It works on bash in Arch linux. 👍

@vyasgiridhar
Copy link

vyasgiridhar commented Sep 2, 2016

But can you change

cmd := exec.Command(shell, "-l", "-i", "-c", "which "+a)

to

cmd := exec.Command(shell, "-l", "-i", "-c",a)

would directly spawn the alias process.

variadico pushed a commit that referenced this issue Sep 8, 2016
@variadico
Copy link
Owner

Merged the alias change to dev, 1598292.

@vyasgiridhar, I liked your change, but I kept it the way I had it because executing the alias in the subshell caused arguments passed to aliases to be escaped.

$ which findhere 
findhere: aliased to find . -iname
$ noti findhere '*.go' # <- Expanding
$ noti findhere '\*.go' # <- Executing directly

Maybe we can optimize this later.

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

No branches or pull requests

3 participants