* cl-assertion error in doc-view
@ 2012-09-19 17:55 Tassilo Horn
2012-09-20 17:25 ` martin rudalics
2012-09-27 3:17 ` Stefan Monnier
0 siblings, 2 replies; 10+ messages in thread
From: Tassilo Horn @ 2012-09-19 17:55 UTC (permalink / raw)
To: emacs-devel; +Cc: Stefan Monnier
Hi Stefan & all,
I've just checked in that new slice by bounding box feature for
doc-view, and additionally this change in `doc-view-display' I'm not to
certain it's TRT.
--8<---------------cut here---------------start------------->8---
@@ -1095,7 +1175,9 @@
"page-[0-9]+\\.png" t)
'doc-view-sort))
(dolist (win (or (get-buffer-window-list buffer nil t)
- (list (selected-window))))
+ (list (let ((w (selected-window)))
+ (set-window-buffer w buffer)
+ w))))
(let* ((page (doc-view-current-page win))
(pagefile (expand-file-name (format "page-%d.png" page)
(doc-view-current-cache-dir))))
@@ -1103,8 +1185,8 @@
(and (not (member pagefile prev-pages))
(member pagefile doc-view-current-files)))
(with-selected-window win
- (cl-assert (eq (current-buffer) buffer))
- (doc-view-goto-page page))))))))
+ (cl-assert (eq (current-buffer) buffer) t)
+ (doc-view-goto-page page))))))))
--8<---------------cut here---------------end--------------->8---
Before that change, when I find some foo.pdf file with emacs -Q, the
cl-assert triggers because the foo.pdf buffer isn't already displayed
thus `get-buffer-window-list' returns nil, and `selected-window' returns
the window I'm in when doing the `C-x C-f foo.pdf'.
Is the change sensible? At least, it fixes the bug, but the multiple
windows on the same doc stuff is yours, so feel free to correct it as
seems fit. Maybe the bug is somewhere else, though, at least it
suprises me that when finding a file and the major mode function runs
the buffer isn't already displayed. I think it used to be different...
Bye,
Tassilo
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: cl-assertion error in doc-view
2012-09-19 17:55 cl-assertion error in doc-view Tassilo Horn
@ 2012-09-20 17:25 ` martin rudalics
2012-09-20 19:54 ` Tassilo Horn
2012-09-20 21:10 ` Stefan Monnier
2012-09-27 3:17 ` Stefan Monnier
1 sibling, 2 replies; 10+ messages in thread
From: martin rudalics @ 2012-09-20 17:25 UTC (permalink / raw)
To: emacs-devel, Stefan Monnier
> Before that change, when I find some foo.pdf file with emacs -Q, the
> cl-assert triggers because the foo.pdf buffer isn't already displayed
> thus `get-buffer-window-list' returns nil, and `selected-window' returns
> the window I'm in when doing the `C-x C-f foo.pdf'.
>
> Is the change sensible? At least, it fixes the bug, but the multiple
> windows on the same doc stuff is yours, so feel free to correct it as
> seems fit. Maybe the bug is somewhere else, though, at least it
> suprises me that when finding a file and the major mode function runs
> the buffer isn't already displayed. I think it used to be different...
I'd use something like the below instead.
martin
=== modified file 'lisp/doc-view.el'
--- lisp/doc-view.el 2012-09-19 17:54:59 +0000
+++ lisp/doc-view.el 2012-09-20 17:18:44 +0000
@@ -1169,15 +1169,16 @@
If FORCE is non-nil, start viewing even if the document does not
have the page we want to view."
(with-current-buffer buffer
- (let ((prev-pages doc-view-current-files))
+ (let ((prev-pages doc-view-current-files)
+ (windows (get-buffer-window-list buffer nil t)))
(setq doc-view-current-files
(sort (directory-files (doc-view-current-cache-dir) t
"page-[0-9]+\\.png" t)
'doc-view-sort))
- (dolist (win (or (get-buffer-window-list buffer nil t)
- (list (let ((w (selected-window)))
- (set-window-buffer w buffer)
- w))))
+ (unless windows
+ (switch-to-buffer buffer)
+ (setq windows (get-buffer-window-list buffer nil t)))
+ (dolist (win windows)
(let* ((page (doc-view-current-page win))
(pagefile (expand-file-name (format "page-%d.png" page)
(doc-view-current-cache-dir))))
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: cl-assertion error in doc-view
2012-09-20 17:25 ` martin rudalics
@ 2012-09-20 19:54 ` Tassilo Horn
2012-09-20 21:10 ` Stefan Monnier
1 sibling, 0 replies; 10+ messages in thread
From: Tassilo Horn @ 2012-09-20 19:54 UTC (permalink / raw)
To: martin rudalics; +Cc: Stefan Monnier, emacs-devel
martin rudalics <rudalics@gmx.at> writes:
>> Is the change sensible? At least, it fixes the bug, but the multiple
>> windows on the same doc stuff is yours, so feel free to correct it as
>> seems fit. Maybe the bug is somewhere else, though, at least it
>> suprises me that when finding a file and the major mode function runs
>> the buffer isn't already displayed. I think it used to be
>> different...
>
> I'd use something like the below instead.
Ok, tested & committed; thank you.
Bye,
Tassilo
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: cl-assertion error in doc-view
2012-09-20 17:25 ` martin rudalics
2012-09-20 19:54 ` Tassilo Horn
@ 2012-09-20 21:10 ` Stefan Monnier
2012-09-20 22:24 ` martin rudalics
1 sibling, 1 reply; 10+ messages in thread
From: Stefan Monnier @ 2012-09-20 21:10 UTC (permalink / raw)
To: martin rudalics; +Cc: emacs-devel
> I'd use something like the below instead.
Using switch-to-buffer or set-window-buffer is incorrect since this
function doesn't know if there's an actual intention to display the
buffer, nor where it will be displayed (and the selected-window may be
inappropriate, dedicated, minibuffer-only).
So we need a different solution that does not rely on displaying
the buffer.
Stefan
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: cl-assertion error in doc-view
2012-09-20 21:10 ` Stefan Monnier
@ 2012-09-20 22:24 ` martin rudalics
2012-09-21 1:12 ` Stefan Monnier
0 siblings, 1 reply; 10+ messages in thread
From: martin rudalics @ 2012-09-20 22:24 UTC (permalink / raw)
To: Stefan Monnier; +Cc: emacs-devel
> Using switch-to-buffer or set-window-buffer is incorrect since this
> function doesn't know if there's an actual intention to display the
> buffer, nor where it will be displayed (and the selected-window may be
> inappropriate, dedicated, minibuffer-only
That's the reason why I suggested `switch-to-buffer' with
FORCE-SAME-WINDOW nil.
).
> So we need a different solution that does not rely on displaying
> the buffer.
What's the purpose of a thing called `doc-view-display' if it's not to
display something?
martin
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: cl-assertion error in doc-view
2012-09-20 22:24 ` martin rudalics
@ 2012-09-21 1:12 ` Stefan Monnier
2012-09-21 7:14 ` Tassilo Horn
0 siblings, 1 reply; 10+ messages in thread
From: Stefan Monnier @ 2012-09-21 1:12 UTC (permalink / raw)
To: martin rudalics; +Cc: emacs-devel
>> So we need a different solution that does not rely on displaying
>> the buffer.
> What's the purpose of a thing called `doc-view-display' if it's not to
> display something?
It's to setup the overlays and properties so that the text is replaced by
the page images (using the `display' property).
That's orthogonal to whether the buffer is displayed in a window.
Stefan
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: cl-assertion error in doc-view
2012-09-21 1:12 ` Stefan Monnier
@ 2012-09-21 7:14 ` Tassilo Horn
2012-09-21 12:59 ` Stefan Monnier
0 siblings, 1 reply; 10+ messages in thread
From: Tassilo Horn @ 2012-09-21 7:14 UTC (permalink / raw)
To: Stefan Monnier; +Cc: martin rudalics, emacs-devel
Stefan Monnier <monnier@iro.umontreal.ca> writes:
>>> So we need a different solution that does not rely on displaying
>>> the buffer.
>> What's the purpose of a thing called `doc-view-display' if it's not
>> to display something?
>
> It's to setup the overlays and properties so that the text is replaced
> by the page images (using the `display' property). That's orthogonal
> to whether the buffer is displayed in a window.
How about using `set-window-buffer' and `save-window-excursion' like so?
That also does the right thing, that is, not show the buffer forcefully,
with `find-file-noselect'.
--8<---------------cut here---------------start------------->8---
=== modified file 'lisp/doc-view.el'
--- lisp/doc-view.el 2012-09-20 19:53:40 +0000
+++ lisp/doc-view.el 2012-09-21 07:05:40 +0000
@@ -1169,25 +1169,25 @@
If FORCE is non-nil, start viewing even if the document does not
have the page we want to view."
(with-current-buffer buffer
- (let ((prev-pages doc-view-current-files)
- (windows (get-buffer-window-list buffer nil t)))
+ (let ((prev-pages doc-view-current-files))
(setq doc-view-current-files
(sort (directory-files (doc-view-current-cache-dir) t
"page-[0-9]+\\.png" t)
'doc-view-sort))
- (unless windows
- (switch-to-buffer buffer)
- (setq windows (get-buffer-window-list buffer nil t)))
- (dolist (win windows)
- (let* ((page (doc-view-current-page win))
- (pagefile (expand-file-name (format "page-%d.png" page)
- (doc-view-current-cache-dir))))
- (when (or force
- (and (not (member pagefile prev-pages))
- (member pagefile doc-view-current-files)))
- (with-selected-window win
- (cl-assert (eq (current-buffer) buffer) t)
- (doc-view-goto-page page))))))))
+ (save-window-excursion
+ (dolist (win (or (get-buffer-window-list buffer nil t)
+ (let ((w (selected-window)))
+ (set-window-buffer w buffer)
+ (list w))))
+ (let* ((page (doc-view-current-page win))
+ (pagefile (expand-file-name (format "page-%d.png" page)
+ (doc-view-current-cache-dir))))
+ (when (or force
+ (and (not (member pagefile prev-pages))
+ (member pagefile doc-view-current-files)))
+ (with-selected-window win
+ (cl-assert (eq (current-buffer) buffer) t)
+ (doc-view-goto-page page)))))))))
(defun doc-view-buffer-message ()
;; Only show this message initially, not when refreshing the buffer (in which
--8<---------------cut here---------------end--------------->8---
Bye,
Tassilo
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: cl-assertion error in doc-view
2012-09-21 7:14 ` Tassilo Horn
@ 2012-09-21 12:59 ` Stefan Monnier
0 siblings, 0 replies; 10+ messages in thread
From: Stefan Monnier @ 2012-09-21 12:59 UTC (permalink / raw)
To: martin rudalics; +Cc: emacs-devel
> How about using `set-window-buffer' and `save-window-excursion' like so?
> That also does the right thing, that is, not show the buffer forcefully,
> with `find-file-noselect'.
No, that'd just be a hack. I'm working on a proper solution which
simply lets the auxiliary data accept a special `not-a-window' value.
Stefan
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: cl-assertion error in doc-view
2012-09-19 17:55 cl-assertion error in doc-view Tassilo Horn
2012-09-20 17:25 ` martin rudalics
@ 2012-09-27 3:17 ` Stefan Monnier
2012-09-27 18:17 ` Tassilo Horn
1 sibling, 1 reply; 10+ messages in thread
From: Stefan Monnier @ 2012-09-27 3:17 UTC (permalink / raw)
To: emacs-devel
> Before that change, when I find some foo.pdf file with emacs -Q, the
> cl-assert triggers because the foo.pdf buffer isn't already displayed
> thus `get-buffer-window-list' returns nil, and `selected-window' returns
> the window I'm in when doing the `C-x C-f foo.pdf'.
Should be fixed in trunk now,
Stefan
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: cl-assertion error in doc-view
2012-09-27 3:17 ` Stefan Monnier
@ 2012-09-27 18:17 ` Tassilo Horn
0 siblings, 0 replies; 10+ messages in thread
From: Tassilo Horn @ 2012-09-27 18:17 UTC (permalink / raw)
To: Stefan Monnier; +Cc: emacs-devel
Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> Before that change, when I find some foo.pdf file with emacs -Q, the
>> cl-assert triggers because the foo.pdf buffer isn't already displayed
>> thus `get-buffer-window-list' returns nil, and `selected-window' returns
>> the window I'm in when doing the `C-x C-f foo.pdf'.
>
> Should be fixed in trunk now,
And seems to work fine.
Thanks,
Tassilo
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2012-09-27 18:17 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-19 17:55 cl-assertion error in doc-view Tassilo Horn
2012-09-20 17:25 ` martin rudalics
2012-09-20 19:54 ` Tassilo Horn
2012-09-20 21:10 ` Stefan Monnier
2012-09-20 22:24 ` martin rudalics
2012-09-21 1:12 ` Stefan Monnier
2012-09-21 7:14 ` Tassilo Horn
2012-09-21 12:59 ` Stefan Monnier
2012-09-27 3:17 ` Stefan Monnier
2012-09-27 18:17 ` Tassilo Horn
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).