agkozak / zsh-z

Jump quickly to directories that you have visited "frecently." A native Zsh port of z.sh with added features.
MIT License
2.01k stars 76 forks source link

Prefer exact matches over partial ones #40

Closed sluukkonen closed 3 years ago

sluukkonen commented 3 years ago

Background: I have some projects, which share a common prefix, let's call them foo and foo-bar. They are located in the same parent directory.

If I type z foo, z will navigate me to foo-bar, since I visit it more often. But sometimes I do actually want to visit foo! So currently my workflow goes something like this, which isn't ideal.

z foo
cd ../foo

If an exact directory suffix match can be found, would it make sense to prefer it over partial matches?

agkozak commented 3 years ago

Try typing

z foo/

When you want to go to /path/to/foo. That should get you there.

agkozak commented 3 years ago

Of course, if there is a path like

/path/to/foo/quux

and it ranks very highly, typing

z foo/

will take you there, and not to

/path/to/foo
sluukkonen commented 3 years ago

Try typing

z foo/

When you want to go to /path/to/foo. That should get you there.

This doesn't seem to work on my machine. z foo/ seems to work only with paths like /path/to/foo/quux, but not with paths like /path/to/foo.

Do you need some specific configuration to make it work? I haven't changed the defaults apart from enabling ZSHZ_UNCOMMON=1.

agkozak commented 3 years ago

Let me think about this for a while. Thanks for your observations.

frei-0xff commented 3 years ago

I encountered the same issue. I have two directories dwm and dwmblocks. And if dwmblocks is ranked higher, I have no way to go to dwm. Typing "dwm/" doesn't work. And because of NO_EXTENDED_GLOB I can't even type something like "dwm~block".

agkozak commented 3 years ago

May I share with you why I think this has never been a problem for me? Every once in a while I want to be able to cd to something using a very few characters, on any computer, no matter what the rank is. In other words, I want it to be as easy as using z, but not actually use z's rules. I want to be able to cd to "z," for example, and end up in the ZSH-z repository, even though my name has a "z" in it, and I develop a moderately popular "agkozak" prompt -- I have no way of being sure that, at any time, on any machine, going to "z" will take me to /path/to/agkozak/zsh-z; it might take me to /path/to/agkozak/agkozak-zsh-prompt or even somewhere else.

So I make "z" into a named directory:

[[ -d ${HOME}/.zinit/plugins/agkozak---zsh-z ]] && hash -d z="${HOME}/.zinit/plugins/agkozak---zsh-z"

Now if I want to go there, I can just type

cd ~z

But I hate typing the tilde. So I set

setopt CDABLE_VARS

Now I can just type

cd z

To me it makes more sense to use a builtin tool like hash -d when I really don't want ZSH-z's rules to apply than it does to rewrite ZSH-z to guess what I want differently. Give it a try.

frei-0xff commented 3 years ago

Thank you very much for the advice, maybe I'll try it. But for me, the main reason for using your great tool is that I don't have to manually manage some aliases or shortcuts list. And IMHO it doesn't seem right that you have no way to say that you want to navigate to dwm, but not to dwmblocks. It would've been very logical, from my point of view, if dwm/ worked, but unfortunately it doesn't.

agkozak commented 3 years ago

@frei-0xff Thanks for your comments and for the pull request. I'll play around with it tomorrow.

agkozak commented 3 years ago

(I am, by the way, mystified that z foo/ doesn't already take you guys to /path/to/foo already instead of /path/to/foobar. I recommended it because it does work on my configuration, and now I'm trying to think what could be different between our configurations that would make it not work. Just curious...)

sluukkonen commented 3 years ago

For what it’s worth, my zsh config is available at https://github.com/sluukkonen/configs

Perhaps it could help with debugging.

agkozak commented 3 years ago

I just tried @frei-0xff's patch, which clearly works for him but throws my shell into an endless loop. I think the main question is why z foo/ takes me where a want to go, but it does not work for you guys. I'll want to look at configurations:

For what it’s worth, my zsh config is available at https://github.com/sluukkonen/configs

Thanks! I'll play around with it!

@frei-0xff, would you be willing to share your .zshrc (and any other z-dot files that might use setopt or anything like that)?

frei-0xff commented 3 years ago

The reason it doesn't work on my system is because all paths in the database file ~/.z are saved without a trailing slash. I thought it is universal for all systems, and my patch was intended to mitigate that. It is quite obvious that the trailing slash in the query doesn't match because there is no trailing slashes in paths from the database.

agkozak commented 3 years ago

@frei-0xff You're right, I think. Changing the database format isn't an option, as we're meant to be compatible with rupa/z and all the other z.sh clones.

I can tell that there is something odd about my own .zshrc that makes z foo/ work correctly in the first place. It isn't you guys at all. If I run zsh -f to achieve a semi-pristine environment, adding the slash makes it so that z simply doesn't change PWD at all.

