Skip to content
This repository has been archived by the owner on Aug 7, 2022. It is now read-only.

Forward search does not scroll the page #604

Closed
LasseBlaauwbroek opened this issue Aug 23, 2020 · 7 comments
Closed

Forward search does not scroll the page #604

LasseBlaauwbroek opened this issue Aug 23, 2020 · 7 comments

Comments

@LasseBlaauwbroek
Copy link

When doing a forward search from a TeX file, pdf-tools correctly finds the location of the search and jumps to the correct page. However, if the location on the page is below the viewport, the page itself is not scrolled. If you manually scroll down, the arrow does point to the correct location though. Strangely, if the location is above the viewport the scrolling does happen. Testcase:

\documentclass{article}
\usepackage{lipsum}
\begin{document}
This is test number one.

\lipsum[3-7]

This is test number two.

\lipsum[7]

This is test number three.
\end{document}

Try doing a forward search on all three test sentences. The first and last always work. The second only if you manually scroll it into view.

Presumably the error is here:

pdf-tools/lisp/pdf-sync.el

Lines 661 to 677 in d971298

(defun pdf-sync-forward-search (&optional line column)
"Display the PDF location corresponding to LINE, COLUMN."
(interactive)
(cl-destructuring-bind (pdf page _x1 y1 _x2 _y2)
(pdf-sync-forward-correlate line column)
(let ((buffer (or (find-buffer-visiting pdf)
(find-file-noselect pdf))))
(with-selected-window (display-buffer
buffer pdf-sync-forward-display-action)
(pdf-util-assert-pdf-window)
(when page
(pdf-view-goto-page page)
(when y1
(let ((top (* y1 (cdr (pdf-view-image-size)))))
(pdf-util-tooltip-arrow (round top))))))
(with-current-buffer buffer
(run-hooks 'pdf-sync-forward-hook)))))
I do indeed not see any explicit scrolling.

On a slightly unrelated note: It would be great if instead of an arrow, a rectangle could be shown around the exact searched area. Especially with a two-column layout it can be difficult to see what the arrow points to.

@antoine-levitt
Copy link

I think this is the same as #648

@ghost
Copy link

ghost commented Jan 27, 2021

I played around a bit with this today. We just aren't scrolling to the location in the pdf (just going to the page and hoping it's visible. I think I have a fix. I'll create a PR and get it finalized, but you can try this if you want:
(Edit: this fix is 'wrong' in a way. I'm seeing now that pdf-util-tooltip-arrow is supposed to account for the image being in view.)

(defun pdf-sync-forward-search (&optional line column)
  "Display the PDF location corresponding to LINE, COLUMN."
  (interactive)
  (cl-destructuring-bind (pdf page _x1 y1 _x2 _y2)
      (pdf-sync-forward-correlate line column)
    (let ((buffer (or (find-buffer-visiting pdf)
                      (find-file-noselect pdf))))
      (with-selected-window (display-buffer
                             buffer pdf-sync-forward-display-action)
        (pdf-util-assert-pdf-window)
        (when page
          (pdf-view-goto-page page)
          (when y1
            (let ((top (* y1 (cdr (pdf-view-image-size)))))
              (pdf-view-show-vertical-location top)
              (pdf-util-tooltip-arrow (round top))))))
      (with-current-buffer buffer
        (run-hooks 'pdf-sync-forward-hook)))))

(defun pdf-view-show-page-location (page-location)
  "Make sure PAGE-LOCATION is visible in window.

If the PAGE-LOCATION is not clearly visible, then we scroll
the PAGE-LOCATION to 1/3 the current view window."
  (let* ((body-height (window-body-height nil t))
         (visible-top (window-vscroll nil t))
         (visible-bottom (+ visible-top body-height))
         (padding (frame-char-height)))

    ;; If page location is out or 'close' to being out of
    ;; the viewport we scroll the spot to the 1/3 spot
    ;; of the viewport
    (when (or (< page-location (+ visible-top padding))
              (> page-location (- visible-bottom padding)))
      (image-set-window-vscroll
       (max 0
            (round (- page-location
                      (/ body-height 3))))))))

@vedang
Copy link

vedang commented Feb 21, 2021

Hi @LasseBlaauwbroek ,

I have merged @tylerware's change into vedang/pdf-tools@2f6be19 , and it fixes this issue.

Re: showing a rectangle around the searched text, I'll track that in a separate todo for myself. Thanks!

@LasseBlaauwbroek
Copy link
Author

Thats great! Thanks for your efforts.

@vedang
Copy link

vedang commented Feb 27, 2021

@LasseBlaauwbroek: Can you please close this issue? This helps me better track the open issues. Thank you!

@LasseBlaauwbroek
Copy link
Author

@vedang It seems a bit strange to track the issues on your fork using this repository. This issue has been fixed there, but not here. If this repo is ever resurrected, the state of the issues would be completely inconsistent.

May I suggest that you import the issues of this repo to your own? A quick google suggests this script may be able to help: https://github.com/IQAndreas/github-issues-import

@vedang
Copy link

vedang commented Feb 28, 2021

Okay @LasseBlaauwbroek!

My idea was that if @politza revives this repo, I'll submit a PR to him from my master branch, with all the patches I've applied. I understand your point of view as well :)

@politza politza closed this as completed Aug 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants