search

abo-abo / swiper

Ivy - a generic completion frontend for Emacs, Swiper - isearch with an overview, and more. Oh, man!
https://oremacs.com/swiper/
2.3k stars 338 forks source link

ivy--re-filter get wrong #3048

Closed yjloong closed 4 months ago

yjloong commented 4 months ago 
Debugger entered--Lisp error: (wrong-type-argument stringp 2164)
  compare-strings("isearch-thing-at-point" nil nil 2164 nil nil t)
  ivy--case-fold-string=("isearch-thing-at-point" 2164)
  cl--position("isearch-thing-at-point" (2164 2269 2972) 0 nil nil)
  cl-position("isearch-thing-at-point" (2164 2269 2972) :test ivy--case-fold-string=)
  ivy--recompute-index("isearch-thing-at-point" (2164 2269 2972))
  ivy--filter("isearch-thing-at-point" (2164 2269 2972))
  ivy--reset-state(#s(ivy-state :prompt "Swiper: " :collection swiper-isearch-function :predicate nil :require-match t :initial-input "isearch-thing-at-point" :history swiper-history :preselect nil :keymap (keymap (18 . swiper-isearch-C-r) (27 keymap (110 . swiper-isearch-thing-at-point)) (remap keymap (ivy-insert-current . swiper--isearch-insert-current)) keymap (3 keymap (6 . swiper-toggle-face-matching)) (67108919 . swiper-mc) (67108903 . swiper-avy) (12 . swiper-recenter-top-bottom) (27 keymap (113 . swiper-query-replace)) (19 . swiper-C-s)) :update-fn #f(compiled-function () #<bytecode 0x69798f5a8643518>) :sort nil :frame #<frame F2 0x561a3b2635c0> :window #<window 3 on packages.el> :buffer #<buffer packages.el> :text nil :action (1 ("o" swiper-isearch-action "default") ("w" swiper-isearch-action-copy "copy") ("i" swiper-isearch-action-insert "insert")) :unwind swiper--isearch-unwind :re-builder swiper--re-builder :matcher nil :dynamic-collection t :display-transformer-fn nil :directory "/home/yjl00405/.emacs.d/lisp/" :caller swiper-isearch :current nil :def nil :ignore t :multi-action nil :extra-props (:fname "/home/yjl00405/.emacs.d/lisp/packages.el")))
  ivy-read("Swiper: " swiper-isearch-function :initial-input "isearch-thing-at-point" :keymap (keymap (18 . swiper-isearch-C-r) (27 keymap (110 . swiper-isearch-thing-at-point)) (remap keymap (ivy-insert-current . swiper--isearch-insert-current)) keymap (3 keymap (6 . swiper-toggle-face-matching)) (67108919 . swiper-mc) (67108903 . swiper-avy) (12 . swiper-recenter-top-bottom) (27 keymap (113 . swiper-query-replace)) (19 . swiper-C-s)) :dynamic-collection t :require-match t :action swiper-isearch-action :re-builder swiper--re-builder :history swiper-history :extra-props (:fname "/home/yjl00405/.emacs.d/lisp/packages.el") :caller swiper-isearch)
  swiper-isearch("isearch-thing-at-point")

After the commit 738da36d1b61e5b2a38e5775b23edd214ad88d6e, a bug run well before has been fixed in it. In my env, function ivy--re-filter's args re a string, and candidates a list(number). They will be called with string-match-p. But they are string and number. So error will ignored with ignore-errors function. Just like nothing has been filtered. But the fix has not considered string-match-p. Maybe the question is only caused by my env's setting.

xavierr commented 4 months ago

I got the same issue. It is triggered (for example) by calling (swiper-isearch "some string"). Thanks.

basil-conto commented 4 months ago

But the fix has not considered string-match-p.

Your analysis is correct, but is missing the same point that I missed in the refactor: that other parts of Ivy apparently rely on ivy--refilter returning nil when an error is encountered. IOW, this is my fault, not a problem with your environment.

So it seems like this is yet another example of non-string candidates being slapped on as an afterthought rather than being handled properly: I find it bizarre that ivy--reset-state (which is called at the start of every completion session) tries to filter something that its caller swiper-isearch knows in advance will not work (in the case of string-matching buffer positions).

Thank you both very much for catching this and for the easy repro, and sorry for the breakage. I hope it's fixed again.

yjloong commented 4 months ago

Thanks for your works. And if I can get the fix soon after the fix commited in repository by use-package?

basil-conto commented 4 months ago

And if I can get the fix soon after the fix commited in repository by use-package?

I'm not sure what your use-package setup is like, but yes, the fix should now be available via GNU-devel ELPA (recommended) and MELPA.

xavierr commented 4 months ago

It works. Thank you very much!