bug#41493: 26.3; Error in MPC directory browser

[Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

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

Hi John,

> The attached patch fixes it for me. The cause seems to be that the
> returned alist for a directory browser has entries for (directory
> ...) and (Last-Modified ...) and the code was only getting rid of
> the former only. That leftover (Last-Modified ...) would then trip
> up the assert in mpc--proc-alist-to-alists when it checks with
> mpc--proc-alist-to-alists-starters.
[...]
> diff --git a/lisp/mpc.el b/lisp/mpc.el
> index 4317fece6fc..05ef546f884 100644
> --- a/lisp/mpc.el
> +++ b/lisp/mpc.el
> @@ -644,9 +644,10 @@ mpc-cmd-find
>                          (mpc-proc-buf-to-alist
>                           (mpc-proc-cmd (list "listallinfo" value)))))
>                     (mpc--proc-alist-to-alists
> -                    ;; Strip away the `directory' entries.
> +                    ;; Strip away the `directory' & `Last-Modified' entries.
>                      (delq nil (mapcar (lambda (pair)
> -                                        (if (eq (car pair) 'directory)
> +                                        (if (or (eq (car pair) 'directory)
> +                                                (eq (car pair) 'Last-Modified))
>                                              nil pair))
>                                        pairs)))))
>                  ((string-match "|" (symbol-name tag))

Hmm... IIUC the output we receive is a list of pairs made of sublists of
the form either:

    (directory . <...>)
    (Last-Modified . <...>)

or

    (file . <...>)
    (Last-Modified . <...>)
    (Format . <...>)
    (Title . <...>)
    ...

IIUC when I wrote the code there were no such `Last-Modified` entries.

We're interested in extracting a list of the files (where each file is
represented by its alist), so I just stripped away all the `directory`
entries and that was it.  But now that `directory` can be followed by
info about that directory (currently only `Last-Modified`), we should
arguably be more careful to also remove everything between
`directory` and the next `file` or `directory`.

So I think the patch below is "more robust".
WDYT?
I also think it's a good opportunity to add some comments/docstrings
because it took me a while to understand what this was trying to do.


        Stefan

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

diff --git a/lisp/mpc.el b/lisp/mpc.el
index ad11007f064..ca450fd959a 100644
--- a/lisp/mpc.el
+++ b/lisp/mpc.el
@@ -312,11 +312,11 @@ mpc--proc-alist-to-alists-starters
 (defun mpc--proc-alist-to-alists (alist)
   (cl-assert (or (null alist)
               (memq (caar alist) mpc--proc-alist-to-alists-starters)))
-  (let ((starter (caar alist))
+  (let (;; (starter (caar alist))
         (alists ())
         tmp)
     (dolist (pair alist)
-      (when (eq (car pair) starter)
+      (when (memq (car pair) mpc--proc-alist-to-alists-starters)
         (if tmp (push (nreverse tmp) alists))
         (setq tmp ()))
       (push pair tmp))
@@ -638,15 +638,14 @@ mpc-cmd-find
                  (mpc-proc-buf-to-alists
                   (mpc-proc-cmd (list "search" "any" value))))
                 ((eq tag 'Directory)
-                 (let ((pairs
-                        (mpc-proc-buf-to-alist
+                 (let ((entries
+                        (mpc-proc-buf-to-alists
                          (mpc-proc-cmd (list "listallinfo" value)))))
-                   (mpc--proc-alist-to-alists
-                    ;; Strip away the `directory' entries.
-                    (delq nil (mapcar (lambda (pair)
-                                        (if (eq (car pair) 'directory)
-                                            nil pair))
-                                      pairs)))))
+                   ;; Strip away the `directory' entries.
+                   (delq nil (mapcar (lambda (entry)
+                                       (if (eq (caar entry) 'directory)
+                                           nil entry))
+                                     entries))))
                 ((string-match "|" (symbol-name tag))
                  (add-to-list 'mpc--find-memoize-union-tags tag)
                  (let ((tag1 (intern (substring (symbol-name tag)