[PATCH] Use vtable for eww-bookmarks

[PATCH] Use vtable for eww-bookmarks

[Sebastián Monía]

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

Hi all,

The attached patch is a first attempt at bringing eww-bookmarks in line
with the previous switch of eww-buffer-list to vtable.

There are features of eww-bookmarks I had no idea about, like the
ability to re-arrange them by killing and yaking. Or navigating to the
next bookmark from an eww buffer or invoking the command via M-x.

Thank you,
Seb


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vtable-eww-bookmarks --]
[-- Type: text/x-patch, Size: 9296 bytes --]

From df509a792a58c5eedd1f516c406378315c8d95ca Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sebasti=C3=A1n=20Mon=C3=ADa?=
 <sebastian.monia@sebasmonia.com>
Date: Tue, 5 Nov 2024 17:48:56 -0500
Subject: [PATCH] Use vtable in eww-list-bookmarks

* lisp/net/eww.el (eww-list-bookmarks): Move logic to...:
(eww--bookmark-prepare, eww--bookmark-format-data )
... these,
and use 'vtable'.
(eww-bookmark-kill, eww-bookmark-yank, eww-bookmark-browse,
 eww-next-bookmark, eww-previous-bookmark,
 eww-buffers-mode-map): Use 'vtable-current-object'.
(eww-bookmark-undo-sort): New command to revert bookmark
 table sort
---
 lisp/net/eww.el | 174 +++++++++++++++++++++++++++---------------------
 1 file changed, 98 insertions(+), 76 deletions(-)

diff --git a/lisp/net/eww.el b/lisp/net/eww.el
index 2d351dff88f..10a7663efc5 100644
--- a/lisp/net/eww.el
+++ b/lisp/net/eww.el
@@ -2373,115 +2373,124 @@ eww-read-bookmarks
       (user-error "No bookmarks are defined"))))
 
 ;;;###autoload
-(defun eww-list-bookmarks ()
-  "Display the bookmarks."
+(defun eww-list-bookmarks (&optional build-only)
+  "Display the eww bookmarks.
+Optional argument BUILD-ONLY, when non-nil, means to build the buffer
+without popping it."
   (interactive)
   (eww-read-bookmarks t)
-  (pop-to-buffer "*eww bookmarks*")
-  (eww-bookmark-prepare))
-
-(defun eww-bookmark-prepare ()
-  (set-buffer (get-buffer-create "*eww bookmarks*"))
-  (eww-bookmark-mode)
-  (let* ((width (/ (window-width) 2))
-	 (format (format "%%-%ds %%s" width))
-	 (inhibit-read-only t)
-	 start title)
+  (with-current-buffer (get-buffer-create "*eww bookmarks*")
+    (eww-bookmark-mode)
+    (eww--bookmark-prepare))
+  (unless build-only
+    (pop-to-buffer "*eww bookmarks*")))
+
+(defun eww--bookmark-prepare ()
+  "Display a table with the list of eww bookmarks.
+Will remove all buffer contents first."
+  (let ((inhibit-read-only t))
     (erase-buffer)
-    (setq header-line-format (concat " " (format format "Title" "URL")))
-    (dolist (bookmark eww-bookmarks)
-      (setq start (point)
-	    title (plist-get bookmark :title))
-      (when (> (length title) width)
-	(setq title (truncate-string-to-width title width)))
-      (insert (format format title (plist-get bookmark :url)) "\n")
-      (put-text-property start (1+ start) 'eww-bookmark bookmark))
-    (goto-char (point-min))))
+    (make-vtable
+     :columns '((:name "Title" :min-width "25%" :max-width "50%")
+                (:name "URL"))
+     :objects-function #'eww--bookmark-format-data
+     ;; use fixed-font face
+     :face 'default)))
+
+(defun eww--bookmark-format-data ()
+  "Format `eww-bookmarks' for use in a vtable.
+The data is returned as list of (title url bookmark) triplets, for use
+in of `eww-bookmark-mode'."
+  (mapcar (lambda (bm)
+            (list (plist-get bm :title)
+                  (plist-get bm :url)
+                  bm))
+          eww-bookmarks))
 
 (defvar eww-bookmark-kill-ring nil)
 
+(defun eww--bookmark-abort-if-sorted ()
+  "Signal a user error if the bookmark vtable at point has been sorted."
+  (when (and (vtable-current-table)
+             (vtable-sort-by (vtable-current-table)))
+    (user-error "Can't kill/yank bookmarks after the table has been sorted")))
+
 (defun eww-bookmark-kill ()
   "Kill the current bookmark."
   (interactive nil eww-bookmark-mode)
-  (let* ((start (line-beginning-position))
-	 (bookmark (get-text-property start 'eww-bookmark))
-	 (inhibit-read-only t))
-    (unless bookmark
+  (eww--bookmark-abort-if-sorted)
+  (let ((bookmark-at-point (nth 2 (vtable-current-object)))
+	(position (point)))
+    (unless bookmark-at-point
       (user-error "No bookmark on the current line"))
     (forward-line 1)
-    (push (buffer-substring start (point)) eww-bookmark-kill-ring)
-    (delete-region start (point))
-    (setq eww-bookmarks (delq bookmark eww-bookmarks))
-    (eww-write-bookmarks)))
+    (push bookmark-at-point eww-bookmark-kill-ring)
+    (setq eww-bookmarks (delq bookmark-at-point eww-bookmarks))
+    (eww-write-bookmarks)
+    (vtable-revert-command)
+    (goto-char position)))
 
 (defun eww-bookmark-yank ()
   "Yank a previously killed bookmark to the current line."
   (interactive nil eww-bookmark-mode)
+  (eww--bookmark-abort-if-sorted)
   (unless eww-bookmark-kill-ring
     (user-error "No previously killed bookmark"))
-  (beginning-of-line)
-  (let ((inhibit-read-only t)
-	(start (point))
-	bookmark)
-    (insert (pop eww-bookmark-kill-ring))
-    (setq bookmark (get-text-property start 'eww-bookmark))
-    (if (= start (point-min))
-	(push bookmark eww-bookmarks)
-      (let ((line (count-lines start (point))))
-	(setcdr (nthcdr (1- line) eww-bookmarks)
-		(cons bookmark (nthcdr line eww-bookmarks)))))
-    (eww-write-bookmarks)))
+  (let* ((bookmark-at-point (nth 2 (vtable-current-object)))
+         (index-bap (seq-position eww-bookmarks bookmark-at-point))
+         (position (point)))
+    ;; TODO: a simpler way of doing this?
+    (setq eww-bookmarks (seq-concatenate
+                         'list
+                         (seq-subseq eww-bookmarks 0 index-bap)
+                         (list (pop eww-bookmark-kill-ring))
+                         (seq-subseq eww-bookmarks index-bap)))
+    (eww-write-bookmarks)
+    (vtable-revert-command)
+    (goto-char position)))
 
 (defun eww-bookmark-browse ()
   "Browse the bookmark under point in eww."
   (interactive nil eww-bookmark-mode)
-  (let ((bookmark (get-text-property (line-beginning-position) 'eww-bookmark)))
-    (unless bookmark
+  (let ((bookmark-at-point (nth 2 (vtable-current-object))))
+    (unless bookmark-at-point
       (user-error "No bookmark on the current line"))
     (quit-window)
-    (eww-browse-url (plist-get bookmark :url))))
+    (eww-browse-url (plist-get bookmark-at-point :url))))
 
 (defun eww-next-bookmark ()
   "Go to the next bookmark in the list."
   (interactive nil eww-bookmark-mode)
-  (let ((first nil)
-	bookmark)
+  (let (fresh-buffer target-bookmark)
     (unless (get-buffer "*eww bookmarks*")
-      (setq first t)
-      (eww-read-bookmarks t)
-      (eww-bookmark-prepare))
+      (setq fresh-buffer t)
+      (eww-list-bookmarks t))
     (with-current-buffer "*eww bookmarks*"
-      (when (and (not first)
-		 (not (eobp)))
-	(forward-line 1))
-      (setq bookmark (get-text-property (line-beginning-position)
-					'eww-bookmark))
-      (unless bookmark
-	(user-error "No next bookmark")))
-    (eww-browse-url (plist-get bookmark :url))))
+      (unless fresh-buffer
+        (forward-line 1))
+      (setq target-bookmark (nth 2 (vtable-current-object))))
+    (unless target-bookmark
+      ;; usually because we moved past end of the table
+      (user-error "No next bookmark"))
+    (eww-browse-url (plist-get target-bookmark :url))))
 
 (defun eww-previous-bookmark ()
   "Go to the previous bookmark in the list."
   (interactive nil eww-bookmark-mode)
-  (let ((first nil)
-	bookmark)
+  (let (fresh-buffer target-bookmark)
     (unless (get-buffer "*eww bookmarks*")
-      (setq first t)
-      (eww-read-bookmarks t)
-      (eww-bookmark-prepare))
+      (setq fresh-buffer t)
+      (eww-list-bookmarks t))
     (with-current-buffer "*eww bookmarks*"
-      (if first
-	  (goto-char (point-max))
-	(beginning-of-line))
-      ;; On the final line.
-      (when (eolp)
-	(forward-line -1))
-      (if (bobp)
-	  (user-error "No previous bookmark")
-	(forward-line -1))
-      (setq bookmark (get-text-property (line-beginning-position)
-					'eww-bookmark)))
-    (eww-browse-url (plist-get bookmark :url))))
+      (when fresh-buffer
+	(vtable-end-of-table))
+      ;; didn't move to a previous line, because we
+      ;; were already on the first one
+      (unless (= -1 (forward-line -1))
+        (setq target-bookmark (nth 2 (vtable-current-object)))))
+    (unless target-bookmark
+      (user-error "No previous bookmark"))
+    (eww-browse-url (plist-get target-bookmark :url))))
 
 (defun eww-bookmark-urls ()
   "Get the URLs from the current list of bookmarks."
@@ -2489,16 +2498,29 @@ eww-bookmark-urls
   (eww-read-bookmarks)
   (mapcar (lambda (x) (plist-get x :url)) eww-bookmarks))
 
+(defun eww-bookmark-undo-sort ()
+  "Remove any sorting by column of the bookmarks vtable.
+This is required before killing and yanking bookmarks to re-arrange
+them."
+  (interactive)
+  (let ((bookmark-at-point (vtable-current-object)))
+    (setf (vtable-sort-by (vtable-current-table)) nil)
+    (vtable-revert-command)
+    (while (not (equal (vtable-current-object)
+                       bookmark-at-point))
+      (forward-line 1))))
+
 (defvar-keymap eww-bookmark-mode-map
   "C-k" #'eww-bookmark-kill
   "C-y" #'eww-bookmark-yank
+  "u" #'eww-bookmark-undo-sort
   "RET" #'eww-bookmark-browse
   :menu '("Eww Bookmark"
           ["Exit" quit-window t]
           ["Browse" eww-bookmark-browse
-           :active (get-text-property (line-beginning-position) 'eww-bookmark)]
+           :active (nth 2 (vtable-current-object))]
           ["Kill" eww-bookmark-kill
-           :active (get-text-property (line-beginning-position) 'eww-bookmark)]
+           :active (nth 2 (vtable-current-object))]
           ["Yank" eww-bookmark-yank
            :active eww-bookmark-kill-ring]))
 
-- 
2.45.2.windows.1


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



-- 
Sebastián Monía
https://site.sebasmonia.com/

Re: [PATCH] Use vtable for eww-bookmarks

[Eli Zaretskii]

> From: Sebastián Monía <sebastian@sebasmonia.com>
> Cc: Jim Porter <jporterbugs@gmail.com>
> Date: Tue, 05 Nov 2024 17:53:44 -0500
> 
> The attached patch is a first attempt at bringing eww-bookmarks in line
> with the previous switch of eww-buffer-list to vtable.
> 
> There are features of eww-bookmarks I had no idea about, like the
> ability to re-arrange them by killing and yaking. Or navigating to the
> next bookmark from an eww buffer or invoking the command via M-x.

I didn't try that, but if it changes the UI and UX, we need to think
hard whether we want to surprise users with such changes.  What's the
rationale and the motivation for this?

Re: [PATCH] Use vtable for eww-bookmarks

[Sebastián Monía]

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

Eli Zaretskii <eliz@gnu.org> writes:
>> From: Sebastián Monía <sebastian@sebasmonia.com>
>> Cc: Jim Porter <jporterbugs@gmail.com>
>> Date: Tue, 05 Nov 2024 17:53:44 -0500
>> 
>> The attached patch is a first attempt at bringing eww-bookmarks in line
>> with the previous switch of eww-buffer-list to vtable.
>> 
>> There are features of eww-bookmarks I had no idea about, like the
>> ability to re-arrange them by killing and yaking. Or navigating to the
>> next bookmark from an eww buffer or invoking the command via M-x.
>
> I didn't try that, but if it changes the UI and UX, we need to think
> hard whether we want to surprise users with such changes.  What's the
> rationale and the motivation for this?

We decided a few weeks ago that to support reverting, the existing
'eww-list-buffers' command could be implemented using a "proper table"
mode. The old code worked by inserting text and adding some properties.

After some conversation we picked vtable for this. The code was marged
recently.

During that work I noticed 'eww-list-bookmarks' also used this artisanal
approach to building the table, and suggested it would be nice to
convert it to vtable, and make it consistent with the new buffer list.
This is the patch that follows on that suggestion.

Regarding UI/UX changes, there isn't much of a difference on how the
table looks. The new one supports sorting though. Although I had to add
a binding to "undo sorting", to support an exising feature of
eww-bookmarks that lets you re-order the list.


[-- Attachment #2: old listing --]
[-- Type: image/png, Size: 26637 bytes --]

[-- Attachment #3: new listing --]
[-- Type: image/png, Size: 27267 bytes --]

[-- Attachment #4: new listing, sorted --]
[-- Type: image/png, Size: 29674 bytes --]

[-- Attachment #5: Type: text/plain, Size: 54 bytes --]


-- 
Sebastián Monía
https://site.sebasmonia.com/

Re: [PATCH] Use vtable for eww-bookmarks

[Eli Zaretskii]

> From: Sebastián Monía <sebastian@sebasmonia.com>
> Cc: emacs-devel@gnu.org,  jporterbugs@gmail.com
> Date: Wed, 06 Nov 2024 08:36:00 -0500
> 
> > I didn't try that, but if it changes the UI and UX, we need to think
> > hard whether we want to surprise users with such changes.  What's the
> > rationale and the motivation for this?
> 
> We decided a few weeks ago that to support reverting, the existing
> 'eww-list-buffers' command could be implemented using a "proper table"
> mode. The old code worked by inserting text and adding some properties.
> 
> After some conversation we picked vtable for this. The code was marged
> recently.
> 
> During that work I noticed 'eww-list-bookmarks' also used this artisanal
> approach to building the table, and suggested it would be nice to
> convert it to vtable, and make it consistent with the new buffer list.
> This is the patch that follows on that suggestion.
> 
> Regarding UI/UX changes, there isn't much of a difference on how the
> table looks. The new one supports sorting though. Although I had to add
> a binding to "undo sorting", to support an exising feature of
> eww-bookmarks that lets you re-order the list.

Thanks, then I guess there should be no problems with these changes.

Re: [PATCH] Use vtable for eww-bookmarks

[Visuwesh]

[புதன் நவம்பர் 06, 2024] Sebastián Monía wrote:

> Eli Zaretskii <eliz@gnu.org> writes:
>>> From: Sebastián Monía <sebastian@sebasmonia.com>
>>> Cc: Jim Porter <jporterbugs@gmail.com>
>>> Date: Tue, 05 Nov 2024 17:53:44 -0500
>>> 
>>> The attached patch is a first attempt at bringing eww-bookmarks in line
>>> with the previous switch of eww-buffer-list to vtable.
>>> 
>>> There are features of eww-bookmarks I had no idea about, like the
>>> ability to re-arrange them by killing and yaking. Or navigating to the
>>> next bookmark from an eww buffer or invoking the command via M-x.
>>
>> I didn't try that, but if it changes the UI and UX, we need to think
>> hard whether we want to surprise users with such changes.  What's the
>> rationale and the motivation for this?
>
> We decided a few weeks ago that to support reverting, the existing
> 'eww-list-buffers' command could be implemented using a "proper table"
> mode. The old code worked by inserting text and adding some properties.
>
> After some conversation we picked vtable for this. The code was marged
> recently.
>
> During that work I noticed 'eww-list-bookmarks' also used this artisanal
> approach to building the table, and suggested it would be nice to
> convert it to vtable, and make it consistent with the new buffer list.
> This is the patch that follows on that suggestion.
>
> Regarding UI/UX changes, there isn't much of a difference on how the
> table looks. The new one supports sorting though. Although I had to add
> a binding to "undo sorting", to support an exising feature of
> eww-bookmarks that lets you re-order the list.

Can the -> bitmaps in the fringe be removed from the new listing?  It is
ugly (and quite misleading too).  Or is there a new column that is
hidden in the new listing that is not shown in the screenshot?

Re: [PATCH] Use vtable for eww-bookmarks

[Sebastián Monía]

Visuwesh <visuweshm@gmail.com> writes:
> [புதன் நவம்பர் 06, 2024] Sebastián Monía wrote:
>
> Can the -> bitmaps in the fringe be removed from the new listing?  It is
> ugly (and quite misleading too).  Or is there a new column that is
> hidden in the new listing that is not shown in the screenshot?

Thanks for the feedback, will look into it.

-- 
Sebastián Monía
https://site.sebasmonia.com/

Re: [PATCH] Use vtable for eww-bookmarks

[Sebastián Monía]

Sebastián Monía <sebastian@sebasmonia.com> writes:
> Visuwesh <visuweshm@gmail.com> writes:
>> [புதன் நவம்பர் 06, 2024] Sebastián Monía wrote:
>>
>> Can the -> bitmaps in the fringe be removed from the new listing?  It is
>> ugly (and quite misleading too).  Or is there a new column that is
>> hidden in the new listing that is not shown in the screenshot?
>
> Thanks for the feedback, will look into it.

Turns out that below what's visible in the screenshot, some of the URLs
were wider than the windows & frame. When I maximize the frame, the
fringe disappears.

-- 
Sebastián Monía
https://site.sebasmonia.com/

Re: [PATCH] Use vtable for eww-bookmarks

[Visuwesh]

[புதன் நவம்பர் 06, 2024] Sebastián Monía wrote:

> Sebastián Monía <sebastian@sebasmonia.com> writes:
>> Visuwesh <visuweshm@gmail.com> writes:
>>> [புதன் நவம்பர் 06, 2024] Sebastián Monía wrote:
>>>
>>> Can the -> bitmaps in the fringe be removed from the new listing?  It is
>>> ugly (and quite misleading too).  Or is there a new column that is
>>> hidden in the new listing that is not shown in the screenshot?
>>
>> Thanks for the feedback, will look into it.
>
> Turns out that below what's visible in the screenshot, some of the URLs
> were wider than the windows & frame. When I maximize the frame, the
> fringe disappears.

I see, thank you for the confirmation.

Re: [PATCH] Use vtable for eww-bookmarks

[Visuwesh]

[புதன் நவம்பர் 06, 2024] Sebastián Monía wrote:

> Sebastián Monía <sebastian@sebasmonia.com> writes:
>> Visuwesh <visuweshm@gmail.com> writes:
>>> [புதன் நவம்பர் 06, 2024] Sebastián Monía wrote:
>>>
>>> Can the -> bitmaps in the fringe be removed from the new listing?  It is
>>> ugly (and quite misleading too).  Or is there a new column that is
>>> hidden in the new listing that is not shown in the screenshot?
>>
>> Thanks for the feedback, will look into it.
>
> Turns out that below what's visible in the screenshot, some of the URLs
> were wider than the windows & frame. When I maximize the frame, the
> fringe disappears.

I see, thank you for the confirmation.

Re: [PATCH] Use vtable for eww-bookmarks

[Sebastián Monía]

Sebastián Monía <sebastian@sebasmonia.com> writes:
> Visuwesh <visuweshm@gmail.com> writes:
>> [புதன் நவம்பர் 06, 2024] Sebastián Monía wrote:
>>
>> Can the -> bitmaps in the fringe be removed from the new listing?  It is
>> ugly (and quite misleading too).  Or is there a new column that is
>> hidden in the new listing that is not shown in the screenshot?
>
> Thanks for the feedback, will look into it.

I realized today that Jim P. replied to these emails and I replied back
without including the rest of the addresses (again, ooops).

I said "the reason for the fringe bitmaps was that some table elements
were larger than the window width. That's because I made the frame small
so the screenshot wasn't too big. And also I didn't capture the whole
list, just the top portion."

And Jim's reply:

> Right. However, the fringe bitmap is present even on rows that would
> fit entirely in the window. That's because rows whose last cell is
> short have trailing whitespace to fill them out to the max width of
> that column. I don't think that trailing whitespace is necessary (at
> least not how EWW is using vtable), and if we removed it, you'd only
> see the fringe bitmap for individual rows that *don't* fit entirely in
> the window.
> 
> - Jim

I think we could tackle this vtable issue separate from the eww-bookmark
changes. But open to your suggestions. All of you, now that I included
everyone 🙃

Thanks,
Seb

-- 
Sebastián Monía
https://site.sebasmonia.com/

Re: [PATCH] Use vtable for eww-bookmarks

[Jim Porter]

On 11/6/2024 6:43 AM, Visuwesh wrote:
> Can the -> bitmaps in the fringe be removed from the new listing?  It is
> ugly (and quite misleading too).  Or is there a new column that is
> hidden in the new listing that is not shown in the screenshot?

Looking at this, I believe it might be a bug in the vtable code. For 
every (left-aligned) cell, vtable adds space to the end so it's the same 
width as the largest cell in that column. That's important for the 
*non*-last columns of course, but I can't figure out why we need that 
for the last column. Does anyone know? Could we just remove that space? 
I think it'd be less misleading without it.

Re: [PATCH] Use vtable for eww-bookmarks

[Jim Porter]

On 11/5/2024 2:53 PM, Sebastián Monía wrote:
> Hi all,
> 
> The attached patch is a first attempt at bringing eww-bookmarks in line
> with the previous switch of eww-buffer-list to vtable.
> 
> There are features of eww-bookmarks I had no idea about, like the
> ability to re-arrange them by killing and yaking. Or navigating to the
> next bookmark from an eww buffer or invoking the command via M-x.

I think this looks good overall. Just one thought: instead of requiring 
users to manually undo sorting when killing/yanking, what if we had 
'eww-bookmark-kill' and 'eww-yank-bookmark' prompt the user to ask if 
they want to undo the sorting first? Something like this (untested):

   (defun eww-bookmark-kill (&optional interactive)
     (interactive "p")
     (when (and interactive
                (vtable-current-table)
                (vtable-sort-by (vtable-current-table)))
       (if (y-or-n-p "Reset sorting of bookmarks?")
           (eww-bookmark-undo-sort)
         (user-error "Can't kill sorted bookmarks")))
     ;; ...
     )

Then we might not even need a keybinding for 'eww-bookmark-sort'.

Re: [PATCH] Use vtable for eww-bookmarks

[Sebastián Monía]

On Sat, Nov 23, 2024, at 2:12 PM, Jim Porter wrote:
> I think this looks good overall. Just one thought: instead of requiring 
> users to manually undo sorting when killing/yanking, what if we had 
> 'eww-bookmark-kill' and 'eww-yank-bookmark' prompt the user to ask if 
> they want to undo the sorting first? 

The problem is, when yanking, do you yank the bookmark to the
current position on the table? or the one where you will end up after
undoing the sort?

That (and a few other cases like it) made me opt for having the user
disable sorting explicitly.
Or even considering just not sorting at all, although it seems like a
useful feature for all cases but the kill/yank scenario.

Re: [PATCH] Use vtable for eww-bookmarks

[Jim Porter]

On 11/27/2024 12:02 PM, Sebastián Monía wrote:
> On Sat, Nov 23, 2024, at 2:12 PM, Jim Porter wrote:
>> I think this looks good overall. Just one thought: instead of requiring
>> users to manually undo sorting when killing/yanking, what if we had
>> 'eww-bookmark-kill' and 'eww-yank-bookmark' prompt the user to ask if
>> they want to undo the sorting first?
> 
> The problem is, when yanking, do you yank the bookmark to the
> current position on the table? or the one where you will end up after
> undoing the sort?

Good point. For what it's worth, I tried reordering bookmarks in the 
"Library" window in Firefox, and it works as in your patch: when sorting 
is active, you can't reorder the bookmarks. Firefox just shows a "No" 
icon and doesn't let you drag-and-drop.

Given that, let's go with your implementation, though I do have one last 
concern with the "undo sort": the keybinding ("u") is the same as what 
other table-like modes (e.g. dired, list-packages) use for "unmark". 
Currently we don't support marking/unmarking in the bookmarks menu, but 
that seems like a useful thing we might add one day: you could mark 
several bookmarks in order to open them all, delete them all, add some 
kind of tag to them, etc.

I'm not sure what a better key would be though. Maybe "C" for "clear sort"?