Let me spend a little time today going through my (admittedly very arcane) .zshrc -- at some point ZSH-z's behavior will become like yours, and maybe I'll get a little insight into what the fix should be.

frei-0xff commented 3 years ago

@agkozak Are paths in your ~/.z saved with trailing slashes? I just tried to run zsh with basically empty zshrc and I've got the same results – no trailing slashes and z foo/ doesn't work. Edit: I'm trying it on two systems Arch Linux and Centos 8. Zsh versions are 5.8 and 5.5.1 respectively.

agkozak commented 3 years ago

Our file format should be the same. This is what it looks like here:

/c/Program Files (x86)/FireCMD/UWIN/usr|3.1809274303193553|1612038567
/home/agkoz/foob|49|1624894619
/c/wamp64|1.2852232041694363|1613715209
/home/agkoz/.zbackup|3|1621910758
/home/agkoz/Desktop/78rpmCollection1920s1930sPopularMusic/flac|3|1624470696
/usr/lib|1.2723709721277419|1588975500

No trailing slashes.

frei-0xff commented 3 years ago

@agkozak It seems very strange to me. If it is the same path in $path_field variable (lines 574-578 of zsh-z.plugin.zsh), I don't understand how z foo/ can work. Because there is a trailing slash in the query but not in a path from the database, how could they match?

agkozak commented 3 years ago

Back up your ~/.z file and try what I put on branch trailing-slash. I threw it together quickly, but it should work. Basically it adds the trailing slash for internal use and removes it before the database is written.

frei-0xff commented 3 years ago

Back up your ~/.z file and try what I put on branch trailing-slash. I threw it together quickly, but it should work. Basically it adds the trailing slash for internal use and removes it before the database is written.

It works for me now.

agkozak commented 3 years ago

Great. Let's all use that branch for a few days. I want to make sure I'm not missing any edge cases before I roll out the fix to everyone.

frei-0xff commented 3 years ago

@agkozak I have some strange behavior with counters no longer incrementing in the database. The new ones are added, but not incremented above 1. IMHO, the problem on line 303 of zsh-z.plugin.zsh, I think there is no need to add / there. With line 303 as  path_field=${line%%\|*} Everything works fine to me.

agkozak commented 3 years ago

Thanks for catching that problem. I should be able to fix it in just a bit; be patient.

agkozak commented 3 years ago

@frei-0xff All right, I made the change you suggested. I also noticed some places where the script was now outputting a trailing slash; I eliminated it, as there are a number of other tools that expect ZSH-z output to follow a certain format.

Let's keep testing. I'll have more time soon to scrutinize what we've done, but it looks as if it should work.

frei-0xff commented 3 years ago

@agkozak It seems this change was pretty invasive. I have a bug with common root not calculated property. I think line 412 should be short=${x%/}

agkozak commented 3 years ago

Yes, let's jettison that branch. I'm sure I can achieve the same thing a little more simply.

Give me a day -- I need the time to do this properly.

agkozak commented 3 years ago

I take that back, actually; I think the trailing-slash branch may be the right approach; I just didn't have time to do it properly today.

@frei-0xff I committed your correction -- thanks!

agkozak commented 3 years ago

Let's keep testing trailing-slash; with @frei-0xff's last correction, it actually seems to be doing what we want.

agkozak commented 3 years ago

I notice that by allowing the trailing slash to denote the final element in a path, we are changing a default behavior that rupa/z (z.sh) and older ZSH-z users might expect. It used to be that if z nts would take you to /path/to/Documents, z nts/ would take you to the highest-ranking choice under /path/to/Documents, e.g. /path/to/Documents/foo.

I might consider introducing an environment variable to enable the new behavior (something like ZSHZ_TRAILING_SLASH=1).

frei-0xff commented 3 years ago

@agkozak There is a bug when you navigate to the root directory, record with a blank path instead of / is added to the database. At the subsequent run, that record with a blank path is removed by the code which deletes nonexistent paths. In my pull request I've been trying to do the same thing with adding a trailing slash to the path_field but in a less invasive manner, so it won't interfere so much with other parts of the script. I've reworked it and added $ZSHZ_TRAILING_SLASH setting as well. Please, check it out, maybe that version would work better although I can't figure out why my first version could produce an endless loop. Edit: I figured out and fixed the endless loop issue. I haven't caught it before because it happened only with $ZSHZ_UNCOMMON active.

agkozak commented 3 years ago

@frei-0xff OK, I merged your changes to the normalized-pathfield branch, and I made one addition. I'm just doing the same thing you're doing with the main routine for the legacy completion routine.

frei-0xff commented 3 years ago

@agkozak I think in your code path_field_normalized is not initialized when $ZSHZ_TRAILING_SLASH is not set.

agkozak commented 3 years ago

@frei-0xff Fixed. Thanks!

agkozak commented 3 years ago

I have merged the new feature onto the master branch. Particular thanks go to @frei-0xff for writing such an elegant solution.