Can't 'wd add' a path with spaces
raxod502 opened this issue · 14 comments
It appears the new behavior of wd add to default to the basename of the current directory does not quite work right when the directory path includes spaces. Here is an example:
folder % pwd
/private/tmp/path/with spaces/in it/folder
folder % wd add folder
* Warp point added
folder % wd rm folder
* Warp point removed
folder % wd add
* Warp point should not contain whitespace
folder % basename $(pwd)
with
in
folder
folder % basename $PWD
folder
It appears to be due to some quite frankly bizarre wordsplitting behavior by zsh. I do not have sh_word_split enabled, as you can see from the result of basename $PWD. I think replacing basename $(pwd) with basename $PWD in the wd add implementation would fix the issue.
I can make a pull request if this seems like a reasonable solution to the problem.
Thanks for reporting!
I will try to get time this weekend to check it out. I think we should make a failing test case and fix it again. Could also be interesting to test different zsh options such as sh_word_split, as you mention, or other weird ones (man zshoptions)..
I frequently see
emulate -LR zsh
at the top of zsh functions, to deal with the user having set weird options. This might be a good idea for wd.
One solution is to get impatient and reimplement wd in Python 😛
Sorry it had to come to that @raxod502. Honestly, I'm doing a year abroad and can only juggle so many courses and research projects at a time. Wish there was someone with the time to create a PR, but that's how it is. I'll let you know when I find the time, but in the mean time, happy hacking 😉
Oh I totally don't blame you at all. And honestly, solving this issue in general might be too difficult. I actually tried writing my reimplementation in Zsh to start with, and man are there a lot of weird edge cases with interesting key names.
I may actually provide a pull request to fix this issue, though. It would be simple.
Oh. Nevermind. It looks like the bug is actually in getopt, which is a little depressing.
Whaat, that would be unexpected. You have a bug ticket or something?
Manpage for getopt, in the BUGS section.
Arguments containing white space or embedded shell metacharacters generally will not survive intact; this looks easy to fix but isn't. People trying to fix getopt or the example in this manpage should check the history of this file in FreeBSD.
That said, I haven't done a complete investigation. But I didn't see any wordsplitting weirdness in your code (and zsh thankfully makes it hard to do by accident), so based on this I assumed the error was in getopt.
Wait no this makes no sense. I think I'm completely wrong about this.
OK I officially don't have a clue. Now I've actually re-read my original post, which indeed points out exactly what the problem and fix was. Pull request provided.
Thanks a lot for the PR 😄 I think detecting the spaces without adding support for it is a good compromise.
I'm convinced that we're never going to circumvent all zsh peculiarities, so we'll just have to get by as best we can.
Oh I do think it's possible. But it does require automatic unit test generation over thousands of permutations of shell meta-characters, which I was halfway through setting up before switching to Python ;)
Hm, that sounds like a systematic approach which actually could give some substantial results. Tag me if you every pick it up again!
Will do. For my own sanity I'm sticking with Python for my own implementation but it might be nice to make a library that provides a consistent API for associative arrays in zsh that works with absolutely any keys and values. I'll put it on my free-time wishlist.