bug#63785: 29.0.91; Clearing package list filters also clears flags

bug#63785: 29.0.91; Clearing package list filters also clears flags

[Robert Pluim]

M-x list-packages
/ n ack ;; navigate to the line for 'ack'
i ;; 'ack' has the 'I' flag
/ / ;; 'ack' no longer has the I flag


I was expecting the filtering commands and the action commands not to
interact at all. This makes it harder to do 'search for foo, mark,
search for bar, mark, install', you have to remember to execute the
action after each mark command.

Thanks

Robert

In GNU Emacs 29.0.91 (build 25, x86_64-pc-linux-gnu, GTK+ Version
 3.24.24, cairo version 1.16.0) of 2023-05-29 built on rltb
Repository revision: a7883d52f808568816e3f144fadbeacdd6f2cdfe
Repository branch: emacs-29
Windowing system distributor 'The X.Org Foundation', version 11.0.12009000
System Description: Debian GNU/Linux 11 (bullseye)

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY
INOTIFY PDUMPER PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS WEBP X11 XDBE XIM XINPUT2 XPM GTK3 ZLIB

Robert
-- 

bug#63785: 29.0.91; Clearing package list filters also clears flags

[Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Robert Pluim <rpluim@gmail.com> writes:

> M-x list-packages
> / n ack ;; navigate to the line for 'ack'
> i ;; 'ack' has the 'I' flag
> / / ;; 'ack' no longer has the I flag
>
> This makes it harder to do 'search for foo, mark,
> search for bar, mark, install', you have to remember to execute the
> action after each mark command.
>

That's an interesting issue, it can be a bit tricky because while
filtering for "bar", "foo" is no longer anywhere to be found in the
buffer.  So AFAIU to solve this Emacs needs to maintain something like a
buffer-local association between list entry (package) ids and the tags
they were given.

This sounds like something that applies more broadly to other
`tabulated-list-mode` derivatives as well.  The attached patch adds a
`tabulated-list-maintain-tags` variable and enables it in
`package-menu-mode`.  With it I can search and mark foo, then search and
mark bar, and then install both, as you say.  WDYT?



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Maintain-tags-when-filtering-packages.patch --]
[-- Type: text/x-patch, Size: 3178 bytes --]

From d94febc93f840a28187cd9e3831be0deb0fa4c61 Mon Sep 17 00:00:00 2001
From: Eshel Yaron <me@eshelyaron.com>
Date: Mon, 29 May 2023 19:23:12 +0300
Subject: [PATCH] Maintain tags when filtering packages

Allow 'tabulated-list-mode' derivatives to maintain tags when
repopulating the buffer.  Enable this option in 'package-menu-mode'.

* lisp/emacs-lisp/tabulated-list.el (tabulated-list-tags-alist)
(tabulated-list-maintain-tags): New buffer-local variables.
(tabulated-list-put-tag): Associate entries with tags in
'tabulated-list-tags-alist'.
(tabulated-list-print): Restore tags if 'tabulated-list-maintain-tags'
is non-nil.

* lisp/emacs-lisp/package.el (package-menu-mode): Enable
'tabulated-list-maintain-tags'.
(Bug#63785)
---
 lisp/emacs-lisp/package.el        |  1 +
 lisp/emacs-lisp/tabulated-list.el | 16 ++++++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 69595601bc8..a050f405ba0 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -3145,6 +3145,7 @@ package-menu-mode
           ("Description" 0 package-menu--description-predicate)])
   (setq tabulated-list-padding 2)
   (setq tabulated-list-sort-key (cons "Status" nil))
