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

ivy-completing-read gets confused about dollar signs in directory names #3060

Open
immerrr opened this issue Dec 18, 2024 · 4 comments · May be fixed by #3064
Open

ivy-completing-read gets confused about dollar signs in directory names #3060

immerrr opened this issue Dec 18, 2024 · 4 comments · May be fixed by #3064
Labels

Comments

@immerrr
Copy link

immerrr commented Dec 18, 2024

I see strange behaviour when ivy starts a find-file completion when in a directory that contains dollar signs (on Linux). Here's a quick example:

Repro:

$ mkdir test && cd test
$ mkdir 'dir1' && mkdir '$dir2'
$ touch 'dir1/file1' 'dir1/file1-1' '$dir2/file2' '$dir2/file2-2'
$ cat > init.el <<EOF
(package-initialize)
(add-to-list 'package-archives
             '("melpa" . "https://melpa.org/packages/"))
(package-refresh-contents)
(package-install 'ivy)
(ivy-mode 1)
(find-file "~/$dir2/file2")
(setq use-file-dialog nil)
(call-interactively 'find-file)

EOF
$ HOME=$PWD emacs -Q -l init.el

When starting find-file when in dir1/file1, ivy populates the starting directory correctly
image

When starting find-file when in $dir2/file2, ivy starts the navigation outside $dir2 (sometimes the list also contains an extra copy of ~/$dir2 at the top as you can see on the screen, but i wasn't unable to repro it reliably):
image

It is likely related to the following:

  • built-in find-file executes substitute-in-file-name on the final input string to replace environment variables in the supplied path
  • hence, the initialisation for the completion escapes the directory as $$dir2 instead of $dir2 so that it can roundtrip without the expansion
  • ivy is probably not ready for this.

I have seen some code that does (replace-regexp-in-string "\\$\\$" "$" s), but here in ivy--reset-state where initial-input contains escaped $$dir2, at least the following potential bugs may happen

  • [https://github.com/abo-abo/swiper/blob/8dc02d5b725f78d1f80904807b46f5406f129674/ivy.el#L2364](this comparison) may fail comparing $$foobar to $foobar
  • [https://github.com/abo-abo/swiper/blob/8dc02d5b725f78d1f80904807b46f5406f129674/ivy.el#L2370](this check) will fail when looking for $$foobar directory

i'll advice/hack around it on my side so that it just works, but i have no idea how to fix this correctly, so i'll leave it up to you folks

@immerrr
Copy link
Author

immerrr commented Dec 18, 2024

Another thing that I'm sorry that I don't have time to look into right now is that https://github.com/abo-abo/swiper/blob/master/CONTRIBUTING.org?plain=1#L16-L20 doesn't seem to work for me:

immerrr@mmrcomp3:~/src$ git clone https://github.com/abo-abo/swiper/
Cloning into 'swiper'...
remote: Enumerating objects: 10735, done.
remote: Counting objects: 100% (542/542), done.
remote: Compressing objects: 100% (209/209), done.
remote: Total 10735 (delta 352), reused 507 (delta 323), pack-reused 10193 (from 1)
Receiving objects: 100% (10735/10735), 8.31 MiB | 8.29 MiB/s, done.
Resolving deltas: 100% (7134/7134), done.
immerrr@mmrcomp3:~/src$ cd swiper
immerrr@mmrcomp3:~/src/swiper$ make plain
emacs -Q -batch -L . -f batch-byte-compile colir.el
emacs -Q -batch -L . -f batch-byte-compile ivy-faces.el
emacs -Q -batch -L . -f batch-byte-compile ivy-overlay.el
emacs -Q -batch -L . -f batch-byte-compile ivy.el
emacs -Q -batch -l elpa.el -L . -f batch-byte-compile ivy-avy.el

In toplevel form:
ivy-avy.el:32:11: Error: Cannot open load file: No such file or directory, avy
make: *** [Makefile:53: ivy-avy.elc] Error 1

@immerrr
Copy link
Author

immerrr commented Dec 18, 2024

This fix seems to have worked for me, feel free to do as you please with it:

master...immerrr:swiper:patch-1

basil-conto added a commit that referenced this issue Jan 9, 2025
* targets/obsolete-config.el:
* doc/ivy.org (Setup): Add directory names to load-path.
(Installing from the Git repository):
* CONTRIBUTING.org (Reporting issues): Mention 'make deps' as a
prerequisite for installing the 'avy' package.
* doc/ivy.texi: Regenerate manual.
* ivy.el (ivy--sorted-files): Add commentary about the potential
need for substitute-in-file-name (#3060).
@basil-conto
Copy link
Collaborator

Thank you for the investigation!

(find-file "~/dir2/file2")

Is that meant to be $dir2 rather than dir2?

https://github.com/abo-abo/swiper/blob/master/CONTRIBUTING.org?plain=1#L16-L20 doesn't seem to work for me:

immerrr@mmrcomp3:~/src/swiper$ make plain
emacs -Q -batch -L . -f batch-byte-compile colir.el
emacs -Q -batch -L . -f batch-byte-compile ivy-faces.el
emacs -Q -batch -L . -f batch-byte-compile ivy-overlay.el
emacs -Q -batch -L . -f batch-byte-compile ivy.el
emacs -Q -batch -l elpa.el -L . -f batch-byte-compile ivy-avy.el

In toplevel form:
ivy-avy.el:32:11: Error: Cannot open load file: No such file or directory, avy
make: *** [Makefile:53: ivy-avy.elc] Error 1

You could be missing a make deps before make plain.

The project's (inter)dependencies are a bit of a mess, and there's some WIP to improve the situation, but for now I've updated the docs in commit abb9e1e.

This fix seems to have worked for me, feel free to do as you please with it:

master...immerrr:swiper:patch-1

Thank you. I don't know if it's TRT in the general case, but it definitely addresses the recipe you originally provided.

Would you like to submit the patch as a PR?

@immerrr
Copy link
Author

immerrr commented Jan 10, 2025

Is that meant to be $dir2 rather than dir2?

Yes, sorry, I've fixed the original report.

Would you like to submit the patch as a PR?

#3064 done, admittedly no tests, not able to add them right now, again, sorry

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants