bug#18010: eww: desktop support

bug#18010: eww: desktop support

[Ivan Shmakov]

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

Package:  emacs
Severity: wishlist
Tags:     patch
Control:  block -1 by 16211
Control:  tags 16211 + patch

	Assuming that #16211 is resolved, adding desktop support to EWW
	seems pretty natural.  (Why, Info already has one.)

	One thing to save is, obviously, eww-current-url.  The other is
	eww-history, but that has to be filtered for the overly bulky
	:text, :dom, and :source properties.

	The last part poses a problem, as eww-restore-history does /not/
	currently handle the case where :text is missing.  A possible
	solution is to use (eww-reload) if :text is nil.  As a side
	effect, eww-current-source and eww-current-dom will also be set.

	Also, AIUI, for the EWW history facility to work properly,
	eww-history-position is also to be saved, which is possible via
	desktop-locals-to-save.

	For anyone eager to try, the patch I suggest is MIMEd.

	This change looks quite substantial to be copyrightable, so I’d
	like to explicitly disclaim copyright on it, as per
	CC0 Public Domain Dedication [0].

	For the most part, eww-desktop-history-1 function is a kind of
	‘reduce’ or ‘remove-if’ for property lists.  Alas, I didn’t find
	any existing function for that, so I coded it the explicit way.

-- 
FSF associate member #7257	http://boycottsystemd.org/

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-diff, Size: 3233 bytes --]

diff --git a/lisp/net/eww.el b/lisp/net/eww.el
index 02fc575..f6ee185 100644
--- a/lisp/net/eww.el
+++ b/lisp/net/eww.el
@@ -479,6 +495,8 @@ word(s) will be searched for via `eww-search-prefix'."
   (setq-local eww-history-position 0)
   (when (boundp 'tool-bar-map)
    (setq-local tool-bar-map eww-tool-bar-map))
+  ;; desktop support
+  (setq-local desktop-save-buffer 'eww-desktop-misc-data)
   (buffer-disable-undo)
   ;;(setq buffer-read-only t)
   )
@@ -514,15 +533,22 @@ word(s) will be searched for via `eww-search-prefix'."
   (eww-restore-history (elt eww-history (1- eww-history-position))))
 
 (defun eww-restore-history (elem)
-  (let ((inhibit-read-only t))
-    (erase-buffer)
-    (insert (plist-get elem :text))
-    (setq eww-current-source (plist-get elem :source))
-    (setq eww-current-dom (plist-get elem :dom))
-    (goto-char (plist-get elem :point))
-    (setq eww-current-url (plist-get elem :url)
-	  eww-current-title (plist-get elem :title))
-    (eww-update-header-line-format)))
+  (let ((inhibit-read-only t)
+	(text (plist-get elem :text))
+	(pos  (plist-get elem :point)))
+    (setq eww-current-source  (plist-get elem :source)
+	  eww-current-dom     (plist-get elem :dom)
+	  eww-current-url     (plist-get elem :url))
+    (if (null text)
+	(eww-reload)
+      (erase-buffer)
+      (insert text)
+      (setq eww-current-title
+	    (plist-get elem :title))
+      (eww-update-header-line-format))
+    ;; FIXME: pos may no longer match the contents if the page gets reloaded
+    (when pos
+      (goto-char pos))))
 
 (defun eww-next-url ()
   "Go to the page marked `next'.
@@ -1343,6 +1371,48 @@ Differences in #targets are ignored."
   (setq buffer-read-only t
 	truncate-lines t))
 
