bug#25049: ibuffer bug when saving existing filter, with patches

[Christopher Genovese]

Hello,

In at least Emacs 25+, the function 'ibuffer-save-filters'
handles saving existing filters and new filters inconsistently.
Specifically, at the following point in the original function:

  (ibuffer-aif (assoc name ibuffer-saved-filters)
      (setcdr it filters)
    (push (list name filters) ibuffer-saved-filters))

the setcdr (existing filters) and the push (new filters)
save different formats for the variable, with the latter
having an extra layer of parentheses.

As a specific example of failure, using the current default
value of 'ibuffer-saved-filters'

        (("gnus"
          ((or
            (mode . message-mode)
            (mode . mail-mode)
            (mode . gnus-group-mode)
            (mode . gnus-summary-mode)
            (mode . gnus-article-mode))))
         ("programming"
          ((or
            (mode . emacs-lisp-mode)
            (mode . cperl-mode)
            (mode . c-mode)
            (mode . java-mode)
            (mode . idl-mode)
            (mode . lisp-mode)))))

and doing

   (ibuffer-save-filters "foo" '((name . "foo") (derived-mode . text-mode)))
   (ibuffer-save-filters "gnus" '((filename . ".")
                                  (or (derived-mode . prog-mode)
                                      (mode . "compilation-mode"))))

gives the following incorrect value for `ibuffer-saved-filters'

        (("foo"
          ((name . "foo")
           (derived-mode . text-mode)))
         ("gnus"
          (filename . ".")
          (or
           (derived-mode . prog-mode)
           (mode . "compilation-mode")))
         ("programming"
          ((or
            (mode . emacs-lisp-mode)
            (mode . cperl-mode)
            (mode . c-mode)
            (mode . java-mode)
            (mode . idl-mode)
            (mode . lisp-mode)))))

because the "foo" and "gnus" entries have different formats,
the latter not consistent with later code to access it
(e.g., in 'ibuffer-included-in-filter-p-1' and 'ibuffer-decompose-filter').

There are two immediate paths for fixing this:

  1. Change the setcdr to add the extra level of nesting.
  2. Change the format of 'ibuffer-saved-filters' to remove
     the level of testing, change the push (list->cons) and
     the later accesses (cadr->cdr).

The first is very simple, but the extra level of nesting
is unsightly, inconsistent with the structure of filter groups,
and mildly annoying when writing filters by hand.  The
second is fairly simple, requiring only a few more small changes,
but introduces the complication of changing an existing
variable's format. (For what it's worth, I prefer the second.)

I have attached a patch file with patches for three commits
as follows:

  1/3: The very simple fix corresponding to choice 1 above.
  2/3: The simple fix corresponding to choice 2 above.
  3/3: Some extra support added for handling the change
       in format for 'ibuffer-saved-filters'.

Let me know if you need any further information.

Thanks, Chris

ibuffer-saved-filters-bug.patch
Description: Text Data