bug#20032: eww: access bookmarks right from the URI prompt

bug#20032: eww: access bookmarks right from the URI prompt

[Ivan Shmakov]

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

Package:  emacs
Severity: wishlist
Tags: patch

	Please consider the tentative patch MIMEd.

	* lisp/net/eww.el (eww-suggest-uris): Add eww-suggest-bookmarks
	to the default value and :options.  (Bug#???)
	(eww-suggest-bookmarks, eww-remove-annotation): New functions.
	(eww-suggested-uris, eww): Use eww-remove-annotation.

	Somehow, I believe that eww-remove-annotation may be generalized
	into something worthy of subr.el.

	On a related note, can we please make eww-bookmarks a defcustom
	and thus replace eww-write-bookmarks, eww-read-bookmarks with
	customize-save-variable and the regular custom-set-variables
	mechanism, respectively?

-- 
FSF associate member #7257  np. Begin Again — Jami Sieber 3013 B6A0 230E 334A

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

--- a/lisp/net/eww.el
+++ b/lisp/net/eww.el
@@ -64,7 +64,8 @@ defcustom eww-download-directory "~/Downloads/"
 (defcustom eww-suggest-uris
   '(eww-links-at-point
     url-get-url-at-point
-    eww-current-url)
+    eww-current-url
+    eww-suggest-bookmarks)
   "List of functions called to form the list of default URIs for `eww'.
 Each of the elements is a function returning either a string or a list
 of strings.  The results will be joined into a single list with
@@ -74,7 +75,8 @@ defcustom eww-suggest-uris
   :type 'hook
   :options '(eww-links-at-point
 	     url-get-url-at-point
-	     eww-current-url))
+	     eww-current-url
+	     eww-suggest-bookmarks))
 
 (defcustom eww-bookmarks-directory user-emacs-directory
   "Directory where bookmark files will be stored."
@@ -234,11 +243,30 @@ defun eww-suggested-uris nil
     (dolist (fun eww-suggest-uris)
       (let ((ret (funcall fun)))
 	(dolist (uri (if (stringp ret) (list ret) ret))
-	  (when (and uri (not (intern-soft uri obseen)))
-	    (intern uri obseen)
-	    (push   uri uris)))))
+	  (let ((key (and uri (eww-remove-annotation uri))))
+	    (when (and key (not (intern-soft key obseen)))
+	      (intern key obseen)
+	      (push   uri uris))))))
     (nreverse uris)))
 
