Skip to content

No highlighting for aliases containing function #803

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

Open
jtyr opened this issue Mar 19, 2021 · 14 comments
Open

No highlighting for aliases containing function #803

jtyr opened this issue Mar 19, 2021 · 14 comments

Comments

@jtyr
Copy link

jtyr commented Mar 19, 2021

If I define an alias:

alias xxx='ls -la'

and then type xxx, it highlights it just fine. But if I define alias containing a function:

alias xxx='f(){ ls -la; }; f'

and then type xxx, it doesn't highlight it as a known command.

@phy1729
Copy link
Member

phy1729 commented Mar 19, 2021

That's because when highlighting f(){ ls -la; }; f the second f is not recognised as a function (as it isn't defined yet). More or less a dup of #223.

For this case in particular, I'd move the function definition out of the alias.

@jtyr
Copy link
Author

jtyr commented Mar 19, 2021

Not sure if I understand correctly, but are you saying that you are analyzing the value of the alias somehow? Wouldn't be better to highlight any alias regardless its value?

@danielshahaf
Copy link
Member

Yes, we do, in order to highlight alias foo=not-installed in red.

@danielshahaf
Copy link
Member

I strongly recommend to change the alias's definition (or convert it to a function) as it'd DTWT in a SHORT_LOOPS context.

@danielshahaf
Copy link
Member

@jtyr alias foo='bar; baz' followed by repeat 2 foo will execute baz once, not twice. That's similar to the need for parentheses and do { … } while (0) guards in C preprocessor macros.

@danielshahaf
Copy link
Member

As to the issue, I don't think it's a duplicate of #223 as that issue doesn't involve function calls, only function definitions.

Cases such as repeat 'RANDOM % 2' f(){}; f mean we can't just highlight the call in green, either.

Perhaps we could highlight the call as indeterminate; compare #695.

@jtyr
Copy link
Author

jtyr commented Mar 20, 2021

I think it would be really useful to have a switch to enable/disable the validation of alias values. I would personally prefer to have any defined alias green even if the value would lead to non-existing command. Something like this:

ZSH_HIGHLIGHT_STYLES[validate_alias_value]='no'

@phy1729
Copy link
Member

phy1729 commented Mar 20, 2021

That's not really an option as the words after an alias may not be arguments.

alias foo='echo Hello &&'
foo echo World

or

alias foo='sudo -u foo'
foo echo bar

In both cases echo should be recognized as a command, but to do so we have to look into the alias. It's possible to ignore unknown-token highlights encountered when expanding an alias by adding another condition to

[[ $1 == unknown-token ]] && alias_style=unknown-token
(though the configuration option shouldn't live in ZSH_HIGHLIGHT_STYLES).

I second the strong recommendation to at least move the function definition out of the alias. It seems odd to redefine a function each time an alias is run aside from the other issues @danielshahaf pointed out.

Regarding highlighting function use after definition in one line, we could handle the usual case of not being in a loop or behind a conditional (including && or ||) and keep track of functions which we know will exist.

@jtyr
Copy link
Author

jtyr commented Mar 20, 2021

It would be really great if this behavior would be configurable in any way. I think it's more important to highlight perfectly valid alias as green than invalid alias as red.

@danielshahaf
Copy link
Member

@jtyr Please distinguish the desired behaviour from the proposed way to obtain it. Both @phy1729's and my ideas about changing the highlighting of function definitions would cause the alias not to be highlighted in red, without adding configuration knobs (which, if necessary, may depend on #362).

In any case, since consensus is that your alias should be changed, I'm not sure I'd accept that particular alias as a use-case/justification for any functional change. Hard cases make bad law.

Note: two distinct, but related, topics are being discussed: 1. Highlighting aliases as green iff they exist (as older zsh-syntax-highlighting did); 2. Highlighting function define-and-call.

@jtyr
Copy link
Author

jtyr commented Mar 22, 2021

I understand that if I externalize the function it would work just fine. But there is a reason why I have the function definition inside the alias and from shell point of view that's absolutely valid alias definition. I believe it would make sense to have an option to highlight every defined alias as green regardless the validity of its value.

@danielshahaf
Copy link
Member

But there is a reason why I have the function definition inside the alias and from shell point of view that's absolutely valid alias definition.

Yes, it's syntactically valid. That hasn't been questioned. As to the reason, I don't see what it is.

I believe it would make sense to have an option to highlight every defined alias as green regardless the validity of its value.

You have already said so and been replied to.

@baldricni
Copy link

That's because when highlighting f(){ ls -la; }; f the second f is not recognised as a function (as it isn't defined yet). More or less a dup of #223.

For this case in particular, I'd move the function definition out of the alias.

so in the roadmap, zsh-syntax-highlighting will not deliver this feature at all, i.e. support alias correct-highlight include function definition?

@danielshahaf
Copy link
Member

danielshahaf commented Aug 3, 2023 via email

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

4 participants