z shows paths with embedded ../ instead of resulting cwd

[samuelkarp]

When I run z, I see paths with embedded ../ instead of the resulting cwd after changing directory. The path I expect to see is much lower.

$ z
0.000	/home/sam/go/src/foo/bar/../../baz/baz/
...
64.50	/home/sam/go/src/baz/baz/

Not sure if this is a bug, but I'm filing this based on our IRC conversation.

[euank]

The path I expect to see is much lower.

As an idle FYI, since the last release the order of the list has been reversed since it made sense to have more likely candidates nearer the top, but that doesn't really change this issue meaningfully.

Not sure if this is a bug.

I think this is a bug. Having a directory show up twice if there are bindmounts or symlinks might not be a bug.
Having one show up twice with a normal directory tree shouldn't happen and means the frecency is now slightly off for that directory.

Anyways, my theory is this happened due to the maybe add relative code.

That code causes things like z ./some-dir-in-cwd to treat that path as "visited", allowing it to be matched even if it wasn't yet in the frecency database.

Since it doesn't do a realpath, it certainly makes sense it could add such an entry..

The other method entries are added (the pazi visit "${PWD}" method caused by cd-ing around) I think will use canonicalized paths on both zsh and bash.

I skimmed bash's code, and it looks like getcwd sets the PWD variable, and that section grabs the name from walking up the directory tree from '.' to '/' and using dirnames, so it seems like it couldn't end up with a .. via that portion.

It seems to me like we should be able to just canonicalize the directory in maybe_add_relative_to and add a test.

Thanks for the report!

[LinuxMercedes]

I was able to reproduce this by running z ../../foo, so the maybe_add_relative code is indeed the culprit.

It looks like std::fs::canonicalize follows symlinks as well as removing .. and ., which is I think too aggressive for this case. I fear we may have to.........implement this ourselves

[LinuxMercedes]

What we ideally want is to be able to call canonicalize_filename_mode with CAN_NOLINKS but this sounds awful. Let's just go with canonicalize and if symlinks turn out to be a big deal later we can fix it then.