Doc View Page Problems

Doc View Page Problems

[Ian D]

[-- Attachment #1: Type: text/plain, Size: 956 bytes --]


Emacs Developers,

        I've been having problems with DocView for quite some time.
        While the buffers were off screen, the document I'd be viewing
        would reset to page one.  I'm sure I'm not the only one having
        this problem, but I do have a solution.

        It turns out the problem comes from when all of the windows in
        which a document was being viewed are gone, there is no way to
        keep track of the location of the document.  I've attached a
        patch that will solve this problem.  What my fix does is updates
        the default entry in `image-mode-winprops-alist' (the one with
        key `t') to keep track of the current page.  Then when there are
        no more windows to use, it uses the last known page.

        I hope this helps others who have been having this problem.

Ian D

P.S.  I apologize if the bug mailing list would have been the
appropriate mailing list to which to send this.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: doc-view.el.patch --]
[-- Type: text/x-diff, Size: 831 bytes --]

diff --git a/lisp/doc-view.el b/lisp/doc-view.el
index 09d5925..0e8f511 100644
--- a/lisp/doc-view.el
+++ b/lisp/doc-view.el
@@ -336,7 +336,7 @@ of the page moves to the previous page."
       ;; Don't do it if there's a conversion is running, since in that case, it
       ;; will be done later.
       (with-selected-window (car winprops)
-        (doc-view-goto-page 1)))))
+        (doc-view-goto-page (image-mode-window-get 'page t))))))
 
 (defvar-local doc-view--current-files nil
   "Only used internally.")
@@ -502,6 +502,7 @@ Typically \"page-%s.png\".")
 	(setq page len)))
     (force-mode-line-update)            ;To update `current-page'.
     (setf (doc-view-current-page) page
+          (image-mode-window-get 'page t) page      ;; Update the default page.
 	  (doc-view-current-info)
 	  (concat
 	   (propertize

Re: Doc View Page Problems

[Stefan Monnier]

>         patch that will solve this problem.  What my fix does is updates
>         the default entry in `image-mode-winprops-alist' (the one with
>         key `t') to keep track of the current page.  Then when there are
>         no more windows to use, it uses the last known page.

Good idea.

> P.S.  I apologize if the bug mailing list would have been the
> appropriate mailing list to which to send this.

It would have worked as well, maybe even better.

> diff --git a/lisp/doc-view.el b/lisp/doc-view.el
> index 09d5925..0e8f511 100644
> --- a/lisp/doc-view.el
> +++ b/lisp/doc-view.el
> @@ -336,7 +336,7 @@ of the page moves to the previous page."
>        ;; Don't do it if there's a conversion is running, since in that case, it
>        ;; will be done later.
>        (with-selected-window (car winprops)
> -        (doc-view-goto-page 1)))))
> +        (doc-view-goto-page (image-mode-window-get 'page t))))))

Could you provide a ChangeLog entry for that change?  Are we sure
(image-mode-window-get 'page t) can't return nil?

>      (setf (doc-view-current-page) page
> +          (image-mode-window-get 'page t) page      ;; Update the default page.

Wouldn't it be better to do that directly inside image-mode-window-put?


        Stefan

Re: Doc View Page Problems

[Ian D]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> P.S.  I apologize if the bug mailing list would have been the
>> appropriate mailing list to which to send this.
>
>It would have worked as well, maybe even better.
>
        Noted.  I will remember that for next time.

> Could you provide a ChangeLog entry for that change?  Are we sure
> (image-mode-window-get 'page t) can't return nil?
>
        I put the ChangeLog entry at the end of this email.  The first
        thing that DocView does when it opens a file is adds the default
        configuration to image-mode-winprop-alist.  image-mode won't
        remove it, since the key is `t'.  That means that
        (image-mode-window-get 'page t) will always return something,
        unless the user has specifically removed it.

>>      (setf (doc-view-current-page) page
>> +          (image-mode-window-get 'page t) page      ;; Update the default page.
>
> Wouldn't it be better to do that directly inside image-mode-window-put?
>
>
>         Stefan
>
        I used `image-mode-window-get' because that was what was being
        used through most of the document (including the line above
        it).  It could just as easily be (image-mode-window-put 'page
        page t).

Ian D


ChangeLog entry:

2014-04-16  Ian Dunn <dunni@gnu.org>

	* doc-view.el (doc-view-new-window-function): If no window
	exists,	move to the last known page.
	(doc-view-goto-page): Updates the page of the default window
	configuration.

Re: Doc View Page Problems

[Stefan Monnier]

>         I put the ChangeLog entry at the end of this email.

Thanks.

>>> (setf (doc-view-current-page) page
>>> +          (image-mode-window-get 'page t) page      ;; Update the default page.
>> Wouldn't it be better to do that directly inside image-mode-window-put?
>         I used `image-mode-window-get' because that was what was being
>         used through most of the document (including the line above
>         it).  It could just as easily be (image-mode-window-put 'page
>         page t).

No, I really meant to do it *inside* image-mode-window-put, not to use
image-mode-window-put instead of setf'ing image-mode-window-get.


        Stefan

Re: Doc View Page Problems

[Ian D]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>>> (setf (doc-view-current-page) page
>>>> +          (image-mode-window-get 'page t) page      ;; Update the default page.
>>> Wouldn't it be better to do that directly inside image-mode-window-put?
>>         I used `image-mode-window-get' because that was what was being
>>         used through most of the document (including the line above
>>         it).  It could just as easily be (image-mode-window-put 'page
>>         page t).
>
> No, I really meant to do it *inside* image-mode-window-put, not to use
> image-mode-window-put instead of setf'ing image-mode-window-get.
>
        Ah.  I think (with my limited knowledge of the Emacs code base)
        that it might be possible.  It looks like (with a quick
        recursive grep) that only DocView uses `image-mode-window-put'
        and `image-mode-window-get'.

        Since image-mode sets up the `t' entry itself, it could be
        possible to continuously update this every time another entry is
        updated, if that's what you're suggesting.  For example, when
        'page is updated, have `image-mode-window-put' update the page
        in the `t' entry as well.

        Is this what you are suggesting?  If so, that shouldn't be too
        much of a problem.

Ian D

Re: Doc View Page Problems

[Stefan Monnier]

>         Since image-mode sets up the `t' entry itself, it could be
>         possible to continuously update this every time another entry is
>         updated, if that's what you're suggesting.

Yes, that's what I'm suggesting.

>         For example, when 'page is updated, have
>         `image-mode-window-put' update the page in the `t' entry
>         as well.

Right, and that would then automatically apply to all other properties
(e.g. horizontal scrolling, ...).


        Stefan

Re: Doc View Page Problems

[Ian D]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>>         Since image-mode sets up the `t' entry itself, it could be
>>         possible to continuously update this every time another entry is
>>         updated, if that's what you're suggesting.
>
> Yes, that's what I'm suggesting.
>
        Alright.  I've got the full patch, and updated ChangeLog entry.

Ian D

diff --git a/lisp/doc-view.el b/lisp/doc-view.el
index 4254485..4ff78b5 100644
--- a/lisp/doc-view.el
+++ b/lisp/doc-view.el
@@ -336,7 +336,7 @@ of the page moves to the previous page."
       ;; Don't do it if there's a conversion is running, since in that case, it
       ;; will be done later.
       (with-selected-window (car winprops)
-        (doc-view-goto-page 1)))))
+        (doc-view-goto-page (image-mode-window-get 'page t))))))
 
 (defvar-local doc-view--current-files nil
   "Only used internally.")
diff --git a/lisp/image-mode.el b/lisp/image-mode.el
index de6bb85..98fc385 100644
--- a/lisp/image-mode.el
+++ b/lisp/image-mode.el
@@ -90,6 +90,9 @@ otherwise it defaults to t, used for times when the buffer is not displayed."
 
 (defun image-mode-window-put (prop val &optional winprops)
   (unless (consp winprops) (setq winprops (image-mode-winprops winprops)))
+  (setcdr (assq t image-mode-winprops-alist)
+          (cons (cons prop val)
+                (delq (assq prop (cdr winprops)) (cdr winprops))))
   (setcdr winprops (cons (cons prop val)
                          (delq (assq prop (cdr winprops)) (cdr winprops)))))
 
2014-04-16 Ian Dunn <dunni@gnu.org>

	* doc-view.el (doc-vew-new-window-function): If no window
	  exists, move to the last known page.
	* image-mode.el (image-mode-window-put): Updates the default
	  configuration.

Re: Doc View Page Problems

[Stefan Monnier]

>         Alright.  I've got the full patch, and updated ChangeLog entry.

Thanks, installed,


        Stefan

Re: Doc View Page Problems

[Ian D]

       One more thing that I noticed is that image-mode itself doesn't
       setup the default window configuration, so I've included a patch
       that will do this.

Ian D

diff --git a/lisp/image-mode.el b/lisp/image-mode.el
index 98fc385..76ea1e9 100644
--- a/lisp/image-mode.el
+++ b/lisp/image-mode.el
@@ -56,7 +56,9 @@ WINDOW defaults to `selected-window' if it displays the current buffer, and
 otherwise it defaults to t, used for times when the buffer is not displayed."
   (cond ((null window)
          (setq window
-               (if (eq (current-buffer) (window-buffer)) (selected-window) t)))
+               (if (and (eq (current-buffer) (window-buffer))
+                        (assq t image-mode-winprops-alist))
+                   (selected-window) t)))
         ((eq window t))
        ((not (windowp window))
         (error "Not a window: %s" window)))

image-mode.el (image-mode-winprops): Setup the default winprop if it
doesn't already exist.

Re: Doc View Page Problems

[Stefan Monnier]

>        One more thing that I noticed is that image-mode itself doesn't
>        setup the default window configuration, so I've included a patch
>        that will do this.

Not sure if that's the right way to do it.
How 'bout the patch below instead?


        Stefan


=== modified file 'lisp/image-mode.el'
--- lisp/image-mode.el	2014-04-17 03:58:25 +0000
+++ lisp/image-mode.el	2014-04-20 02:46:54 +0000
@@ -90,9 +90,8 @@
 
 (defun image-mode-window-put (prop val &optional winprops)
   (unless (consp winprops) (setq winprops (image-mode-winprops winprops)))
-  (setcdr (assq t image-mode-winprops-alist)
-          (cons (cons prop val)
-                (delq (assq prop (cdr winprops)) (cdr winprops))))
+  (unless (eq t (car winprops))
+    (image-mode-window-put prop val t))
   (setcdr winprops (cons (cons prop val)
                          (delq (assq prop (cdr winprops)) (cdr winprops)))))
 

Re: Doc View Page Problems

[Ian D]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>>        One more thing that I noticed is that image-mode itself doesn't
>>        setup the default window configuration, so I've included a patch
>>        that will do this.
>
> Not sure if that's the right way to do it.
> How 'bout the patch below instead?
>
>
>         Stefan
>
>
> === modified file 'lisp/image-mode.el'
> --- lisp/image-mode.el	2014-04-17 03:58:25 +0000
> +++ lisp/image-mode.el	2014-04-20 02:46:54 +0000
> @@ -90,9 +90,8 @@
>  
>  (defun image-mode-window-put (prop val &optional winprops)
>    (unless (consp winprops) (setq winprops (image-mode-winprops winprops)))
> -  (setcdr (assq t image-mode-winprops-alist)
> -          (cons (cons prop val)
> -                (delq (assq prop (cdr winprops)) (cdr winprops))))
> +  (unless (eq t (car winprops))
> +    (image-mode-window-put prop val t))
>    (setcdr winprops (cons (cons prop val)
>                           (delq (assq prop (cdr winprops)) (cdr winprops)))))
>  
>

        That seems to do the trick.  I say go for it.

Ian D