+  (setq tabulated-list-maintain-tags t)
   (add-hook 'tabulated-list-revert-hook #'package-menu--refresh nil t)
   (tabulated-list-init-header)
   (setq revert-buffer-function 'package-menu--refresh-contents)
diff --git a/lisp/emacs-lisp/tabulated-list.el b/lisp/emacs-lisp/tabulated-list.el
index ddac6ed1746..e1922279039 100644
--- a/lisp/emacs-lisp/tabulated-list.el
+++ b/lisp/emacs-lisp/tabulated-list.el
@@ -167,6 +167,12 @@ tabulated-list-sort-key
 non-nil, means to invert the resulting sort.")
 (put 'tabulated-list-sort-key 'permanent-local t)
 
+(defvar-local tabulated-list-tags-alist nil
+  "Association between entry ids and their tags in the current buffer.")
+
+(defvar-local tabulated-list-maintain-tags nil
+  "When non-nil, maintain tags when repopulating the current tabulated list.")
+
 (defsubst tabulated-list-get-id (&optional pos)
   "Return the entry ID of the Tabulated List entry at POS.
 The value is an ID object from `tabulated-list-entries', or nil.
@@ -200,6 +206,8 @@ tabulated-list-put-tag
 		       (make-string (- tabulated-list-padding width) ?\s))
 	     (truncate-string-to-width tag tabulated-list-padding))))
 	(delete-region beg (+ beg tabulated-list-padding)))))
+  (when-let ((id (tabulated-list-get-id)))
+    (setf (alist-get id tabulated-list-tags-alist) tag))
   (if advance
       (forward-line)))
 
@@ -497,6 +505,14 @@ tabulated-list-print
       (setq entries (cdr entries)))
     (when update
       (delete-region (point) (point-max)))
+    (when tabulated-list-maintain-tags
+      (save-excursion
+        (goto-char (point-min))
+        (while (not (eobp))
+          (when-let ((id (tabulated-list-get-id))
+                     (tag (alist-get id tabulated-list-tags-alist)))
+            (tabulated-list-put-tag tag))
+          (forward-line))))
     (set-buffer-modified-p nil)
     ;; If REMEMBER-POS was specified, move to the "old" location.
     (if saved-pt
-- 
2.40.1

bug#63785: 29.0.91; Clearing package list filters also clears flags

[Robert Pluim]

>>>>> On Mon, 29 May 2023 19:48:45 +0300, Eshel Yaron <me@eshelyaron.com> said:

    Eshel> Robert Pluim <rpluim@gmail.com> writes:
    >> M-x list-packages
    >> / n ack ;; navigate to the line for 'ack'
    >> i ;; 'ack' has the 'I' flag
    >> / / ;; 'ack' no longer has the I flag
    >> 
    >> This makes it harder to do 'search for foo, mark,
    >> search for bar, mark, install', you have to remember to execute the
    >> action after each mark command.
    >> 

    Eshel> That's an interesting issue, it can be a bit tricky because while
    Eshel> filtering for "bar", "foo" is no longer anywhere to be found in the
    Eshel> buffer.  So AFAIU to solve this Emacs needs to maintain something like a
    Eshel> buffer-local association between list entry (package) ids and the tags
    Eshel> they were given.

    Eshel> This sounds like something that applies more broadly to other
    Eshel> `tabulated-list-mode` derivatives as well.  The attached patch adds a
    Eshel> `tabulated-list-maintain-tags` variable and enables it in
    Eshel> `package-menu-mode`.  With it I can search and mark foo, then search and
    Eshel> mark bar, and then install both, as you say.  WDYT?

Itʼs close. The following is still suprising

/ n helm
i
/ /
/ n ack
i
x

That only offer to installs ack, even though Iʼve marked helm. If I
clear the filters then 'x' offers to install both. Although I could
argue that Iʼm in a filtered state, so offering to install only
visible packages is normal :-)

Robert
-- 

bug#63785: 29.0.91; Clearing package list filters also clears flags

[Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Robert Pluim <rpluim@gmail.com> writes:

>>>>>> On Mon, 29 May 2023 19:48:45 +0300, Eshel Yaron <me@eshelyaron.com> said:
>
>     Eshel> Robert Pluim <rpluim@gmail.com> writes:
>     >> M-x list-packages
>     >> / n ack ;; navigate to the line for 'ack'
>     >> i ;; 'ack' has the 'I' flag
>     >> / / ;; 'ack' no longer has the I flag
>     >> 
>     >> This makes it harder to do 'search for foo, mark,
>     >> search for bar, mark, install', you have to remember to execute the
>     >> action after each mark command.
>     >> 
>
>     Eshel> That's an interesting issue, it can be a bit tricky because while
>     Eshel> filtering for "bar", "foo" is no longer anywhere to be found in the
>     Eshel> buffer.  So AFAIU to solve this Emacs needs to maintain something like a
>     Eshel> buffer-local association between list entry (package) ids and the tags
>     Eshel> they were given.
>
>     Eshel> This sounds like something that applies more broadly to other
>     Eshel> `tabulated-list-mode` derivatives as well.  The attached patch adds a
>     Eshel> `tabulated-list-maintain-tags` variable and enables it in
>     Eshel> `package-menu-mode`.  With it I can search and mark foo, then search and
>     Eshel> mark bar, and then install both, as you say.  WDYT?
>
> Itʼs close. The following is still suprising
>
> / n helm
> i
> / /
> / n ack
> i
> x
>
> That only offer to installs ack, even though Iʼve marked helm. If I
> clear the filters then 'x' offers to install both.  Although I could
> argue that Iʼm in a filtered state, so offering to install only
> visible packages is normal :-)

Yes, that's what I thought.  Only installing visible packages when the
menu is filtered seems sensible to me.  Although I guess
`package-menu-execute` could be extended with some option that says what
to do in these cases, maybe by default it could suggest clearing the
filter first?

bug#63785: 29.0.91; Clearing package list filters also clears flags

[Robert Pluim]

>>>>> On Tue, 30 May 2023 11:43:51 +0300, Eshel Yaron <me@eshelyaron.com> said:
    >> That only offer to installs ack, even though Iʼve marked helm. If I
    >> clear the filters then 'x' offers to install both.  Although I could
    >> argue that Iʼm in a filtered state, so offering to install only
    >> visible packages is normal :-)

    Eshel> Yes, that's what I thought.  Only installing visible packages when the
    Eshel> menu is filtered seems sensible to me.  Although I guess
    Eshel> `package-menu-execute` could be extended with some option that says what
    Eshel> to do in these cases, maybe by default it could suggest clearing the
    Eshel> filter first?

Maybe. Iʼm wary of adding too much DWIMery here. Limiting the
installation to the visible packages seems like a reasonable and
unsurprising way to behave (and people who want to install all the
selected packages just need to do '/ /')

Robert
--