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

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 19226 in the body.
You can then email your comments to 19226 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Ivan Shmakov <ivan <at> siamics.net>
To: submit <at> debbugs.gnu.org
Subject: eww.el desktop support fixes: autoload eww-mode,
 use inhibit-read-only 
Date: Sun, 30 Nov 2014 11:04:04 +0000

[Message part 1 (text/plain, inline)]

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

[Message part 2 (text/diff, inline)]

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

Message #8 received at 19226 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Ivan Shmakov <ivan <at> siamics.net>
Cc: 19226 <at> debbugs.gnu.org
Subject: Re: bug#19226: eww.el desktop support fixes: autoload eww-mode,
 use inhibit-read-only
Date: Sun, 30 Nov 2014 14:57:11 -0500

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

Message #11 received at 19226 <at> debbugs.gnu.org (full text, mbox):

From: Ivan Shmakov <ivan <at> siamics.net>
To: 19226 <at> debbugs.gnu.org
Subject: Re: bug#19226: eww.el desktop support fixes: autoload eww-mode,
 use inhibit-read-only 
Date: Sun, 30 Nov 2014 20:15:49 +0000

>>>>> Glenn Morris <rgm <at> 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

Message #14 received at 19226 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: 19226 <at> debbugs.gnu.org
Subject: Re: bug#19226: eww.el desktop support fixes: autoload eww-mode,
 use inhibit-read-only
Date: Sun, 30 Nov 2014 21:22:13 -0500

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.

Message #17 received at 19226 <at> debbugs.gnu.org (full text, mbox):

From: Ivan Shmakov <ivan <at> siamics.net>
To: 19226 <at> debbugs.gnu.org
Subject: Re: bug#19226: eww.el desktop support fixes: autoload eww-mode,
 use inhibit-read-only 
Date: Mon, 01 Dec 2014 05:59:38 +0000

>>>>> Glenn Morris <rgm <at> 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

Message #20 received at 19226 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: 19226 <at> debbugs.gnu.org
Subject: Re: bug#19226: eww.el desktop support fixes: autoload eww-mode,
 use inhibit-read-only
Date: Mon, 01 Dec 2014 08:52:01 -0500

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

Message #23 received at 19226 <at> debbugs.gnu.org (full text, mbox):

From: Ivan Shmakov <ivan <at> siamics.net>
To: 19226 <at> debbugs.gnu.org
Subject: Re: bug#19226: eww.el desktop support fixes: autoload eww-mode,
 use inhibit-read-only 
Date: Sun, 07 Dec 2014 18:56:52 +0000

>>>>> Stefan Monnier <monnier <at> 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

Message #26 received at 19226 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: 19226 <at> debbugs.gnu.org
Subject: Re: bug#19226: eww.el desktop support fixes: autoload eww-mode,
 use inhibit-read-only
Date: Sun, 07 Dec 2014 21:54:36 -0500

> 	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

Message #29 received at 19226 <at> debbugs.gnu.org (full text, mbox):

From: Ivan Shmakov <ivan <at> siamics.net>
To: 19226 <at> debbugs.gnu.org
Subject: Re: bug#19226: eww.el desktop support fixes: autoload eww-mode,
 use inhibit-read-only 
Date: Tue, 09 Dec 2014 19:45:47 +0000

>>>>> Stefan Monnier <monnier <at> 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

Message #32 received at 19226 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: 19226 <at> debbugs.gnu.org
Subject: Re: bug#19226: eww.el desktop support fixes: autoload eww-mode,
 use inhibit-read-only
Date: Tue, 09 Dec 2014 19:35:50 -0500

> 	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

Message #35 received at 19226 <at> debbugs.gnu.org (full text, mbox):

From: Ivan Shmakov <ivan <at> siamics.net>
To: 19226 <at> debbugs.gnu.org
Subject: Re: bug#19226: eww.el desktop support fixes: autoload eww-mode,
 use inhibit-read-only 
Date: Sat, 14 Feb 2015 20:50:41 +0000

[Message part 1 (text/plain, inline)]

	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

[Message part 2 (text/diff, inline)]

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

[Message part 3 (text/diff, inline)]

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

Message #40 received at 19226-done <at> debbugs.gnu.org (full text, mbox):

From: Ivan Shmakov <ivan <at> siamics.net>
To: 19226-done <at> debbugs.gnu.org
Subject: Re: bug#19226: eww.el desktop support fixes: autoload eww-mode,
 use inhibit-read-only 
Date: Mon, 16 Feb 2015 19:12:09 +0000

>>>>> Ivan Shmakov <ivan <at> 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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.