+;;; Desktop support
+
+(defvar eww-desktop-history-save
+  '(:url :title :point)
+  "List of `eww-history' values to preserve in the desktop file.")
+
+(defun eww-desktop-history-1 (alist)
+  (let ((acc  nil)
+        (tail alist))
+    (while tail
+      (let ((k (car  tail))
+            (v (cadr tail)))
+        (when (memq k eww-desktop-history-save)
+          (setq acc (cons k (cons v acc)))))
+      (setq tail  (cddr tail)))
+    acc))
+
+(defun eww-desktop-misc-data (directory)
+  "Return a property list with data used to restore eww buffers.
+This list will contain the URI to browse as the :uri property, and, as
+:history, a copy of eww-history with the (usually overly large) :dom,
+:source and :text properties removed."
+  (list :history    (mapcar 'eww-desktop-history-1 eww-history)
+        :uri        eww-current-url))
+
+(defun eww-restore-desktop (file-name buffer-name misc-data)
+  "Restore an eww buffer from its desktop file record."
+  (with-current-buffer (get-buffer-create buffer-name)
+    (eww-mode)
+    ;; NB: eww-history is buffer-local per (eww-mode)
+    (setq eww-history   (plist-get :history misc-data))
+    (unless file-name
+      (let ((uri        (plist-get :uri     misc-data)))
+        (when uri
+          (eww uri))))
+    (current-buffer)))
+
+(add-to-list 'desktop-locals-to-save
+	     'eww-history-position)
+(add-to-list 'desktop-buffer-mode-handlers
+             '(eww-mode . eww-restore-desktop))
+
 (provide 'eww)
 
 ;;; eww.el ends here

bug#18010: eww: desktop support

[Ted Zlatanov]

On Sun, 13 Jul 2014 12:17:47 +0000 Ivan Shmakov <ivan@siamics.net> wrote: 

IS> Package:  emacs
IS> Severity: wishlist
IS> Tags:     patch
IS> Control:  block -1 by 16211
IS> Control:  tags 16211 + patch

IS> 	Assuming that #16211 is resolved, adding desktop support to EWW
IS> 	seems pretty natural.  (Why, Info already has one.)

I'm OK with this in principle and think it's nicely useful, but the
patch is too large so Lars will have to review it.

Ted

bug#18010: eww: desktop support

[Lars Magne Ingebrigtsen]

Ivan Shmakov <ivan@siamics.net> writes:

> 	Also, AIUI, for the EWW history facility to work properly,
> 	eww-history-position is also to be saved, which is possible via
> 	desktop-locals-to-save.
>
> 	For anyone eager to try, the patch I suggest is MIMEd.

This looks very nice, but the history saving has changed a bit (earlier
today :-), so the patch won't apply now.  

> 	This change looks quite substantial to be copyrightable, so I’d
> 	like to explicitly disclaim copyright on it, as per
> 	CC0 Public Domain Dedication [0].

But I've never applied a patch of this size that I haven't had paperwork
for.  Could someone advise here?  Would this disclaimal (that should be
a word) suffice?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#18010: eww: desktop support

[Glenn Morris]

Lars Magne Ingebrigtsen wrote:

>> 	This change looks quite substantial to be copyrightable, so I'd
>> 	like to explicitly disclaim copyright on it, as per
>> 	CC0 Public Domain Dedication [0].
>
> But I've never applied a patch of this size that I haven't had paperwork
> for.  Could someone advise here?  Would this disclaimal (that should be
> a word) suffice?

No, it doesn't suffice.

But the OP has an assignment on file now, so just go ahead.

bug#18010: eww: desktop support

[Lars Magne Ingebrigtsen]

Glenn Morris <rgm@gnu.org> writes:

> But the OP has an assignment on file now, so just go ahead.

Great.

Ivan, if you could respin this patch to apply to eww now, I'll apply it.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#18010: eww: desktop support

[Ivan Shmakov]

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

>>>>> Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

 > Ivan, if you could respin this patch to apply to eww now, I'll apply
 > it.

	Please consider the patch MIMEd.  Albeit I’ve tested it only
	somewhat superficially, it does seem to work as intended.

	Beware of the line numbers being slightly offset, for I also
	have other patches applied to eww.el locally.

-- 
FSF associate member #7257  http://boycottsystemd.org/  … 3013 B6A0 230E 334A

[-- Attachment #2: Type: text/plain, Size: 5170 bytes --]

--- a/lisp/net/eww.el
+++ b/lisp/net/eww.el
@@ -65,6 +65,36 @@
   :group 'eww
   :type 'string)
 
+(defcustom eww-desktop-remove-duplicates t
+  "Whether to remove duplicates from the history when saving desktop data.
+If non-nil, repetitive EWW history entries (comprising of the URI, the
+title, and the point position) will not be saved as part of the Emacs
+desktop.  Otherwise, such entries will be retained."
+  :version "24.4"
+  :group 'eww
+  :type 'boolean)
+
+(defcustom eww-restore-desktop nil
+  "How to restore EWW buffers on `desktop-restore'.
+If t or 'auto, the buffers will be reloaded automatically.
+If nil, buffers will require manual reload, and will contain the text
+specified in `eww-restore-reload-prompt' instead of the actual Web
+page contents."
+  :version "24.4"
+  :group 'eww
+  :type '(choice (const :tag "Restore all automatically" t)
+                 (const :tag "Require manual reload" nil)))
+
+(defcustom eww-restore-reload-prompt
+  "\n\n *** Use \\[eww-reload] to reload this buffer. ***\n"
+  "The string to put in the buffers not reloaded on `desktop-restore'.
+This prompt will be used if `eww-restore-desktop' is nil.
+
+The string will be passed through `substitute-command-keys'."
+  :version "24.4"
+  :group 'eww
+  :type 'string)
+
 (defcustom eww-use-external-browser-for-content-type
   "\\`\\(video/\\|audio/\\|application/ogg\\)"
   "Always use external browser for specified content-type."
@@ -583,6 +633,8 @@ define-derived-mode eww-mode nil "eww"
   (setq-local eww-history-position 0)
   (when (boundp 'tool-bar-map)
    (setq-local tool-bar-map eww-tool-bar-map))
+  ;; desktop support
+  (setq-local desktop-save-buffer 'eww-desktop-misc-data)
   (buffer-disable-undo)
   (setq buffer-read-only t))
 
@@ -611,12 +660,15 @@
   (eww-restore-history (elt eww-history (1- eww-history-position))))
 
 (defun eww-restore-history (elem)
-  (let ((inhibit-read-only t))
-    (erase-buffer)
-    (insert (plist-get elem :text))
-    (goto-char (plist-get elem :point))
+  (let ((inhibit-read-only t)
+	(text (plist-get elem :text)))
     (setq eww-data elem)
-    (eww-update-header-line-format)))
+    (if (null text)
+	(eww-reload)			; FIXME: restore :point?
+      (erase-buffer)
+      (insert text)
+      (goto-char (plist-get elem :point))
+      (eww-update-header-line-format))))
 
 (defun eww-next-url ()
   "Go to the page marked `next'.
@@ -1518,6 +1585,82 @@
   (setq buffer-read-only t
 	truncate-lines t))
 
+;;; Desktop support
+
+(defvar eww-desktop-data-save
+  '(:url :title :point)
+  "List of `eww-data' properties to preserve in the desktop file.
+Also used when saving `eww-history'.")
+
+(defun eww-desktop-data-1 (alist)
+  (let ((acc  nil)
+        (tail alist))
+    (while tail
+      (let ((k (car  tail))
+            (v (cadr tail)))
+        (when (memq k eww-desktop-data-save)
+          (setq acc (cons k (cons v acc)))))
+      (setq tail  (cddr tail)))
+    acc))
+
+(defun eww-desktop-history-duplicate (a b)
+  (let ((tail a) (r t))
+    (while tail
+      (if (or (memq (car tail) '(:point)) ; ignore :point
+	      (equal (cadr tail)
+		     (plist-get b (car tail))))
+	  (setq tail (cddr tail))
+	(setq tail nil
+	      r    nil)))
+    ;; .
+    r))
+
+(defun eww-desktop-misc-data (directory)
+  "Return a property list with data used to restore eww buffers.
+This list will contain, as :history, the list, whose first element is
+the value of `eww-data', and the tail is `eww-history'.
+
+If `eww-desktop-remove-duplicates' is non-nil, duplicate
+entries (if any) will be removed from the list.
+
+Only the properties listed in `eww-desktop-data-save' are included.
+Generally, the list should not include the (usually overly large)
+:dom, :source and :text properties."
+  (let ((history  (mapcar 'eww-desktop-data-1
+			  (cons eww-data eww-history))))
+    (list :history  (if eww-desktop-remove-duplicates
+			(remove-duplicates
+			 history :test 'eww-desktop-history-duplicate)
+		      history))))
+
+(defun eww-restore-desktop (file-name buffer-name misc-data)
+  "Restore an eww buffer from its desktop file record.
+If `eww-restore-desktop' is t or 'auto, this function will also
+initiate the retrieval of the respective URI in the background.
+Otherwise, the restored buffer will contain a prompt to do so by using
+\\[eww-reload]."
+  (with-current-buffer (get-buffer-create buffer-name)
+    (eww-mode)
+    ;; NB: eww-history, eww-data are buffer-local per (eww-mode)
+    (setq eww-history       (cdr (plist-get misc-data :history))
+	  eww-data      (or (car (plist-get misc-data :history))
+			    ;; backwards compatibility
+			    (list :url (plist-get misc-data :uri))))
+    (unless file-name
+      (when (plist-get eww-data :url)
+	(case eww-restore-desktop
+	  ((t auto) (eww (plist-get eww-data :url)))
+	  ((zerop (buffer-size))
+	   (insert (substitute-command-keys
+		    eww-restore-reload-prompt))))))
+    ;; .
+    (current-buffer)))
+
+(add-to-list 'desktop-locals-to-save
+	     'eww-history-position)
+(add-to-list 'desktop-buffer-mode-handlers
+             '(eww-mode . eww-restore-desktop))
+
 (provide 'eww)
 
 ;;; eww.el ends here

bug#18010: eww: desktop support

[Lars Magne Ingebrigtsen]

Ivan Shmakov <ivan@siamics.net> writes:

>>>>>> Lars Magne Ingebrigtsen <larsi@gnus.org> writes:
>
>  > Ivan, if you could respin this patch to apply to eww now, I'll apply
>  > it.
>
> 	Please consider the patch MIMEd.  Albeit I’ve tested it only
> 	somewhat superficially, it does seem to work as intended.
>
> 	Beware of the line numbers being slightly offset, for I also
> 	have other patches applied to eww.el locally.

Applied now.  Could you also write a NEWS entry and possibly write
documentation for this?  Either in the eww manual or somewhere in the
desktop manual, I guess.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#18010: eww: desktop support

[Ivan Shmakov]

>>>>> Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

[…]

 > Could you also write a NEWS entry and possibly write documentation
 > for this?

	I’ll try to get to it within the next few days.

 > Either in the eww manual or somewhere in the desktop manual, I guess.

	As it seems, the “Saving Emacs Sessions” (emacs/misc.texi)
	section is only concerned with the facilities of desktop.el
	proper.  Thus the documentation for eww-desktop-* belongs to
	misc/eww.texi.

-- 
FSF associate member #7257  np. Isle of Avalon — Iron Maiden    … 230E 334A

bug#18010: eww: desktop support

[Ivan Shmakov]

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

>>>>> Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

[…]

 > Could you also write a NEWS entry and possibly write documentation
 > for this?  Either in the eww manual or somewhere in the desktop
 > manual, I guess.

	Please consider the patch MIMEd.

	I was unsure where to put the details of the EWW desktop support
	in eww.texi, so I’ve added them to the end of the “Advanced”
	section, as they’re somewhat obscure and unlikely to require
	much of the user’s attention.

	What may require such attention is, however, the possible use of
	(setq desktop-save-buffer nil) to disable this new feature.

-- 
FSF associate member #7257  http://boycottsystemd.org/  … 3013 B6A0 230E 334A

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/diff, Size: 2463 bytes --]

--- a/doc/misc/eww.texi	2014-11-23 10:57:19 +0000
+++ b/doc/misc/eww.texi	2014-11-23 14:57:43 +0000
@@ -219,6 +219,40 @@
 variables @code{shr-color-visible-distance-min} and
 @code{shr-color-visible-luminance-min} to get a better contrast.
 
+@cindex Desktop Support
+@cindex Saving Sessions
+  In addition to maintaining the history at run-time, EWW will also
+save the partial state of its buffers (the URIs and the titles of the
+pages visited) in the desktop file if one is used.  @xref{Saving Emacs
+Sessions, , emacs, The GNU Emacs Manual}
+
+@vindex eww-desktop-remove-duplicates
+  EWW history may sensibly contain multiple entries for the same page
+URI.  At run-time, these entries may still have different associated
+point positions or the actual Web page contents.
+The latter, however, tend to be overly large to preserve in the
+desktop file, so they get omitted, thus rendering the respective
+entries entirely equivalent.  By default, such duplicate entries are
+not saved.  Setting @code{eww-desktop-remove-duplicates} to nil will
+force EWW to save them anyway.
+
+@vindex eww-restore-desktop
+  Restoring EWW buffers' contents may prove to take too long to
+finish.  When the @code{eww-restore-desktop} variable is set to
+@code{nil} (the default), EWW will not try to reload the last visited
+Web page when the buffer is restored from the desktop file, thus
+allowing for faster Emacs start-up times.  When set to @code{t},
+restoring the buffers will also initiate the reloading of such pages.
+
+@vindex eww-restore-reload-prompt
+  The EWW buffer restored from the desktop file but not yet reloaded
+will contain a prompt, as specified by the
+@code{eww-restore-reload-prompt} variable.  The value of this variable
+will be passed through @code{substitute-command-keys} upon each use,
+thus allowing for the use of the usual substitutions, such as
+@code{\[eww-reload]} for the current key binding of the
+@code{eww-reload} command.
+
 @node History and Acknowledgments
 @appendix History and Acknowledgments
 
--- a/etc/NEWS	2014-11-23 10:57:19 +0000
+++ b/etc/NEWS	2014-11-22 19:35:16 +0000
@@ -177,6 +177,9 @@
 *** You can now use several eww buffers in parallel by renaming eww
 buffers you want to keep separate.
 
+*** Partial state of the eww buffers (the URIs and the titles of the
+pages visited) is now preserved in the desktop file.
+
 *** `eww-after-render-hook' is now called after eww has rendered
 the data in the buffer.
 

bug#18010: eww: desktop support

[Lars Magne Ingebrigtsen]

Ivan Shmakov <ivan@siamics.net> writes:

> 	I was unsure where to put the details of the EWW desktop support
> 	in eww.texi, so I’ve added them to the end of the “Advanced”
> 	section, as they’re somewhat obscure and unlikely to require
> 	much of the user’s attention.
>
> 	What may require such attention is, however, the possible use of
> 	(setq desktop-save-buffer nil) to disable this new feature.

Thanks; applied.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#19226: eww.el desktop support fixes: autoload eww-mode, use inhibit-read-only

[Ivan Shmakov]

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

Package:  emacs
Tags:     patch

	Please consider the patch MIMEd.

	* eww.el (eww-mode): Autoload, as expected by desktop.el.
	(eww-restore-desktop): Use inhibit-read-only when inserting
	eww-restore-reload-prompt.

-- 
FSF associate member #7257  np. Your Leaving — Jami Sieber   … B6A0 230E 334A

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/diff, Size: 651 bytes --]

--- a/lisp/net/eww.el
+++ b/lisp/net/eww.el
@@ -662,6 +662,8 @@
     map)
   "Tool bar for `eww-mode'.")
 
+;; autoload cookie needed by desktop.el
+;;;###autoload
 (define-derived-mode eww-mode nil "eww"
   "Mode for browsing the web.
 
@@ -1710,8 +1712,9 @@ defun eww-restore-desktop (file-name buffer-name misc-data)
 	(case eww-restore-desktop
 	  ((t auto) (eww (plist-get eww-data :url)))
 	  ((zerop (buffer-size))
-	   (insert (substitute-command-keys
-		    eww-restore-reload-prompt))))))
+	   (let ((inhibit-read-only t))
+	     (insert (substitute-command-keys
+		      eww-restore-reload-prompt)))))))
     ;; .
     (current-buffer)))
 

