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

Aliases are automatically expanded before the command is executed #5

Closed
romkatv opened this issue Dec 31, 2020 · 16 comments
Closed

Aliases are automatically expanded before the command is executed #5

romkatv opened this issue Dec 31, 2020 · 16 comments
Labels
enhancement New feature or request

Comments

@romkatv
Copy link

romkatv commented Dec 31, 2020

To reproduce:

% zsh -f
% git clone https://github.com/marlonrichert/zsh-hist.git
% source zsh-hist/zsh-hist.plugin.zsh
% alias x='echo hello'
% x

Expected: upon hitting Enter after entering the last command the following lines appear:

% x
hello

Actual: these lines appear instead:

%   echo hello
hello

Commit: bf89313.

@marlonrichert
Copy link
Owner

marlonrichert commented Dec 31, 2020

Yeah, I know. It’s a side effect of using $functions to format the code. Personally, I don’t mind. Do you?

As a workaround, you can disable automatic code formatting with

add-zle-hook-widget -d line-finish .hist.format.hook

@marlonrichert
Copy link
Owner

Actually, I just realized it’s a side effect of using eval. I’ll see if I can work around it/not use eval for this.

@marlonrichert
Copy link
Owner

setopt localoptions NO_aliases should probably do the trick. I’ll have to test it.

@romkatv
Copy link
Author

romkatv commented Jan 1, 2021

Personally, I don’t mind. Do you?

I've opened this issue because automatic alias expansion looks like an important enough effect to warrant a mention in the docs. It's an odd thing to bundle with formatting.

Note that alias expansion can change the effect of typed commands. For example:

% alias echo='echo foo'
% echo bar

Normally this will print "foo bar" but with automatic formatting it'll print "foo foo bar".

There are also more obscure cases where automatic formatting introduces side effects. For example, try executing the following command:

: }; touch foo; () { :

Without automatic formatting this command fails to parse and does nothing. With automatic formatting it touches file foo.

I don't mind any of it. Feel free to close this issue if or when you find it appropriate. Whatever you decide to do (including nothing at all) is fine with me.

@marlonrichert
Copy link
Owner

Thanks for the detailed motivation. After reading this, I definitely want to fix it. It threw me off guard, too, the first time the alias expansion happened to me.

There are also more obscure cases where automatic formatting introduces side effects. For example, try executing the following command:

: }; touch foo; () { :

Without automatic formatting this command fails to parse and does nothing. With automatic formatting it touches file foo.

Ah, that’s nasty. I’ll definitely have to see if I can fix that. It probably won’t come up often, but it looks like it can have seriously bad side effects.

marlonrichert added a commit that referenced this issue Jan 1, 2021
@marlonrichert
Copy link
Owner

Problem solved.

@romkatv
Copy link
Author

romkatv commented Jan 1, 2021

I took a look at the fix. There are still corner cases where enabling auto-formatting will change the effect of commands. Example:

% alias -g ';'=
% echo foo; echo bar

Without automatic formatting this prints "foo echo bar". With automatic formatting it prints "foo" and "bar".

@marlonrichert marlonrichert reopened this Jan 1, 2021
@marlonrichert
Copy link
Owner

Is there a real use case for a global alias like that?

@romkatv
Copy link
Author

romkatv commented Jan 1, 2021

Is there a real use case for a global alias like that?

I don't know. Does it matter?

It would be useful to provide the following guarantee:

  • Automatic formatting never changes the meaning of commands.

When you put it this way, it would appear very surprising if this guarantee weren't provided. Yet there are many violations of this guarantee right now. I've posted one example above. Here's another:

% emulate zsh -o extended_glob
% echo '
do
'

With automatic formatting the last command is rewritten to this:

echo '; do
'

The usual disclaimer applies: I don't have a personal preference one way or another. It's fine with me if you do nothing or change the code in any way you like.

@marlonrichert
Copy link
Owner

Hm, that last example is definitely a bug. I think I'll have to change my approach on how to implement this.

@marlonrichert
Copy link
Owner

marlonrichert commented Jan 1, 2021

Is there a real use case for a global alias like that?

I don't know. Does it matter?

Yes, that matters. It's a cost-benefit analysis: If there's no way this is going to bother anyone, then there's no value in fixing it. It's not worth it to spend time on things that don't benefit anyone.

@romkatv
Copy link
Author

romkatv commented Jan 1, 2021

It’s not something I want to argue about.

marlonrichert added a commit that referenced this issue Jan 1, 2021
…and do less custom formatting.

Fixes issue #5, except for the global `;` alias problem.
@marlonrichert
Copy link
Owner

OK, I pushed in another fix. This should take care of everything except the global ; alias thing.

@romkatv
Copy link
Author

romkatv commented Jan 1, 2021

The guarantee I’ve mentioned above is still not provided even in the environment without aliases. Unfortunately, I’ve run out of goodwill to post more test cases. You’ll probably find them easily if you look at the code with the intent of seeing the bugs in it.

@marlonrichert
Copy link
Owner

@romkatv Patches welcome.

@romkatv
Copy link
Author

romkatv commented Jan 1, 2021

I don’t know how to implement automatic parsing without changing the effect of entered commands. At the same time I think it crazy to use a plugin that can change the effect of typed commands in unspecified circumstances. I’ve opened this issue in case you find the information useful.

marlonrichert added a commit that referenced this issue Jan 2, 2021
marlonrichert added a commit that referenced this issue Jan 2, 2021
@marlonrichert marlonrichert added the enhancement New feature or request label May 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants