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

swiper-avy jumps to wrong line #1541

Closed
basil-conto opened this issue Apr 26, 2018 · 5 comments
Closed

swiper-avy jumps to wrong line #1541

basil-conto opened this issue Apr 26, 2018 · 5 comments

Comments

@basil-conto
Copy link
Collaborator

@basil-conto basil-conto commented Apr 26, 2018

Recipe

  1. make plain
  2. C-xfswiper.elRET
  3. M->
  4. C-sswiper
  5. C-'lf
    swiper-avy
    • Expected result: jump to line 978
    • Actual result: jump to line 1

Discussion

My instinct tells me this behaviour might be related to the fact that candidate positions are calculated with narrowing in effect:

swiper/swiper.el

Lines 188 to 199 in 187d826

(save-excursion
(save-restriction
(narrow-to-region (window-start) (window-end))
(goto-char (point-min))
(forward-line)
(let ((cands))
(while (< (point) (point-max))
(push (cons (1+ (point))
(selected-window))
cands)
(forward-line))
cands)))))

whereas the line-number-at-pos call is in a widened context:

swiper/swiper.el

Lines 213 to 220 in 187d826

(if (window-minibuffer-p (cdr candidate))
(progn
(ivy-set-index (- (line-number-at-pos (car candidate)) 2))
(ivy--exhibit)
(ivy-done)
(ivy-call))
(ivy-quit-and-run
(avy-action-goto (avy-candidate-beg candidate)))))))

By the way, candidates collection, as it currently stands, can be made a bit more efficient:

diff --git a/swiper.el b/swiper.el
index 288682d..15d178a 100644
--- a/swiper.el
+++ b/swiper.el
@@ -180,7 +180,7 @@ swiper-avy
                                                  swiper-faces))
                                   (setq min-overlay-start (overlay-start ov))))
                               visible-overlays))
-           (candidates (append
+           (candidates (nconc
                         (mapcar (lambda (ov)
                                   (cons (overlay-start ov)
                                         (overlay-get ov 'window)))
@@ -190,11 +190,10 @@ swiper-avy
                             (narrow-to-region (window-start) (window-end))
                             (goto-char (point-min))
                             (forward-line)
-                            (let ((cands))
-                              (while (< (point) (point-max))
-                                (push (cons (1+ (point))
-                                            (selected-window))
-                                      cands)
+                            (let ((win (selected-window))
+                                  cands)
+                              (while (not (eobp))
+                                (push (cons (1+ (point)) win) cands)
                                 (forward-line))
                               cands)))))
            (candidate (unwind-protect
@abo-abo
Copy link
Owner

@abo-abo abo-abo commented Apr 26, 2018

Thanks for the report. PRs welcome. I'm on vacation until Wednesday, so I can't code much until then.

Loading

@basil-conto
Copy link
Collaborator Author

@basil-conto basil-conto commented Apr 26, 2018

@abo-abo

I'm on vacation until Wednesday

Wish you fun and rest. :)

Loading

@libniwtr
Copy link

@libniwtr libniwtr commented Oct 6, 2018

Same problem here. Hoping it'd be fixed in the future!

Loading

@abo-abo abo-abo closed this in 985018a Oct 7, 2018
@abo-abo
Copy link
Owner

@abo-abo abo-abo commented Oct 7, 2018

Thanks all, this should finally be fixed now. Please test.

Loading

@basil-conto
Copy link
Collaborator Author

@basil-conto basil-conto commented Oct 7, 2018

Thanks @abo-abo, can confirm it fixes the issue I was having.

Loading

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

Successfully merging a pull request may close this issue.

None yet
3 participants