unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#72241: 31.0.50; [PATCH] Use a dedicated buffer for `doc-view-open-text'
@ 2024-07-22  8:26 Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-07-22 18:11 ` Tassilo Horn
  0 siblings, 1 reply; 13+ messages in thread
From: Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-07-22  8:26 UTC (permalink / raw)
  To: 72241; +Cc: Tassilo Horn

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



Hi,

Here is a patch for DocView that makes it use a dedicated buffer for the
text representation of a document.  This is what was suggested by Stefan
M. in a comment (circa 2019).


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Use-a-dedicated-buffer-for-doc-view-open-text.patch --]
[-- Type: text/x-patch, Size: 4073 bytes --]

From 03d83f95c9a6502bf6b85d0b14e47022cf29bd3d Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Sun, 21 Jul 2024 18:52:52 +0200
Subject: [PATCH] Use a dedicated buffer for `doc-view-open-text'

* lisp/doc-view.el (doc-view-open-text): Create a new "doc's
contents" buffer an switch to it.
(doc-view-toggle-display): Switch back to the document buffer and
kill the "doc's contents" one.
---
 lisp/doc-view.el | 60 +++++++++++++++++++-----------------------------
 1 file changed, 24 insertions(+), 36 deletions(-)

diff --git a/lisp/doc-view.el b/lisp/doc-view.el
index 801783bd766..6313ee83857 100644
--- a/lisp/doc-view.el
+++ b/lisp/doc-view.el
@@ -1768,34 +1768,25 @@ doc-view-open-text
     (let ((txt (expand-file-name "doc.txt" (doc-view--current-cache-dir)))
           (page (doc-view-current-page)))
       (if (file-readable-p txt)
-	  (let ((inhibit-read-only t)
-		(buffer-undo-list t)
-		(dv-bfn doc-view--buffer-file-name))
-	    (erase-buffer)
-            ;; FIXME: Replacing the buffer's PDF content with its txt rendering
-            ;; is pretty risky.  We should probably use *another*
-            ;; buffer instead, so there's much less risk of
-            ;; overwriting the PDF file with some text rendering.
-	    (set-buffer-multibyte t)
-	    (insert-file-contents txt)
-	    (doc-view--text-view-mode)
-	    (setq-local doc-view--buffer-file-name dv-bfn)
-	    (set-buffer-modified-p nil)
-	    (doc-view-minor-mode)
-            (goto-char (point-min))
-            ;; Put point at the start of the page the user was
-            ;; reading.  Pages are separated by Control-L characters.
-            (re-search-forward page-delimiter nil t (1- page))
-	    (add-hook 'write-file-functions
-		      (lambda ()
-                        ;; FIXME: If the user changes major mode and then
-                        ;; saves the buffer, the PDF file will be clobbered
-                        ;; with its txt rendering!
-			(when (eq major-mode 'doc-view--text-view-mode)
-			  (error "Cannot save text contents of document %s"
-				 buffer-file-name)))
-		      nil t))
-	(doc-view-doc->txt txt 'doc-view-open-text)))))
+          (let ((dv-bfn doc-view--buffer-file-name)
+                (dv-text-buffer-name (format "%s/text" (buffer-name))))
+            ;; Prepare the text buffer
+            (with-current-buffer (get-buffer-create dv-text-buffer-name)
+              (let ((inhibit-read-only t)
+                    (buffer-undo-list t))
+                (erase-buffer)
+                (set-buffer-multibyte t)
+                (insert-file-contents txt)
+                (doc-view--text-view-mode)
+                (setq-local doc-view--buffer-file-name dv-bfn)
+                (set-buffer-modified-p nil)
+                (doc-view-minor-mode)
+                (goto-char (point-min))
+                ;; Put point at the start of the page the user was
+                ;; reading.  Pages are separated by Control-L characters.
+                (re-search-forward page-delimiter nil t (1- page))))
+            (switch-to-buffer (get-buffer dv-text-buffer-name)))
+        (doc-view-doc->txt txt 'doc-view-open-text)))))
 
 ;;;;; Toggle between editing and viewing
 
@@ -1816,14 +1807,11 @@ doc-view-toggle-display
     (doc-view-fallback-mode)
     (doc-view-minor-mode 1))
    ((eq major-mode 'doc-view--text-view-mode)
-    (let ((buffer-undo-list t))
-      ;; We're currently viewing the document's text contents, so switch
-      ;; back to .
-      (setq buffer-read-only nil)
-      (insert-file-contents doc-view--buffer-file-name nil nil nil t)
-      (doc-view-fallback-mode)
-      (doc-view-minor-mode 1)
-      (set-buffer-modified-p nil)))
+    ;; We're currently viewing the document's text contents, switch to
+    ;; the buffer visiting the real document and kill myself.
+    (let ((dv-buffer (find-buffer-visiting doc-view--buffer-file-name)))
+      (kill-buffer)
+      (switch-to-buffer dv-buffer)))
    (t
     ;; Switch to doc-view-mode
     (when (and (buffer-modified-p)
-- 
2.45.2


[-- Attachment #3: Type: text/plain, Size: 8185 bytes --]



In GNU Emacs 31.0.50 (build 27, x86_64-unknown-openbsd7.5) of 2024-07-22
 built on computer
Repository revision: 03d83f95c9a6502bf6b85d0b14e47022cf29bd3d
Repository branch: mgi/doc-view
Windowing system distributor 'The X.Org Foundation', version 11.0.12101013
System Description: OpenBSD computer 7.5 GENERIC.MP#198 amd64

Configured using:
 'configure CC=egcc CPPFLAGS=-I/usr/local/include
 LDFLAGS=-L/usr/local/lib MAKEINFO=gmakeinfo --prefix=/home/manuel/emacs
 --bindir=/home/manuel/bin --with-x-toolkit=no --without-cairo
 --without-compress-install'

Configured features:
DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG LCMS2 LIBOTF
LIBXML2 MODULES NOTIFY KQUEUE OLDXMENU PDUMPER PNG RSVG SQLITE3 THREADS
TIFF TREE_SITTER WEBP X11 XDBE XFT XIM XINPUT2 XPM ZLIB

Important settings:
  value of $LC_CTYPE: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Diff

Minor modes in effect:
  whitespace-mode: t
  display-time-mode: t
  display-battery-mode: t
  desktop-save-mode: t
  exwm-randr-mode: t
  server-mode: t
  electric-pair-mode: t
  override-global-mode: t
  repeat-mode: t
  global-eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  minibuffer-regexp-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
/home/manuel/prog/elisp/exwm/exwm-randr hides /home/manuel/.emacs.d/elpa/exwm-0.31/exwm-randr
/home/manuel/prog/elisp/exwm/exwm hides /home/manuel/.emacs.d/elpa/exwm-0.31/exwm
/home/manuel/prog/elisp/exwm/exwm-xsettings hides /home/manuel/.emacs.d/elpa/exwm-0.31/exwm-xsettings
/home/manuel/prog/elisp/exwm/exwm-xim hides /home/manuel/.emacs.d/elpa/exwm-0.31/exwm-xim
/home/manuel/prog/elisp/exwm/exwm-workspace hides /home/manuel/.emacs.d/elpa/exwm-0.31/exwm-workspace
/home/manuel/prog/elisp/exwm/exwm-systemtray hides /home/manuel/.emacs.d/elpa/exwm-0.31/exwm-systemtray
/home/manuel/prog/elisp/exwm/exwm-manage hides /home/manuel/.emacs.d/elpa/exwm-0.31/exwm-manage
/home/manuel/prog/elisp/exwm/exwm-layout hides /home/manuel/.emacs.d/elpa/exwm-0.31/exwm-layout
/home/manuel/prog/elisp/exwm/exwm-input hides /home/manuel/.emacs.d/elpa/exwm-0.31/exwm-input
/home/manuel/prog/elisp/exwm/exwm-floating hides /home/manuel/.emacs.d/elpa/exwm-0.31/exwm-floating
/home/manuel/prog/elisp/exwm/exwm-core hides /home/manuel/.emacs.d/elpa/exwm-0.31/exwm-core
/home/manuel/prog/elisp/exwm/exwm-config hides /home/manuel/.emacs.d/elpa/exwm-0.31/exwm-config
/home/manuel/prog/elisp/exwm/exwm-background hides /home/manuel/.emacs.d/elpa/exwm-0.31/exwm-background
/home/manuel/.emacs.d/elpa/ef-themes-1.7.0/theme-loaddefs hides /home/manuel/emacs/share/emacs/31.0.50/lisp/theme-loaddefs

Features:
(shadow sort mail-extr smerge-mode diff whitespace pulse emacsbug imenu
display-line-numbers bookmark cal-china lunar solar cal-dst cal-bahai
cal-islam cal-hebrew cal-julian holidays holiday-loaddefs cal-iso
face-remap texinfo texinfo-loaddefs org-indent org-agenda flymake-cc
flymake warnings python conf-mode oc-basic org-element org-persist
org-id org-element-ast inline avl-tree ol-eww eww url-queue mm-url
ol-rmail ol-mhe ol-irc ol-info ol-gnus nnselect ol-docview doc-view
filenotify jka-compr image-mode exif ol-bibtex bibtex ol-bbdb ol-w3m
ol-doi org-link-doi gnus-icalendar org-capture org-refile org ob
ob-tangle ob-ref ob-lob ob-table ob-exp org-macro org-src ob-comint
org-pcomplete org-list org-footnote org-faces org-entities org-version
ob-emacs-lisp ob-core ob-eval org-cycle org-table ol org-fold
org-fold-core org-keys oc org-loaddefs org-compat org-macs vc-cvs vc-rcs
log-view pcvs-util make-mode view sh-script smie treesit executable
mule-util on-screen gnus-dired bug-reference vc-git diff-mode
track-changes vc-dir ewoc vc vc-dispatcher time battery cus-load desktop
frameset exwm-randr xcb-randr exwm exwm-input xcb-keysyms xcb-xkb
exwm-manage exwm-floating xcb-cursor xcb-render exwm-layout
exwm-workspace exwm-core xcb-ewmh xcb-icccm xcb xcb-xproto xcb-types
xcb-debug server ef-kassio-theme ef-themes modus-operandi-theme
modus-themes zone speed-type url-http url-auth url-gw nsm ytdious
mpdired transmission color calc-bin calc-ext calc calc-loaddefs rect
calc-macs supercite regi ebdb-gnus gnus-msg gnus-art mm-uu mml2015
mm-view mml-smime smime gnutls dig gnus-sum shr pixel-fill kinsoku
url-file svg dom gnus-group gnus-undo gnus-start gnus-dbus dbus xml
gnus-cloud nnimap nnmail mail-source utf7 nnoo gnus-spec gnus-int
gnus-range gnus-win ebdb-message message sendmail yank-media puny rfc822
mml mml-sec epa epg rfc6068 epg-config mm-decode mm-bodies mm-encode
mail-parse rfc2231 rfc2047 rfc2045 ietf-drums gmm-utils mailheader
ebdb-mua ebdb-com crm ebdb-format ebdb mailabbrev eieio-opt speedbar
ezimage dframe find-func eieio-base timezone icalendar gnus nnheader
gnus-util mail-utils range mm-util mail-prsvr wid-edit web-mode derived
disp-table erlang-start skeleton cc-mode cc-fonts cc-guess cc-menus
cc-cmds cc-styles cc-align cc-engine cc-vars cc-defs slime-asdf grep
slime-tramp tramp rx trampver tramp-integration files-x tramp-message
tramp-compat xdg shell pcomplete parse-time iso8601 time-date
format-spec tramp-loaddefs slime-fancy slime-indentation slime-cl-indent
cl-indent slime-trace-dialog slime-fontifying-fu slime-package-fu
slime-references slime-compiler-notes-tree advice slime-scratch
slime-presentations bridge slime-macrostep macrostep compat
slime-mdot-fu slime-enclosing-context slime-fuzzy slime-fancy-trace
slime-fancy-inspector slime-c-p-c slime-editing-commands slime-autodoc
slime-repl slime-parse slime apropos compile text-property-search etags
fileloop generator xref project arc-mode archive-mode noutline outline
pp comint ansi-osc ansi-color ring hyperspec thingatpt elec-pair edmacro
kmacro use-package-bind-key bind-key appt diary-lib diary-loaddefs
cal-menu calendar cal-loaddefs pcase dired-x dired-aux dired
dired-loaddefs use-package-core repeat easy-mmode calfw-autoloads
calfw-cal-autoloads calfw-org-autoloads debbugs-autoloads ebdb-autoloads
cl-extra help-mode ef-themes-autoloads exwm-autoloads
hyperbole-autoloads kotl-autoloads hact set hhist on-screen-autoloads
osm-autoloads rust-mode-autoloads info slime-autoloads
macrostep-autoloads speed-type-autoloads transmission-autoloads
xelb-autoloads ytdious-autoloads package browse-url url url-proxy
url-privacy url-expand url-methods url-history url-cookie
generate-lisp-file url-domsuf url-util mailcap url-handlers url-parse
auth-source cl-seq eieio eieio-core cl-macs icons password-cache json
subr-x map byte-opt gv bytecomp byte-compile url-vars cl-loaddefs cl-lib
rmc iso-transl tooltip cconv eldoc paren electric uniquify ediff-hook
vc-hooks lisp-float-type elisp-mode mwheel term/x-win x-win
term/common-win x-dnd touch-screen tool-bar dnd fontset image regexp-opt
fringe tabulated-list replace newcomment text-mode lisp-mode prog-mode
register page tab-bar menu-bar rfn-eshadow isearch easymenu timer select
scroll-bar mouse jit-lock font-lock syntax font-core term/tty-colors
frame minibuffer nadvice seq simple cl-generic indonesian philippine
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure
cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp
files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget keymap hashtable-print-readable backquote
threads dbusbind kqueue lcms2 dynamic-setting system-font-setting
font-render-setting xinput2 x multi-tty move-toolbar
make-network-process emacs)

Memory information:
((conses 16 933704 633678) (symbols 48 53527 2)
 (strings 32 250981 60259) (string-bytes 1 6331174)
 (vectors 16 153303) (vector-slots 8 2126367 44503)
 (floats 8 1075 1790) (intervals 56 28683 309) (buffers 992 146))

-- 
Manuel Giraud

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* bug#72241: 31.0.50; [PATCH] Use a dedicated buffer for `doc-view-open-text'
  2024-07-22  8:26 bug#72241: 31.0.50; [PATCH] Use a dedicated buffer for `doc-view-open-text' Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-07-22 18:11 ` Tassilo Horn
  2024-07-22 18:29   ` Eli Zaretskii
  2024-07-23  7:55   ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 2 replies; 13+ messages in thread
From: Tassilo Horn @ 2024-07-22 18:11 UTC (permalink / raw)
  To: 72241, Manuel Giraud

Manuel Giraud <manuel@ledu-giraud.fr> writes:

Hi Manuel,

> Here is a patch for DocView that makes it use a dedicated buffer for
> the text representation of a document.  This is what was suggested by
> Stefan M. in a comment (circa 2019).

I think that's a good idea in general and addresses a FIXME.

Since it doesn't fix a bug, I think it should only be applied to master.

Oh, and in the commit message there's an "an" where an "and" was meant.
And maybe a "text-contents buffer" is a slightly better term than "doc's
contents buffer".

I'm not sure if a NEWS entry is needed for that change.  It certainly
changes existing behavior but not in a way that usage patterns/muscle
memory would need to be adapted...

(info "(emacs) Document View") might need a small update for the
description where some requirement is not met (e.g., emacs in a TTY
frame) where it says that "the (document) buffer" is put in text mode
when one answers the query if the contents should be shown as plain text
with yes.

Bye,
  Tassilo





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#72241: 31.0.50; [PATCH] Use a dedicated buffer for `doc-view-open-text'
  2024-07-22 18:11 ` Tassilo Horn
@ 2024-07-22 18:29   ` Eli Zaretskii
  2024-07-22 18:39     ` Tassilo Horn
  2024-07-23  7:55   ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2024-07-22 18:29 UTC (permalink / raw)
  To: Tassilo Horn; +Cc: 72241, manuel

> From: Tassilo Horn <tsdh@gnu.org>
> Date: Mon, 22 Jul 2024 20:11:34 +0200
> 
> I'm not sure if a NEWS entry is needed for that change.  It certainly
> changes existing behavior but not in a way that usage patterns/muscle
> memory would need to be adapted...

Can you tell in what way the behavior will change after this?  I'd
like to think whether a NEWS entry is necessary and what to day there.

Thanks.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#72241: 31.0.50; [PATCH] Use a dedicated buffer for `doc-view-open-text'
  2024-07-22 18:29   ` Eli Zaretskii
@ 2024-07-22 18:39     ` Tassilo Horn
  2024-07-22 18:48       ` Eli Zaretskii
  2024-07-23  8:10       ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 2 replies; 13+ messages in thread
From: Tassilo Horn @ 2024-07-22 18:39 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 72241, manuel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Tassilo Horn <tsdh@gnu.org>
>> Date: Mon, 22 Jul 2024 20:11:34 +0200
>> 
>> I'm not sure if a NEWS entry is needed for that change.  It certainly
>> changes existing behavior but not in a way that usage patterns/muscle
>> memory would need to be adapted...
>
> Can you tell in what way the behavior will change after this?  I'd
> like to think whether a NEWS entry is necessary and what to day there.

Right now, when you open foo.pdf you see the images generated from the
PDF.  When you do C-c C-t, it'll replace the foo.pdf buffer contents
with the plain text contents of the PDF.  With another C-c C-c, the
contents are again replaced with the PDF and you see the images again.

With Manuel's patch, C-c C-t shows the plain text contents in a separate
buffer foo.pdf/text.  C-c C-c in there kills that separate buffer and
switches back to the original foo.pdf buffer.

So basically everything you have to do stays the same but there are now
two instead of just one buffer.  That has the advantage that you can
switch between them and removes the possibility that you accidentally
overwrite your PDF with the plain text contents (which in what Stefan's
FIXME was about).

Bye,
  Tassilo





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#72241: 31.0.50; [PATCH] Use a dedicated buffer for `doc-view-open-text'
  2024-07-22 18:39     ` Tassilo Horn
@ 2024-07-22 18:48       ` Eli Zaretskii
  2024-07-23  8:10       ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 0 replies; 13+ messages in thread
From: Eli Zaretskii @ 2024-07-22 18:48 UTC (permalink / raw)
  To: Tassilo Horn; +Cc: 72241, manuel

> From: Tassilo Horn <tsdh@gnu.org>
> Cc: 72241@debbugs.gnu.org,  manuel@ledu-giraud.fr
> Date: Mon, 22 Jul 2024 20:39:54 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Can you tell in what way the behavior will change after this?  I'd
> > like to think whether a NEWS entry is necessary and what to day there.
> 
> Right now, when you open foo.pdf you see the images generated from the
> PDF.  When you do C-c C-t, it'll replace the foo.pdf buffer contents
> with the plain text contents of the PDF.  With another C-c C-c, the
> contents are again replaced with the PDF and you see the images again.
> 
> With Manuel's patch, C-c C-t shows the plain text contents in a separate
> buffer foo.pdf/text.  C-c C-c in there kills that separate buffer and
> switches back to the original foo.pdf buffer.
> 
> So basically everything you have to do stays the same but there are now
> two instead of just one buffer.  That has the advantage that you can
> switch between them and removes the possibility that you accidentally
> overwrite your PDF with the plain text contents (which in what Stefan's
> FIXME was about).

Thanks, I think this is enough in users' faces to warrant a NEWS
entry.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#72241: 31.0.50; [PATCH] Use a dedicated buffer for `doc-view-open-text'
  2024-07-22 18:11 ` Tassilo Horn
  2024-07-22 18:29   ` Eli Zaretskii
@ 2024-07-23  7:55   ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 0 replies; 13+ messages in thread
From: Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-07-23  7:55 UTC (permalink / raw)
  To: Tassilo Horn; +Cc: 72241

Tassilo Horn <tsdh@gnu.org> writes:

> Manuel Giraud <manuel@ledu-giraud.fr> writes:
>
> Hi Manuel,
>
>> Here is a patch for DocView that makes it use a dedicated buffer for
>> the text representation of a document.  This is what was suggested by
>> Stefan M. in a comment (circa 2019).
>
> I think that's a good idea in general and addresses a FIXME.
>
> Since it doesn't fix a bug, I think it should only be applied to
> master.

Yes, I also do think so.

> Oh, and in the commit message there's an "an" where an "and" was meant.
> And maybe a "text-contents buffer" is a slightly better term than "doc's
> contents buffer".

Ok.  I'll fix that.

[...]

> (info "(emacs) Document View") might need a small update for the
> description where some requirement is not met (e.g., emacs in a TTY
> frame) where it says that "the (document) buffer" is put in text mode
> when one answers the query if the contents should be shown as plain text
> with yes.

I have not tested in a TTY… maybe I should 😅
-- 
Manuel Giraud





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#72241: 31.0.50; [PATCH] Use a dedicated buffer for `doc-view-open-text'
  2024-07-22 18:39     ` Tassilo Horn
  2024-07-22 18:48       ` Eli Zaretskii
@ 2024-07-23  8:10       ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-07-23 10:39         ` Tassilo Horn
  1 sibling, 1 reply; 13+ messages in thread
From: Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-07-23  8:10 UTC (permalink / raw)
  To: Tassilo Horn; +Cc: Eli Zaretskii, 72241

Tassilo Horn <tsdh@gnu.org> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> From: Tassilo Horn <tsdh@gnu.org>
>>> Date: Mon, 22 Jul 2024 20:11:34 +0200
>>> 
>>> I'm not sure if a NEWS entry is needed for that change.  It certainly
>>> changes existing behavior but not in a way that usage patterns/muscle
>>> memory would need to be adapted...
>>
>> Can you tell in what way the behavior will change after this?  I'd
>> like to think whether a NEWS entry is necessary and what to day there.
>
> Right now, when you open foo.pdf you see the images generated from the
> PDF.  When you do C-c C-t, it'll replace the foo.pdf buffer contents
> with the plain text contents of the PDF.  With another C-c C-c, the
> contents are again replaced with the PDF and you see the images again.

FWIW, this is not the current behaviour that I see:

      - C-c C-t replace the buffer contents with the text version
      
      - C-c C-c, DocView switches back to the "edit" view: the raw
        content of a PDF for instance is displayed into the buffer.

      - another C-c C-c, DocView switches to the "display" view where
        you see the images again

Maybe while here, we should clarify how DocView cycles through those 3
view: display, edit and text.  Or maybe, there is no 3-states cycling
and the "text contents" view is just considered a "side" view.  WDYT?
-- 
Manuel Giraud





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#72241: 31.0.50; [PATCH] Use a dedicated buffer for `doc-view-open-text'
  2024-07-23  8:10       ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-07-23 10:39         ` Tassilo Horn
  2024-07-23 12:00           ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 13+ messages in thread
From: Tassilo Horn @ 2024-07-23 10:39 UTC (permalink / raw)
  To: Manuel Giraud; +Cc: Eli Zaretskii, 72241

Manuel Giraud <manuel@ledu-giraud.fr> writes:

Hi,

>>> Can you tell in what way the behavior will change after this?  I'd
>>> like to think whether a NEWS entry is necessary and what to day there.
>>
>> Right now, when you open foo.pdf you see the images generated from
>> the PDF.  When you do C-c C-t, it'll replace the foo.pdf buffer
>> contents with the plain text contents of the PDF.  With another C-c
>> C-c, the contents are again replaced with the PDF and you see the
>> images again.
>
> FWIW, this is not the current behaviour that I see:
>
>       - C-c C-t replace the buffer contents with the text version
>       
>       - C-c C-c, DocView switches back to the "edit" view: the raw
>         content of a PDF for instance is displayed into the buffer.
>
>       - another C-c C-c, DocView switches to the "display" view where
>         you see the images again

That's right and my answer was aproximately correct.  The beef is that
for the "text version", you get a separate buffer with your patch.
"edit" and "display" are in the same buffer because the contents (raw
PDF, DVI, PostScript,...) are the same, just how they are presented to
the user differs.

> Maybe while here, we should clarify how DocView cycles through those 3
> view: display, edit and text.  Or maybe, there is no 3-states cycling
> and the "text contents" view is just considered a "side" view.  WDYT?

Here's a state machine describing the current behavior.

    display <-- C-c C-c --> edit
    \     ^
     \     \    
 C-c C-t  C-c C-c
       \     \
        `> text

So with C-c C-c you can toggle between "display" and "edit" which just
changes how the buffer contents are presented to the user (images or raw
text).

In the "display" state C-c C-t one gets a plain text version of the
contents, and that's in a separate foo.pdf/text buffer with Manuel's
patch.  In there, C-c C-c kills the foo.pdf/text buffer and returns to
the original foo.pdf buffer.

However, that now one can switch between foo.pdf and foo.pdf/text
independently, there is no guarantee that C-c C-c in the foo.pdf/text
buffer will return to foo.pdf in "display" state.  One could have
toggled to edit state there or even killed the foo.pdf buffer, so
there's nothing to return to.

So I'd say: right now it is a state machine with 3 states but with
Manuel's patch the current "text" state becomes an auxiliary view.

Bye,
  Tassilo





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#72241: 31.0.50; [PATCH] Use a dedicated buffer for `doc-view-open-text'
  2024-07-23 10:39         ` Tassilo Horn
@ 2024-07-23 12:00           ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-07-23 12:16             ` Tassilo Horn
  0 siblings, 1 reply; 13+ messages in thread
From: Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-07-23 12:00 UTC (permalink / raw)
  To: Tassilo Horn; +Cc: Eli Zaretskii, 72241

Tassilo Horn <tsdh@gnu.org> writes:

[...]

>> Maybe while here, we should clarify how DocView cycles through those 3
>> view: display, edit and text.  Or maybe, there is no 3-states cycling
>> and the "text contents" view is just considered a "side" view.  WDYT?
>
> Here's a state machine describing the current behavior.
>
>     display <-- C-c C-c --> edit
>     \     ^
>      \     \
>  C-c C-t  C-c C-c
>        \     \
>         `> text

I think the current behavior is more like this (I suck at ASCII art):

    display <-- C-c C-c --> edit
    \                      ^
     \                  /--
 C-c C-t            C-c C-c
       \        /--
        `> text

Your state machine is what my patch gives you.

[...]

> However, that now one can switch between foo.pdf and foo.pdf/text
> independently, there is no guarantee that C-c C-c in the foo.pdf/text
> buffer will return to foo.pdf in "display" state.  One could have
> toggled to edit state there or even killed the foo.pdf buffer, so
> there's nothing to return to.

Yes, you're right.  In the latter case (the foo.pdf buffer was killed
otherwise), a C-c C-c in the foo.pdf/text buffer just kill it, go to
another buffer and the document is not open anywhere in any form.  Maybe
it is a not so bad behavior.

> So I'd say: right now it is a state machine with 3 states but with
> Manuel's patch the current "text" state becomes an auxiliary view.

Yes.  So I'm going to try to fix my patch with your remarks (maybe fix
the info also) and come up with a NEWS entry.  Thanks.
-- 
Manuel Giraud





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#72241: 31.0.50; [PATCH] Use a dedicated buffer for `doc-view-open-text'
  2024-07-23 12:00           ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-07-23 12:16             ` Tassilo Horn
  2024-07-23 13:45               ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-07-23 14:32               ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 2 replies; 13+ messages in thread
From: Tassilo Horn @ 2024-07-23 12:16 UTC (permalink / raw)
  To: Manuel Giraud; +Cc: Eli Zaretskii, 72241

Manuel Giraud <manuel@ledu-giraud.fr> writes:

>> Here's a state machine describing the current behavior.
>>
>>     display <-- C-c C-c --> edit
>>     \     ^
>>      \     \
>>  C-c C-t  C-c C-c
>>        \     \
>>         `> text
>
> I think the current behavior is more like this (I suck at ASCII art):
>
>     display <-- C-c C-c --> edit
>     \                      ^
>      \                  /--
>  C-c C-t            C-c C-c
>        \        /--
>         `> text

You are right.

>> However, that now one can switch between foo.pdf and foo.pdf/text
>> independently, there is no guarantee that C-c C-c in the foo.pdf/text
>> buffer will return to foo.pdf in "display" state.  One could have
>> toggled to edit state there or even killed the foo.pdf buffer, so
>> there's nothing to return to.
>
> Yes, you're right.  In the latter case (the foo.pdf buffer was killed
> otherwise), a C-c C-c in the foo.pdf/text buffer just kill it, go to
> another buffer and the document is not open anywhere in any form.
> Maybe it is a not so bad behavior.

No, I think that's ok.

>> So I'd say: right now it is a state machine with 3 states but with
>> Manuel's patch the current "text" state becomes an auxiliary view.
>
> Yes.  So I'm going to try to fix my patch with your remarks (maybe fix
> the info also) and come up with a NEWS entry.  Thanks.

Great, thanks!

Bye,
  Tassilo





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#72241: 31.0.50; [PATCH] Use a dedicated buffer for `doc-view-open-text'
  2024-07-23 12:16             ` Tassilo Horn
@ 2024-07-23 13:45               ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-07-23 14:32               ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 0 replies; 13+ messages in thread
From: Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-07-23 13:45 UTC (permalink / raw)
  To: Tassilo Horn; +Cc: Eli Zaretskii, 72241

Tassilo Horn <tsdh@gnu.org> writes:

[...]

>> Yes.  So I'm going to try to fix my patch with your remarks (maybe fix
>> the info also) and come up with a NEWS entry.  Thanks.
>
> Great, thanks!

Hi again,

I don't think I'm going to modify the info page with this patch.  I've
just tested on TTY emacs to open a PDF and both without and with my
patch DocView does not load and the buffer stays in fundamental-mode.
So I guess, we need more work in this area.
-- 
Manuel Giraud





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#72241: 31.0.50; [PATCH] Use a dedicated buffer for `doc-view-open-text'
  2024-07-23 12:16             ` Tassilo Horn
  2024-07-23 13:45               ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-07-23 14:32               ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-07-25  6:25                 ` Tassilo Horn
  1 sibling, 1 reply; 13+ messages in thread
From: Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-07-23 14:32 UTC (permalink / raw)
  To: Tassilo Horn; +Cc: Eli Zaretskii, 72241

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

Here is an updated version of this patch.  WDYT?

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Use-a-dedicated-buffer-for-doc-view-open-text.patch --]
[-- Type: text/x-patch, Size: 4704 bytes --]

From 6e32534012cafeda1d7e67aab23a8206bc887c9f Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Sun, 21 Jul 2024 18:52:52 +0200
Subject: [PATCH] Use a dedicated buffer for `doc-view-open-text'

* lisp/doc-view.el (doc-view-open-text): Create a new "text
contents" buffer and switch to it.
(doc-view-toggle-display): Switch back to the document buffer
and kill the "text contents" one.
* etc/NEWS: Mention the change.
---
 etc/NEWS         |  7 ++++++
 lisp/doc-view.el | 60 +++++++++++++++++++-----------------------------
 2 files changed, 31 insertions(+), 36 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index d683db606ec..81382200eef 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -149,6 +149,13 @@ This affects calls to 'warn', 'lwarn', 'display-warning', and
 In most cases, having it enabled leads to a large amount of false
 positives.
 
+** DocView
+
+---
+*** Dedicated buffer for plain text contents.
+When switching to the plain text contents with 'doc-view-open-text',
+DocView now creates a dedicated buffer to display it. 'C-c C-c' gets you
+back to real DocView buffer if it still exists.
 \f
 * New Modes and Packages in Emacs 31.1
 
diff --git a/lisp/doc-view.el b/lisp/doc-view.el
index 801783bd766..6313ee83857 100644
--- a/lisp/doc-view.el
+++ b/lisp/doc-view.el
@@ -1768,34 +1768,25 @@ doc-view-open-text
     (let ((txt (expand-file-name "doc.txt" (doc-view--current-cache-dir)))
           (page (doc-view-current-page)))
       (if (file-readable-p txt)
-	  (let ((inhibit-read-only t)
-		(buffer-undo-list t)
-		(dv-bfn doc-view--buffer-file-name))
-	    (erase-buffer)
-            ;; FIXME: Replacing the buffer's PDF content with its txt rendering
-            ;; is pretty risky.  We should probably use *another*
-            ;; buffer instead, so there's much less risk of
-            ;; overwriting the PDF file with some text rendering.
-	    (set-buffer-multibyte t)
-	    (insert-file-contents txt)
-	    (doc-view--text-view-mode)
-	    (setq-local doc-view--buffer-file-name dv-bfn)
-	    (set-buffer-modified-p nil)
-	    (doc-view-minor-mode)
-            (goto-char (point-min))
-            ;; Put point at the start of the page the user was
-            ;; reading.  Pages are separated by Control-L characters.
-            (re-search-forward page-delimiter nil t (1- page))
-	    (add-hook 'write-file-functions
-		      (lambda ()
-                        ;; FIXME: If the user changes major mode and then
-                        ;; saves the buffer, the PDF file will be clobbered
-                        ;; with its txt rendering!
-			(when (eq major-mode 'doc-view--text-view-mode)
-			  (error "Cannot save text contents of document %s"
-				 buffer-file-name)))
-		      nil t))
-	(doc-view-doc->txt txt 'doc-view-open-text)))))
+          (let ((dv-bfn doc-view--buffer-file-name)
+                (dv-text-buffer-name (format "%s/text" (buffer-name))))
+            ;; Prepare the text buffer
+            (with-current-buffer (get-buffer-create dv-text-buffer-name)
+              (let ((inhibit-read-only t)
+                    (buffer-undo-list t))
+                (erase-buffer)
+                (set-buffer-multibyte t)
+                (insert-file-contents txt)
+                (doc-view--text-view-mode)
+                (setq-local doc-view--buffer-file-name dv-bfn)
+                (set-buffer-modified-p nil)
+                (doc-view-minor-mode)
+                (goto-char (point-min))
+                ;; Put point at the start of the page the user was
+                ;; reading.  Pages are separated by Control-L characters.
+                (re-search-forward page-delimiter nil t (1- page))))
+            (switch-to-buffer (get-buffer dv-text-buffer-name)))
+        (doc-view-doc->txt txt 'doc-view-open-text)))))
 
 ;;;;; Toggle between editing and viewing
 
@@ -1816,14 +1807,11 @@ doc-view-toggle-display
     (doc-view-fallback-mode)
     (doc-view-minor-mode 1))
    ((eq major-mode 'doc-view--text-view-mode)
-    (let ((buffer-undo-list t))
-      ;; We're currently viewing the document's text contents, so switch
-      ;; back to .
-      (setq buffer-read-only nil)
-      (insert-file-contents doc-view--buffer-file-name nil nil nil t)
-      (doc-view-fallback-mode)
-      (doc-view-minor-mode 1)
-      (set-buffer-modified-p nil)))
+    ;; We're currently viewing the document's text contents, switch to
+    ;; the buffer visiting the real document and kill myself.
+    (let ((dv-buffer (find-buffer-visiting doc-view--buffer-file-name)))
+      (kill-buffer)
+      (switch-to-buffer dv-buffer)))
    (t
     ;; Switch to doc-view-mode
     (when (and (buffer-modified-p)
-- 
2.45.2


[-- Attachment #3: Type: text/plain, Size: 18 bytes --]

-- 
Manuel Giraud

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* bug#72241: 31.0.50; [PATCH] Use a dedicated buffer for `doc-view-open-text'
  2024-07-23 14:32               ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-07-25  6:25                 ` Tassilo Horn
  0 siblings, 0 replies; 13+ messages in thread
From: Tassilo Horn @ 2024-07-25  6:25 UTC (permalink / raw)
  To: Manuel Giraud; +Cc: Eli Zaretskii, 72241-done

Manuel Giraud <manuel@ledu-giraud.fr> writes:

Hi Manuel,

> Here is an updated version of this patch.  WDYT?

Looks and works good.  Applied and pushed to master.  I have also
adjusted the info docs slightly so that it says that the text contents
are displayed in a separate buffed with /text suffix.

Thanks again!
  Tassilo





^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2024-07-25  6:25 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-22  8:26 bug#72241: 31.0.50; [PATCH] Use a dedicated buffer for `doc-view-open-text' Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-07-22 18:11 ` Tassilo Horn
2024-07-22 18:29   ` Eli Zaretskii
2024-07-22 18:39     ` Tassilo Horn
2024-07-22 18:48       ` Eli Zaretskii
2024-07-23  8:10       ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-07-23 10:39         ` Tassilo Horn
2024-07-23 12:00           ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-07-23 12:16             ` Tassilo Horn
2024-07-23 13:45               ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-07-23 14:32               ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-07-25  6:25                 ` Tassilo Horn
2024-07-23  7:55   ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors

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).