bug#19226: eww.el desktop support fixes: autoload eww-mode, use inhibit-read-only

[Glenn Morris]

Ivan Shmakov wrote:

> 	* eww.el (eww-mode): Autoload, as expected by desktop.el.

Then IMO desktop.el should autoload it, rather than adding a general
autoload cookie. (Yes, this may be inconsistent with some existing cases.)

bug#19226: eww.el desktop support fixes: autoload eww-mode, use inhibit-read-only

[Ivan Shmakov]

>>>>> Glenn Morris <rgm@gnu.org> writes:
>>>>> Ivan Shmakov wrote:

 >> * eww.el (eww-mode): Autoload, as expected by desktop.el.

 > Then IMO desktop.el should autoload it, rather than adding a general
 > autoload cookie.

	That’s the point: as currently implemented, desktop.el uses a
	general autoload cookie to map major modes (as preserved in the
	desktop file) to Emacs features to load before handling any
	given buffer.  Consider, for instance:

   514	Modules that define a major mode that needs a special handler should contain
   515	code like
   516	
   517	   (defun foo-restore-desktop-buffer
   518	   ...
   519	   (add-to-list 'desktop-buffer-mode-handlers
   520	                '(foo-mode . foo-restore-desktop-buffer))
   521	
   522	Furthermore the major mode function must be autoloaded.")
…
  1319	(defun desktop-load-file (function)
  1320	  "Load the file where auto loaded FUNCTION is defined."
  1321	  (when (fboundp function)
  1322	    (autoload-do-load (symbol-function function) function)))
…

	(See also the uses of desktop-load-file in desktop.el.)

 > (Yes, this may be inconsistent with some existing cases.)

	As for the existing cases, the change suggested exactly matches
	info.el:

  4222	;; Autoload cookie needed by desktop.el
  4223	;;;###autoload
  4224	(define-derived-mode Info-mode nil "Info" ;FIXME: Derive from special-mode?
…
  4312	  (setq-local bookmark-make-record-function #'Info-bookmark-make-record))

-- 
FSF associate member #7257  http://boycottsystemd.org/  … 3013 B6A0 230E 334A

bug#19226: eww.el desktop support fixes: autoload eww-mode, use inhibit-read-only

[Glenn Morris]

Come to think of it, desktop mode could solve this itself:

When restoring foo-mode, if foo-mode is not bound, try requiring foo.
That would solve this case and many others.

For a complete solution, when desktop saves a major/minor mode to
restore, it could also save the name of the library that defines the
mode. When restoring, first of all require that library.

bug#19226: eww.el desktop support fixes: autoload eww-mode, use inhibit-read-only

[Ivan Shmakov]

>>>>> Glenn Morris <rgm@gnu.org> writes:

 > Come to think of it, desktop mode could solve this itself:

 > When restoring foo-mode, if foo-mode is not bound, try requiring foo.
 > That would solve this case and many others.

	Yet will leave some unsolved.  Such as, for instance, my own
	zealous-save-mode, defined in 'zealsave.

 > For a complete solution, when desktop saves a major/minor mode to
 > restore, it could also save the name of the library that defines the
 > mode.

	How would it obtain such information?  Especially if the user
	has modified the -mode function code and used eval-defun to
	redefine it.  (That’s also what I do at times.)

 > When restoring, first of all require that library.

	And fail miserably should the library be renamed.

	Moreover, “general purpose” modes are ought to be autoloaded
	anyway.  I see no reason to second-guess this case at the least.

-- 
FSF associate member #7257  http://boycottsystemd.org/  … 3013 B6A0 230E 334A

bug#19226: eww.el desktop support fixes: autoload eww-mode, use inhibit-read-only

[Stefan Monnier]

>> Come to think of it, desktop mode could solve this itself:

It could, but I think this would be trying to be too clever.

> 	Moreover, “general purpose” modes are ought to be autoloaded
> 	anyway.  I see no reason to second-guess this case at the least.

I tend to agree.  If the major-mode is not autoloaded (as in the eww
case), it's quite likely to be a special major mode that is better
entered via some other function (e.g. store the URL and recreate the
buffer via `eww' which will take care calling eww-mode (and loading any
file that this may require)).


        Stefan

bug#19226: eww.el desktop support fixes: autoload eww-mode, use inhibit-read-only

[Ivan Shmakov]

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

[…]

 >> Moreover, “general purpose” modes are ought to be autoloaded anyway.
 >> I see no reason to second-guess this case at the least.

 > I tend to agree.  If the major-mode is not autoloaded (as in the eww
 > case), it's quite likely to be a special major mode that is better
 > entered via some other function

	The problem here is that desktop.el should explicitly be pointed
	to the function to do all the special handling for the mode,
	which is (conventionally) done like:

(add-to-list 'desktop-buffer-mode-handlers
             '(eww-mode . eww-restore-desktop))

	The problem is: to get /that/ evaluated, desktop.el needs to
	first load eww.el, leading to a chicken and egg problem.

	The current implementation suggests it to be solved as follows:

	• the major mode is marked as autoloaded;

	• when desktop-read encounters a major mode which is not
	  currently loaded, it loads the respective library (using
	  desktop-load-file, which in turn calls autoload-do-load);

	• that library modifies desktop-buffer-mode-handlers as
	  appropriate (see above.)

	The following “special” modes explicitly document the use of
	autoloads to satisfy this desktop.el requirement:

$ grep -rF --include=\*.el -- needed\ by\ desktop.el lisp/ 
lisp/dired.el:;; Autoload cookie needed by desktop.el
lisp/info.el:;; Autoload cookie needed by desktop.el
lisp/mh-e/mh-folder.el:;; Autoload cookie needed by desktop.el
$ 

	The others (doc-view-mode?) do that quietly.  And I guess there
	may be a few (vc-dir-mode?) that fail there.

 > (e. g. store the URL and recreate the buffer via `eww' which will
 > take care calling eww-mode (and loading any file that this may
 > require)).

	… Yet that’s by no means necessary, – it’s perfectly possible to
	invoke M-x eww-mode from just any (empty) buffer, and then use
	‘G’ to point EWW to the Web page of interest.

	Hence, the trick mentioned in etc/NEWS isn’t necessary, either.

*** You can now use several eww buffers in parallel by renaming eww
buffers you want to keep separate.

-- 
FSF associate member #7257  http://boycottsystemd.org/  … 3013 B6A0 230E 334A

bug#19226: eww.el desktop support fixes: autoload eww-mode, use inhibit-read-only

[Stefan Monnier]

> 	The problem here is that desktop.el should explicitly be pointed
> 	to the function to do all the special handling for the mode,
> 	which is (conventionally) done like:

> (add-to-list 'desktop-buffer-mode-handlers
>              '(eww-mode . eww-restore-desktop))

> 	The problem is: to get /that/ evaluated, desktop.el needs to
> 	first load eww.el, leading to a chicken and egg problem.

I see.  Maybe the problem then is that desktop.el should be changed so
that it records `eww-restore-desktop' as the handler function for this
buffer in the desktop file.  I.e. those desktop-buffer-mode-handlers
should be looked up while *saving* the desktop file rather than while
reading them.


        Stefan

bug#19226: eww.el desktop support fixes: autoload eww-mode, use inhibit-read-only

[Ivan Shmakov]

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

 >> The problem here is that desktop.el should explicitly be pointed to
 >> the function to do all the special handling for the mode, which is
 >> (conventionally) done like:

 >> (add-to-list 'desktop-buffer-mode-handlers
 >>              '(eww-mode . eww-restore-desktop))

 >> The problem is: to get /that/ evaluated, desktop.el needs to first
 >> load eww.el, leading to a chicken and egg problem.

 > I see.  Maybe the problem then is that desktop.el should be changed
 > so that it records `eww-restore-desktop' as the handler function for
 > this buffer in the desktop file.  I. e. those
 > desktop-buffer-mode-handlers should be looked up while *saving* the
 > desktop file rather than while reading them.

	Yes.  Yet I fail to see how that would be an improvement?

	First of all, we already can trigger .emacs.desktop
	incompatibility when the format of the value stored in there
	changes.  Now, we also make that fail when it’s the /name/ of
	the handler function that changes.  (I presume that the change
	in the mode name is much less likely on one side, and much more
	likely to get a defalias on the other.)

	Moreover, instead of (or, well, in addition to) an autoload for
	the mode function, we now need an autoload for the handler.

-- 
FSF associate member #7257  http://boycottsystemd.org/  … 3013 B6A0 230E 334A

bug#19226: eww.el desktop support fixes: autoload eww-mode, use inhibit-read-only

[Stefan Monnier]

> 	Yes.  Yet I fail to see how that would be an improvement?

Maybe it's not worth the trouble, indeed.

> 	changes.  Now, we also make that fail when it’s the /name/ of
> 	the handler function that changes.  (I presume that the change
> 	in the mode name is much less likely on one side, and much more
> 	likely to get a defalias on the other.)

The issue at hand was presumably that the buffer's mode (i.e. eww-mode)
is not meant as a "normal entry point".  So it's more normal to have to
keep the name of the handler stable, since that is an explicit entry point.

> 	Moreover, instead of (or, well, in addition to) an autoload for
> 	the mode function, we now need an autoload for the handler.

But this handler *is* a normal entry point.


        Stefan

bug#19226: eww.el desktop support fixes: autoload eww-mode, use inhibit-read-only

[Ivan Shmakov]

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

	The discussion, as it seems, have lead us nowhere.

	Would there be any objections against me pushing the (updated)
	fix sometime within the next two days, /as well as/ a couple of
	unrelated minor changes (both MIMEd)?  (Somehow, I believe that
	the discussion over the possible desktop.el vs. special modes
	changes can safely be postponed until something specific is
	suggested.)

	TIA.

Fix eww.el desktop support.

* lisp/net/eww.el (eww-mode): Add autoload cookie.
(eww-restore-desktop): Use inhibit-read-only.

Fixes: debbugs:19226

Minor eww.el fixes.

* lisp/net/eww.el (eww-suggest-uris): Add autoload cookie, so that
add-hook works correctly even if the file is not yet loaded.
(eww-list-histories): Do not pad the rightmost (URIs) column.

-- 
FSF associate member #7257  http://boycottsystemd.org/  … 3013 B6A0 230E 334A

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/diff, Size: 702 bytes --]

--- a/lisp/net/eww.el
+++ b/lisp/net/eww.el
@@ -686,6 +687,8 @@
     map)
   "Tool bar for `eww-mode'.")
 
+;; Autoload cookie needed by desktop.el.
+;;;###autoload
 (define-derived-mode eww-mode special-mode "eww"
   "Mode for browsing the web."
   (setq-local eww-data (list :title ""))
@@ -1877,8 +1880,9 @@ defun eww-restore-desktop (file-name buffer-name misc-data)
 	(case eww-restore-desktop
 	  ((t auto) (eww (plist-get eww-data :url)))
 	  ((zerop (buffer-size))
-	   (insert (substitute-command-keys
-		    eww-restore-reload-prompt))))))
+	   (let ((inhibit-read-only t))
+	     (insert (substitute-command-keys
+		      eww-restore-reload-prompt)))))))
     ;; .
     (current-buffer)))
 

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: Type: text/diff, Size: 665 bytes --]

