MichaelAquilina / zsh-you-should-use

📎 ZSH plugin that reminds you to use existing aliases for commands you just typed
GNU General Public License v3.0
1.46k stars 43 forks source link

Using hardcore mode will break cp ../../ with ohmyzsh directory aliases #33

Open eigan opened 6 years ago

eigan commented 6 years ago

Summary

This bug is encountered because of an alias built-in to oh-my-zsh. Using cp ... does not work.

How to reproduce

Test directory:

.
├── file
└── start
    └── level
        └── level

Configuration:

YSU_MODE=ALL
YSU_HARDCORE=1

Commands to execute

$ cd start/level/level
$ cp ../../../file.txt .
Found existing global alias for "../..". You should use: "..."
Found existing global alias for "../../..". You should use: "...."
You Should Use hardcore mode enabled. Use your aliases!
$  cp ..../file.txt .
cp: cannot stat '..../file.txt': No such file or directory
cp .../../file.txt .
Found existing global alias for "../..". You should use: "..."
You Should Use hardcore mode enabled. Use your aliases!

The aliases for "..." is defined here: https://github.com/robbyrussell/oh-my-zsh/blob/master/lib/directories.zsh

MichaelAquilina commented 6 years ago

I'm not entirely certain what the bug is from reading this bug report.

Could you give an example of expected behaviour and actual behaviour? I think the bug is that cp .../../file.txt should be suggested as ..../file.txt but I'm not entirely sure

eigan commented 6 years ago

Sorry, should have made it more clear. This plugin suggest to rewrite the command ( cp ../../../file.txt ) to something that does not work ( cp ..../file.txt . )

It correctly says that .... is an alias, but it cannot be used with cp (as seen in example: (cannot stat ..../file.txt etc))

This could be a bug with oh-my-zsh, I am not sure..

MichaelAquilina commented 6 years ago

ah. Why does .... exist as an alias if it doesn't work?

eigan commented 6 years ago

The aliases for "..." is defined here: https://github.com/robbyrussell/oh-my-zsh/blob/master/lib/directories.zsh

It is sourced here: https://github.com/robbyrussell/oh-my-zsh/blob/master/oh-my-zsh.sh#L31

The aliases are checked in _check_global_aliases().

This code on line 71:. if [[ "$1" = *"$v"* ]]; then will match any commands containing the alias defined. So the alias ...='../..' will match when I type cp ../../ and zsh-you-should-use will complain that I am not using this alias.

Note that whatever command I type which include ../.., zsh-you-should-use will complain:

$ ls ../../
Found existing global alias for "../..". You should use: "..."
You Should Use hardcore mode enabled. Use your aliases!

Edit, using ls ... actually works, so this was a bad example. But mv .../file.txt . does not.

I have solved it by doing this:

@@ -68,7 +68,7 @@ function _check_global_aliases() {
     # Need to remove leading and trailing ' if they exist
     v="${(Q)tokens[2]}"

-    if [[ "$1" = *"$v"* ]]; then
+    if [[ "$1" = "$v"* ]]; then
       ysu_global_message "$v" "$k"
       found=true
     fi
eigan commented 6 years ago

ah. Why does .... exist as an alias if it doesn't work?

It does work.

$ pwd
/home/einar/tmp/test
$ ....
$ pwd
/home
eigan commented 6 years ago

Oh,

$ ls ...

This command actually works. But cp .../file.txt, mv .../file.txt etc does not.

Edit: Sorry for all the spamming :|