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

Add basic support for aliases #3

Merged
merged 10 commits into from
Jul 12, 2020
Merged

Add basic support for aliases #3

merged 10 commits into from
Jul 12, 2020

Conversation

benjamineskola
Copy link
Contributor

@benjamineskola benjamineskola commented Jun 9, 2020

Description

Adds abbreviations to the dictionary when initially loading and when running the alias command. Closes #1.

Notes

Aliases are just a shortcut for writing functions, so if an alias is defined using the function command this won't catch it.

Previously the alias whitelist feature meant that by default no aliases would be checked, which doesn’t make sense if aliases are actually supported. Now the whitelist will only apply at all if it’s not empty: an empty whitelist will allow all aliases. (I also wondered if a blacklist would be useful, but that could be a separate PR if it’s a feature you’re interested in.)

@gazorby gazorby added the enhancement New feature or request label Jun 13, 2020
conf.d/abbr_tips.fish Outdated Show resolved Hide resolved
conf.d/abbr_tips.fish Outdated Show resolved Hide resolved
conf.d/abbr_tips.fish Outdated Show resolved Hide resolved
@gazorby
Copy link
Owner

gazorby commented Jun 13, 2020

Thanks for the PR, greatly appreciated!

@benjamineskola
Copy link
Contributor Author

I was a little concerned about an edge case: the alias builtin counts by the number of parameters, and not by whether the first parameter contains =. This means that if two parameters are provided and the first contains =, it creates a function with = in the name: alias foo=bar baz creates an alias called foo=bar which calls the command baz.

The code as I’ve written would actually create a tip for foo with the command bar baz, because it's much easier to check for the presence of = than to count the number of shell parameters (accounting for quote marks, spaces, backslashes, etc).

I thought this might lead to a problem if people defined aliases using the = character, however it seems like it’s actually impossible to execute a function with = in the name:

❯ foo=bar
fish: Unsupported use of '='. In fish, please use 'set foo bar'.

So in practice, I don’t think anybody’s going to be affected by this inconsistency.

conf.d/abbr_tips.fish Outdated Show resolved Hide resolved
@gazorby gazorby self-requested a review June 15, 2020 11:57
conf.d/abbr_tips.fish Show resolved Hide resolved
@gazorby
Copy link
Owner

gazorby commented Jun 15, 2020

Good catch !

I'm concerned about this too, since this will actually show a wrong tip, so it make more sense for me to prevent this unwanted behaviour by not creating the tip in this particular case.

@gazorby
Copy link
Owner

gazorby commented Jun 15, 2020

I just found another weird behaviour (but not caused by your PR) :

When you add an alias just before installing the fish plugin, the init hook won't create the tip for the previously defined alias because of being called in a spawned shell, and the alias does not seem to has been sourced (I think that logout an login again before installing should prevent this). But i think such a situation is rare enough to mention it in the Readme as a caution.

Reliably parsing multiple words in this case would add a lot of
complexity, so it's best simply to ignore it entirely and show no tip
than to show a wrong tip.

Multi-word aliases with the non-= syntax (e.g. `alias foo "bar baz"`)
continue to work as expected.
@benjamineskola
Copy link
Contributor Author

There's another slightly weird behaviour which I've come across in testing.

Defining an alias will add its tips to the variables, which are universal. But aliases aren't automatically saved. So when the shell terminates, the tips will still exist even if the aliases don't.

It's not a problem for abbreviations, since by default abbreviations are saved in universal variables and therefore will persist when the shell closes.

conf.d/abbr_tips.fish Show resolved Hide resolved
@gazorby
Copy link
Owner

gazorby commented Jun 20, 2020

This is weird as users could also see wrong tips because of this. A quick fix would be to prefix aliases keys with something like a__key before adding them to the variable, this way we could know whether or not the tip come from an alias and check for its existence (using functions -q) before printing to the terminal. In case the alias does not exists (when it has been created in a previous shell), the matching tip would be erased and nothing would be printed.

conf.d/abbr_tips.fish Outdated Show resolved Hide resolved
@gazorby gazorby merged commit b02b13b into gazorby:master Jul 12, 2020
@gazorby
Copy link
Owner

gazorby commented Jul 12, 2020

I think it's all good, thanks for your contribution @benjamineskola !

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

Successfully merging this pull request may close these issues.

[Feature] : Add support for aliases
2 participants