--- a/lisp/net/eww.el
+++ b/lisp/net/eww.el
@@ -60,6 +60,7 @@
   :group 'eww
   :type 'string)
 
+;;;###autoload
 (defcustom eww-suggest-uris
   '(eww-links-at-point
     url-get-url-at-point
@@ -1634,7 +1637,7 @@ defun eww-list-histories ()
 	(setq start (point))
 	(setq domain-length (max domain-length (length (plist-get history :url))))
 	(setq title-length (max title-length (length (plist-get history :title)))))
-      (setq format (format "%%-%ds %%-%ds" title-length domain-length)
+      (setq format (format "%%-%ds %%s" title-length)
 	    header-line-format
 	    (concat " " (format format "Title" "URL")))
       (dolist (history eww-history-trans)

bug#19226: eww.el desktop support fixes: autoload eww-mode, use inhibit-read-only

[Ivan Shmakov]

>>>>> Ivan Shmakov <ivan@siamics.net> writes:

[…]

 > Would there be any objections against me pushing the (updated) fix
 > sometime within the next two days, /as well as/ a couple of unrelated
 > minor changes (both MIMEd)?

	Pushed, as there were no objections; closing.

[…]

 > (eww-list-histories): Do not pad the rightmost (URIs) column.

	… Except for this padding change, as the same change should
	probably be applied to eww-list-buffers at the same time.

[…]

commit 8b36bfafeecf5f0578c64129be1d2017483b00f7
CommitDate: 2015-02-16 19:01:50 +0000

    Add autoload cookie for the eww-suggest-uris variable.
    
    * lisp/net/eww.el (eww-suggest-uris): Add autoload cookie, so that
    add-hook works correctly even if the file is not yet loaded.

commit 2ea5364ca8d1a8dc3f8ac4c9a5ba5c7f03666258
CommitDate: 2015-02-16 18:55:02 +0000

    Fix eww.el desktop support.
    
    * lisp/net/eww.el (eww-mode): Add autoload cookie.
    (eww-restore-desktop): Use inhibit-read-only.
    
    Fixes: debbugs:19226

-- 
FSF associate member #7257  http://boycottsystemd.org/  … 3013 B6A0 230E 334A