+(defun eww-remove-annotation (string &optional start)
+  (unless start
+    (setq start 0))
+  (let ((init (get-text-property start 'annotation string))
+	(chan (next-single-property-change start 'annotation string)))
+    (if (not chan)
+	(if init nil		  ; The whole string is an annotation.
+	  string)
+      (let ((acc  nil)
+	    (from (if init chan start)))
+	(while from
+	  (let ((to (next-single-property-change from 'annotation string)))
+	    (setq acc  (concat acc (substring string from to))
+		  from (and to
+			    (next-single-property-change
+			     to 'annotation string)))))
+	acc))))
+
 ;;;###autoload
 (defun eww (url)
   "Fetch URL and render the page.
@@ -249,7 +277,8 @@ defun eww (url)
 	  (prompt (concat "Enter URL or keywords"
 			  (if uris (format " (default %s)" (car uris)) "")
 			  ": ")))
-     (list (read-string prompt nil nil uris))))
+     (when-let ((uri (read-string prompt nil nil uris)))
+       (list (eww-remove-annotation uri)))))
   (setq url (string-trim url))
   (cond ((string-match-p "\\`file:/" url))
 	;; Don't mangle file: URLs at all.
@@ -549,6 +564,22 @@ defun eww-links-at-point ()
 	(list (get-text-property (point) 'shr-url)
 	      (get-text-property (point) 'image-url))))
 
+(defun eww-suggest-bookmarks ()
+  "Return list of bookmarked URIs, if any.
+The URIs returned may contain arbitrary annotations.  Apply
+`eww-remove-annotation' on elements of the list returned to obtain the
+URIs proper."
+  (mapcar (lambda (elt)
+	    (let ((uri (plist-get elt :url))
+		  (title (plist-get elt :title)))
+	      (when uri
+		(concat uri
+			(propertize " "
+				    'display (concat " (" title ")")
+				    'annotation t
+				    'face 'minibuffer-prompt)))))
+	  eww-bookmarks))
+
 (defun eww-view-source ()
   "View the HTML source code of the current page."
   (interactive)

bug#20032: eww: access bookmarks right from the URI prompt

[Ivan Shmakov]

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

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

[…]

	That variant didn’t work quite well in practice.  The primary
	issue was that using 'display meant it wasn’t possible to search
	/annotations/ (using C-s, C-r, etc.) – only the URIs proper,
	which isn’t as nice as it could be.

	One another is that once we replace the use of 'display with
	appending the actual annotation to the value, with a non-nil
	'field property (for navigation purposes), move-end-of-line
	starts to misbehave in certain cases, apparently due to the fact
	that ‘line-move’ returns t after moving point to another /field/
	(as opposed to /line/.)  I haven’t identified the cause as of
	yet, but placing the title /before/ URI seems to make this
	problem harder to actually stumble upon.  Now given that there
	may be different preferences anyway, I’ve decided to introduce a
	new customizable variable for this one.

	(I’ve cleaned the new code up somewhat as well.)

	Please consider the revised patch MIMEd.

	* lisp/net/eww.el (eww-suggest-uris): Add eww-suggest-bookmarks
	to the default value and :options.  (Bug#20032)
	(eww-suggested-bookmarks-annotation): New customizable variable.
	(eww-suggest-bookmarks, eww-remove-annotation)
	(eww-substring-nil-property): New functions.
	(eww-suggested-uris, eww): Use eww-remove-annotation.

 > Somehow, I believe that eww-remove-annotation may be generalized
 > into something worthy of subr.el.

	(Or subr-x.el; or some other library.)  Generalized in the
	current revision of the patch as eww-substring-nil-property.

-- 
FSF associate member #7257  np. In the Garden — David Modica … B6A0 230E 334A

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

--- a/lisp/net/eww.el
+++ b/lisp/net/eww.el
@@ -64,7 +64,8 @@
 (defcustom eww-suggest-uris
   '(eww-links-at-point
     url-get-url-at-point
-    eww-current-url)
+    eww-current-url
+    eww-suggest-bookmarks)
   "List of functions called to form the list of default URIs for `eww'.
 Each of the elements is a function returning either a string or a list
 of strings.  The results will be joined into a single list with
@@ -74,7 +75,21 @@ defcustom eww-suggest-uris
   :type 'hook
   :options '(eww-links-at-point
 	     url-get-url-at-point
-	     eww-current-url))
+	     eww-current-url
+	     eww-suggest-bookmarks))
+
+(defcustom eww-suggested-bookmarks-annotation 'before
+  "Where to place annotation in the list of bookmarks at the URI prompt.
+This variable controls where the bookmark annotation (title) is placed.
+
+If 'before, annotation goes before URI.
+If 'after, annotation goes after URI.
+If nil, no annotation is provided."
+  :version "25.1"
+  :group 'eww
+  :type '(choice (const :tag "Annotation before URI" before)
+		 (const :tag "Annotation after URI" after)
+		 (const :tag "No annotation" nil)))
 
 (defcustom eww-bookmarks-directory user-emacs-directory
   "Directory where bookmark files will be stored."
@@ -234,9 +256,10 @@ defun eww-suggested-uris nil
     (dolist (fun eww-suggest-uris)
       (let ((ret (funcall fun)))
 	(dolist (uri (if (stringp ret) (list ret) ret))
-	  (when (and uri (not (intern-soft uri obseen)))
-	    (intern uri obseen)
-	    (push   uri uris)))))
+	  (let ((key (and uri (eww-remove-annotation uri))))
+	    (when (and key (not (intern-soft key obseen)))
+	      (intern key obseen)
+	      (push   uri uris))))))
     (nreverse uris)))
 
 ;;;###autoload
@@ -249,7 +272,9 @@ defun eww (url)
 	  (prompt (concat "Enter URL or keywords"
 			  (if uris (format " (default %s)" (car uris)) "")
 			  ": ")))
-     (list (read-string prompt nil nil uris))))
+     (let ((minibuffer-allow-text-properties t))
+       (list (eww-remove-annotation
+	      (read-string prompt nil nil uris))))))
   (setq url (string-trim url))
   (cond ((string-match-p "\\`file:/" url))
 	;; Don't mangle file: URLs at all.
@@ -549,6 +560,67 @@
 	(list (get-text-property (point) 'shr-url)
 	      (get-text-property (point) 'image-url))))
 
+(defun eww-suggested-bookmarks ()
+  "Return list of bookmarked URIs, if any.
+The URIs returned may contain arbitrary annotations.  Apply
+`eww-remove-annotation' on elements of the list returned to obtain the
+URIs proper."
+  (let* ((afterp (eq 'after eww-suggested-bookmarks-annotation))
+	 (prop (and eww-suggested-bookmarks-annotation
+		    (nconc (if afterp nil
+			     (list 'front-sticky t
+				   'rear-nonsticky t))
+			   '(field eww-title
+			     annotation t
+			     ; read-only t
+			     face minibuffer-prompt))))
+	 (fmt  (and prop
+		    (apply #'propertize
+			   (if afterp " (%s)" "(%s) ")
+			   prop))))
+    (mapcar (lambda (elt)
+	      (let ((uri (plist-get elt :url))
+		    (title (plist-get elt :title)))
+		(when uri
+		  (cond ((not fmt) uri)
+			(afterp (concat uri (format fmt title)))
+			(t      (concat (format fmt title) uri))))))
+	  eww-bookmarks)))
+
+(defun eww-remove-annotation (string &optional start limit)
+  (eww-substring-nil-property 'annotation string start limit))
+
+(defun eww-substring-nil-property (property &optional object start limit)
+  "Return the contents of OBJECT where PROPERTY is not set or nil.
+Return nil if all the contents of OBJECT has PROPERTY set and non-nil.
+
+OBJECT is either a string, buffer, or nil (which means the
+current buffer).
+
+If START is nil, use 0 for a string, or point for a buffer OBJECT.
+If LIMIT is nil, use string length for a string or (point-max) for a
+buffer."
+  (save-current-buffer
+    (cond ((stringp object))
+          (object
+           (set-buffer object)
+           (setq object nil)))
+    ;; From now on, object is either a string or nil.
+    (unless start
+      (setq start (if object 0 (point))))
+    (unless (or object limit)
+      (setq limit (point-max)))
+    (let ((acc  nil) 
+          (from start))
+      (while from
+        (let ((to (next-single-property-change from property object)))
+          (unless (get-text-property from property object)
+            (setq acc (concat acc
+                              (if object
+                                  (substring object from to)
+                                (buffer-substring from (or to limit))))))
+          (setq from to)))
+      acc)))
 (defun eww-view-source ()
   "View the HTML source code of the current page."
   (interactive)

bug#20032: eww: access bookmarks right from the URI prompt

[Lars Ingebrigtsen]

Ivan Shmakov <ivan@siamics.net> writes:

> 	On a related note, can we please make eww-bookmarks a defcustom
> 	and thus replace eww-write-bookmarks, eww-read-bookmarks with
> 	customize-save-variable and the regular custom-set-variables
> 	mechanism, respectively?

No, I think it makes more sense to keep the bookmarks separate, since
there may be many of them.  They would just be noise in .emacs.

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

bug#20032: eww: access bookmarks right from the URI prompt

[Lars Ingebrigtsen]

Ivan Shmakov <ivan@siamics.net> writes:

> 	Please consider the revised patch MIMEd.
>
> 	* lisp/net/eww.el (eww-suggest-uris): Add eww-suggest-bookmarks
> 	to the default value and :options.  (Bug#20032)
> 	(eww-suggested-bookmarks-annotation): New customizable variable.
> 	(eww-suggest-bookmarks, eww-remove-annotation)
> 	(eww-substring-nil-property): New functions.
> 	(eww-suggested-uris, eww): Use eww-remove-annotation.

I get a compilation warning after applying the patch...  (The defvar for
eww-bookmarks should be moved.)

But conceptually I'm not sure this is a good idea.  If you have a
bookmarks file, then if you say `M-x eww', you will apparently default
to the first entry in the bookmarks file?  That doesn't seem very
helpful.  It would be OK to have that as the first entry in the `M-n'
history, but not as the default.  If I read the code correctly.

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

bug#20032: eww: access bookmarks right from the URI prompt

[Ivan Shmakov]

>>>>> Lars Ingebrigtsen <larsi@gnus.org> writes:
>>>>> Ivan Shmakov <ivan@siamics.net> writes:

 >> Please consider the revised patch MIMEd.

 >> * lisp/net/eww.el (eww-suggest-uris): Add eww-suggest-bookmarks to
 >> the default value and :options.  (Bug#20032)
 >> (eww-suggested-bookmarks-annotation): New customizable variable.
 >> (eww-suggest-bookmarks, eww-remove-annotation)
 >> (eww-substring-nil-property): New functions.
 >> (eww-suggested-uris, eww): Use eww-remove-annotation.

 > I get a compilation warning after applying the patch...  (The defvar
 > for eww-bookmarks should be moved.)

	Yes.

 > But conceptually I'm not sure this is a good idea.  If you have a
 > bookmarks file, then if you say `M-x eww', you will apparently
 > default to the first entry in the bookmarks file?

	Indeed; unless the point is on a URI, where you’d get that URI
	as the default.

	Speaking of the bookmarks file, eww-suggested-bookmarks is
	missing a eww-read-bookmarks call in that patch.

 > That doesn't seem very helpful.

	Actually, it is: I’ve set the eww-bookmarks variable so that the
	resources I visit the most often are at the top of the list, and
	now I can access them with just a few M-ns, or, for the topmost,
	just RET.

 > It would be OK to have that as the first entry in the `M-n' history,
 > but not as the default.  If I read the code correctly.

	I guess that could be fixed by using a modified version of
	read-string and flagging the values returned by
	eww-suggest-bookmarks in such a way as to never be considered as
	the default by that function.  I doubt that that’s worth the
	effort, however.  (Unless such a function already exists.)

	What bothers me more is that the sheer corpus of contemporary
	software will generally filter the list of bookmarks per the
	search terms typed in by the user first, and only then present
	the result for their respective equivalents of M-n.  Which gets
	handy if your bookmarks (or history, or pretty much anything of
	that kind) number in the hundreds (or more.)  I know of no
	similar facility in the current Emacs, however.

-- 
FSF associate member #7257  http://am-1.org/~ivan/      … 3013 B6A0 230E 334A

bug#20032: eww: access bookmarks right from the URI prompt

[Ivan Shmakov]

>>>>> Lars Ingebrigtsen <larsi@gnus.org> writes:
>>>>> Ivan Shmakov <ivan@siamics.net> writes:

 >> On a related note, can we please make eww-bookmarks a defcustom and
 >> thus replace eww-write-bookmarks, eww-read-bookmarks with
 >> customize-save-variable and the regular custom-set-variables
 >> mechanism, respectively?

 > No, I think it makes more sense to keep the bookmarks separate, since
 > there may be many of them.  They would just be noise in .emacs.

	The customizations are saved to custom-file, which is not
	necessarily ~/.emacs.

	Alternatively, we may decide to use 'bookmark for the URI
	bookmarks as well.  I have no idea what the benefits (apart from
	getting rid of potential code duplication) it may bring, if any.

	But personally, I’ve ended up just using a plain setq in my
	~/.emacs, with neither Customize nor EWW native bookmark support
	getting in the way.  (One may want to redefine both
	eww-read-bookmarks and eww-write-bookmarks to no-op when going
	this way, BTW.)

-- 
FSF associate member #7257  http://am-1.org/~ivan/      … 3013 B6A0 230E 334A

bug#20032: eww: access bookmarks right from the URI prompt

[Lars Ingebrigtsen]

Ivan Shmakov <ivan@siamics.net> writes:

>> It would be OK to have that as the first entry in the `M-n' history,
>> but not as the default.  If I read the code correctly.
>
> I guess that could be fixed by using a modified version of
> read-string and flagging the values returned by
> eww-suggest-bookmarks in such a way as to never be considered as
> the default by that function.  I doubt that that’s worth the
> effort, however.  (Unless such a function already exists.)

`read-string' with nil initial-input and default-value will have no
defaults.

> What bothers me more is that the sheer corpus of contemporary
> software will generally filter the list of bookmarks per the
> search terms typed in by the user first, and only then present
> the result for their respective equivalents of M-n.  Which gets
> handy if your bookmarks (or history, or pretty much anything of
> that kind) number in the hundreds (or more.)  I know of no
> similar facility in the current Emacs, however.

Yeah, just sticking the bookmarks in the history isn't very useful.

Message uses ecomplete to do "electric" completion.

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

bug#20032: eww: access bookmarks right from the URI prompt

[Ivan Shmakov]

>>>>> Lars Ingebrigtsen <larsi@gnus.org> writes:
>>>>> Ivan Shmakov <ivan@siamics.net> writes:

 >>> It would be OK to have that as the first entry in the `M-n'
 >>> history, but not as the default.  If I read the code correctly.

 >> I guess that could be fixed by using a modified version of
 >> read-string and flagging the values returned by
 >> eww-suggest-bookmarks in such a way as to never be considered as the
 >> default by that function.  I doubt that that’s worth the effort,
 >> however.  (Unless such a function already exists.)

 > `read-string' with nil initial-input and default-value will have no
 > defaults.

	So?  I don’t see how this is supposed to help in this case.

 >> What bothers me more is that the sheer corpus of contemporary
 >> software will generally filter the list of bookmarks per the search
 >> terms typed in by the user first, and only then present the result
 >> for their respective equivalents of M-n.  Which gets handy if your
 >> bookmarks (or history, or pretty much anything of that kind) number
 >> in the hundreds (or more.)  I know of no similar facility in the
 >> current Emacs, however.

 > Yeah, just sticking the bookmarks in the history isn't very useful.

	Having used this feature ever since I’ve got it implemented,
	I tend to disagree.

 > Message uses ecomplete to do "electric" completion.

	AIUI, it currently allows for only a single completion table,
	and I doubt that it will make much sense to have that table
	shared among Message and EWW (and perhaps other possible users.)

-- 
FSF associate member #7257  http://am-1.org/~ivan/      … 3013 B6A0 230E 334A

bug#20032: eww: access bookmarks right from the URI prompt

[Lars Ingebrigtsen]

Ivan Shmakov <ivan@siamics.net> writes:

> AIUI, it currently allows for only a single completion table,
> and I doubt that it will make much sense to have that table
> shared among Message and EWW (and perhaps other possible users.)

That can be fixed rather trivially.

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

bug#20032: eww: access bookmarks right from the URI prompt

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Ivan Shmakov <ivan@siamics.net> writes:
>
>> AIUI, it currently allows for only a single completion table,
>> and I doubt that it will make much sense to have that table
>> shared among Message and EWW (and perhaps other possible users.)
>
> That can be fixed rather trivially.

If you want to implement bookmark completion with ecomplete, I think
that would be a nice idea.  Using the "normal" completion just doesn't
fit how I think users want to be presented with partial match completion
of these long items.

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

bug#20032: eww: access bookmarks right from the URI prompt

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Lars Ingebrigtsen <larsi@gnus.org> writes:
>
>> Ivan Shmakov <ivan@siamics.net> writes:
>>
>>> AIUI, it currently allows for only a single completion table,
>>> and I doubt that it will make much sense to have that table
>>> shared among Message and EWW (and perhaps other possible users.)
>>
>> That can be fixed rather trivially.
>
> If you want to implement bookmark completion with ecomplete, I think
> that would be a nice idea.  Using the "normal" completion just doesn't
> fit how I think users want to be presented with partial match completion
> of these long items.

I still agree with myself that completing normally over the bookmarks
would be confusing, so I'm closing this bug report.

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