* bug#25049: ibuffer bug when saving existing filter, with patches
@ 2016-11-28 6:54 Christopher Genovese
2016-11-29 15:06 ` Tino Calancha
0 siblings, 1 reply; 12+ messages in thread
From: Christopher Genovese @ 2016-11-28 6:54 UTC (permalink / raw)
To: 25049; +Cc: Tino Calancha
[-- Attachment #1.1: Type: text/plain, Size: 2986 bytes --]
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
[-- Attachment #1.2: Type: text/html, Size: 3920 bytes --]
[-- Attachment #2: ibuffer-saved-filters-bug.patch --]
[-- Type: text/x-diff, Size: 17727 bytes --]
From 01e41027abf27f8fca01f7e4b6fead55c032b1ed Mon Sep 17 00:00:00 2001
From: "Christopher R. Genovese" <genovese@cmu.edu>
Date: Sun, 27 Nov 2016 23:34:50 -0500
Subject: [PATCH 1/3] ibuffer: one possible fix for bug when saving existing
filter
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.
This commit takes the first choice.
Change Log:
2016-11-27 Christopher R. Genovese <genovese@cmu.edu>
* lisp/ibuf-ext.el (ibuffer-save-filters): added extra
level of nesting when saving filter to an existing name.
---
lisp/ibuf-ext.el | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lisp/ibuf-ext.el b/lisp/ibuf-ext.el
index 5ef0746..103232c 100644
--- a/lisp/ibuf-ext.el
+++ b/lisp/ibuf-ext.el
@@ -935,7 +935,7 @@ Interactively, prompt for NAME, and use the current filters."
(read-from-minibuffer "Save current filters as: ")
ibuffer-filtering-qualifiers)))
(ibuffer-aif (assoc name ibuffer-saved-filters)
- (setcdr it filters)
+ (setcdr it (list filters))
(push (list name filters) ibuffer-saved-filters))
(ibuffer-maybe-save-stuff))
--
2.10.0
From 0791820e0ff3665b447ea174b4b915a36dc3eced Mon Sep 17 00:00:00 2001
From: "Christopher R. Genovese" <genovese@cmu.edu>
Date: Mon, 28 Nov 2016 00:33:36 -0500
Subject: [PATCH 2/3] ibuffer: another fix for bug when saving an existing
filter
As described in the previous commit, the function 'ibuffer-save-filters'
handles saving existing filters and new filters inconsistently,
and there are two paths to fixing the problem.
The previous commit fixed the problem while maintaining the format of
the variable 'ibuffer-saved-filters' with its extra level of nesting.
This commit instead takes the second path: removing the extra level of
nesting from the saved filter format.
Change Log:
2016-11-27 Christopher R. Genovese <genovese@cmu.edu>
* lisp/ibuf-ext.el (ibuffer-saved-filters): removed extra
nesting level in the alist values and updated docstring.
(ibuffer-save-filters): removed extra level of nesting
in saved filter alist values when saving new filters.
(ibuffer-included-in-filter-p): changed access of saved
filter data (cadr->cdr) to account for reduced nesting.
(ibuffer-decompose-filter): changed access of saved
filter data (cadr->cdr) to account for reduced nesting.
---
lisp/ibuf-ext.el | 67 ++++++++++++++++++++++++++++----------------------------
1 file changed, 33 insertions(+), 34 deletions(-)
diff --git a/lisp/ibuf-ext.el b/lisp/ibuf-ext.el
index 103232c..e4a7dfa 100644
--- a/lisp/ibuf-ext.el
+++ b/lisp/ibuf-ext.el
@@ -120,28 +120,32 @@ Buffers whose major mode is in this list, are not searched."
(defvar ibuffer-auto-buffers-changed nil)
(defcustom 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)))))
-
- "An alist of filter qualifiers to switch between.
-
-This variable should look like ((\"STRING\" QUALIFIERS)
- (\"STRING\" QUALIFIERS) ...), where
-QUALIFIERS is a list of the same form as
-`ibuffer-filtering-qualifiers'.
-See also the variables `ibuffer-filtering-qualifiers',
-`ibuffer-filtering-alist', and the functions
-`ibuffer-switch-to-saved-filters', `ibuffer-save-filters'."
+ (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))))
+
+ "An alist mapping saved filter names to filter specifications.
+
+Each element should look like (\"NAME\" . FILTER-LIST), where
+FILTER-LIST has the same structure as the variable
+`ibuffer-filtering-qualifiers', which see. The filters defined
+here are joined with an implicit logical `and' and associated
+with NAME. The combined specification can be used by name in
+other filter specifications via the `saved' qualifier (again, see
+`ibuffer-filtering-qualifiers'). They can also be switched to by
+name (see the functions `ibuffer-switch-to-saved-filters' and
+`ibuffer-save-filters'). The variable `ibuffer-save-with-custom'
+affects how this information is saved for future sessions. This
+variable can be set directly from lisp code."
:type '(repeat sexp)
:group 'ibuffer)
@@ -535,13 +539,11 @@ To evaluate a form without viewing the buffer, see `ibuffer-do-eval'."
(ibuffer-included-in-filter-p buf x))
(cdr filter))))
(`saved
- (let ((data
- (assoc (cdr filter)
- ibuffer-saved-filters)))
+ (let ((data (assoc (cdr filter) ibuffer-saved-filters)))
(unless data
(ibuffer-filter-disable t)
(error "Unknown saved filter %s" (cdr filter)))
- (ibuffer-included-in-filters-p buf (cadr data))))
+ (ibuffer-included-in-filters-p buf (cdr data))))
(_
(pcase-let ((`(,_type ,_desc ,func)
(assq (car filter) ibuffer-filtering-alist)))
@@ -849,15 +851,12 @@ turned into two separate filters [name: foo] and [mode: bar-mode]."
(cdr lim)
ibuffer-filtering-qualifiers)))
(`saved
- (let ((data
- (assoc (cdr lim)
- ibuffer-saved-filters)))
+ (let ((data (assoc (cdr lim) ibuffer-saved-filters)))
(unless data
(ibuffer-filter-disable)
(error "Unknown saved filter %s" (cdr lim)))
- (setq ibuffer-filtering-qualifiers (append
- (cadr data)
- ibuffer-filtering-qualifiers))))
+ (setq ibuffer-filtering-qualifiers
+ (append (cdr data) ibuffer-filtering-qualifiers))))
(`not
(push (cdr lim)
ibuffer-filtering-qualifiers))
@@ -935,8 +934,8 @@ Interactively, prompt for NAME, and use the current filters."
(read-from-minibuffer "Save current filters as: ")
ibuffer-filtering-qualifiers)))
(ibuffer-aif (assoc name ibuffer-saved-filters)
- (setcdr it (list filters))
- (push (list name filters) ibuffer-saved-filters))
+ (setcdr it filters)
+ (push (cons name filters) ibuffer-saved-filters))
(ibuffer-maybe-save-stuff))
;;;###autoload
--
2.10.0
From aee1c3cdd47db9c0da2a2f9d1c3a53c40a81cfde Mon Sep 17 00:00:00 2001
From: "Christopher R. Genovese" <genovese@cmu.edu>
Date: Mon, 28 Nov 2016 01:29:04 -0500
Subject: [PATCH 3/3] ibuffer: added support for saved filter format change and
test
As described in the previous commit, one fix to the
inconsistency in 'ibuffer-save-filters' involves simplifying
the format of 'ibuffer-saved-filters' to reduce the extra
nesting level. This adds some support for this transition,
including a customize setter to transparently update old
format values and a command to check and repair the saved
values if desired.
Also added a test of 'ibuffer-save-filter'.
Change Log:
2016-11-27 Christopher R. Genovese <genovese@cmu.edu>
* lisp/ibuf-ext.el (ibuffer-saved-filters): added more accurate
customization type and transparent setter to adjust old-format
values.
(ibuffer-update-saved-filters-format): updates old-format
for saved buffer data to new format with reduced nesting level.
(ibuffer-repair-saved-filters): new command to check and
repair saved filters format.
(ibuffer-old-saved-filters-warning): new variable with
clickable message with repair options to be displayed
as a warning if 'ibuffer-repair-saved-filters' detects
a format mismatch.
* test/lisp/ibuffer-tests.el (ibuffer-save-filters): added
a test that filters are saved in the proper format.
---
lisp/ibuf-ext.el | 87 +++++++++++++++++++++++++++++++++++++++++++++-
test/lisp/ibuffer-tests.el | 29 ++++++++++++++++
2 files changed, 115 insertions(+), 1 deletion(-)
diff --git a/lisp/ibuf-ext.el b/lisp/ibuf-ext.el
index e4a7dfa..1d79729 100644
--- a/lisp/ibuf-ext.el
+++ b/lisp/ibuf-ext.el
@@ -119,6 +119,26 @@ Buffers whose major mode is in this list, are not searched."
(defvar ibuffer-auto-buffers-changed nil)
+(defun ibuffer-update-saved-filters-format (filters)
+ "Transforms alist from old to new `ibuffer-saved-filters' format.
+
+Specifically, converts old-format alist with values of the
+form (STRING (FILTER-SPECS...)) to alist with values of the
+form (STRING FILTER-SPECS...), where each filter spec should be a
+cons cell with a symbol in the car. Any elements in the latter
+form are kept as is.
+
+Returns (OLD-FORMAT-DETECTED? . UPDATED-SAVED-FILTERS-LIST)."
+ (when filters
+ (let* ((old-format-detected nil)
+ (fix-filter (lambda (filter-spec)
+ (if (symbolp (car (cadr filter-spec)))
+ filter-spec
+ (setq old-format-detected t) ; side-effect
+ (cons (car filter-spec) (cadr filter-spec)))))
+ (fixed (mapcar fix-filter filters)))
+ (cons old-format-detected fixed))))
+
(defcustom ibuffer-saved-filters '(("gnus"
(or (mode . message-mode)
(mode . mail-mode)
@@ -146,9 +166,74 @@ name (see the functions `ibuffer-switch-to-saved-filters' and
`ibuffer-save-filters'). The variable `ibuffer-save-with-custom'
affects how this information is saved for future sessions. This
variable can be set directly from lisp code."
- :type '(repeat sexp)
+ :type '(alist :key-type (string :tag "Filter name")
+ :value-type (repeat :tag "Filter specification" sexp))
+ :set (lambda (symbol value)
+ ;; Just set-default but update legacy old-style format
+ (set-default symbol (cdr (ibuffer-update-saved-filters-format value))))
:group 'ibuffer)
+(defvar ibuffer-old-saved-filters-warning
+ (concat "Deprecated format detected for variable `ibuffer-saved-filters'.
+
+The format has been repaired and the variable modified accordingly.
+You can save the current value through the customize system by
+either clicking or hitting return "
+ (make-text-button
+ "here" nil
+ 'face '(:weight bold :inherit button)
+ 'mouse-face '(:weight normal :background "gray50" :inherit button)
+ 'follow-link t
+ 'help-echo "Click or RET: save new value in customize"
+ 'action (lambda (b)
+ (if (not (fboundp 'customize-save-variable))
+ (message "Customize not available; value not saved")
+ (customize-save-variable 'ibuffer-saved-filters
+ ibuffer-saved-filters)
+ (message "Saved updated ibuffer-saved-filters."))))
+ ". See below for
+an explanation and alternative ways to save the repaired value.
+
+Explanation: For the list variable `ibuffer-saved-filters',
+elements of the form (STRING (FILTER-SPECS...)) are deprecated
+and should instead have the form (STRING FILTER-SPECS...), where
+each filter spec is a cons cell with a symbol in the car. See
+`ibuffer-saved-filters' for details. The repaired value fixes
+this format without changing the meaning of the saved filters.
+
+Alternative ways to save the repaired value:
+
+ 1. Do M-x customize-variable and entering `ibuffer-saved-filters'
+ when prompted.
+
+ 2. Set the updated value manually by copying the
+ following emacs-lisp form to your emacs init file.
+
+%s
+"))
+
+(defun ibuffer-repair-saved-filters ()
+ "Updates `ibuffer-saved-filters' to its new-style format, if needed.
+
+If this list has any elements of the old-style format, a
+deprecation warning is raised, with a button allowing persistent
+update. Any updated filters retain their meaning in the new
+format. See `ibuffer-update-saved-filters-format' and
+`ibuffer-saved-filters' for details of the old and new formats."
+ (interactive)
+ (when (and (boundp 'ibuffer-saved-filters) ibuffer-saved-filters)
+ (let ((fixed (ibuffer-update-saved-filters-format ibuffer-saved-filters)))
+ (prog1
+ (setq ibuffer-saved-filters (cdr fixed))
+ (when-let (old-format-detected? (car fixed))
+ (let ((warning-series t)
+ (updated-form
+ (with-output-to-string
+ (pp `(setq ibuffer-saved-filters ',ibuffer-saved-filters)))))
+ (display-warning
+ 'ibuffer
+ (format ibuffer-old-saved-filters-warning updated-form))))))))
+
(defvar ibuffer-filtering-qualifiers nil
"A list like (SYMBOL . QUALIFIER) which filters the current buffer list.
See also `ibuffer-filtering-alist'.")
diff --git a/test/lisp/ibuffer-tests.el b/test/lisp/ibuffer-tests.el
index 3a4def3..6d5187a 100644
--- a/test/lisp/ibuffer-tests.el
+++ b/test/lisp/ibuffer-tests.el
@@ -66,5 +66,34 @@
(mapc (lambda (buf) (when (buffer-live-p buf)
(kill-buffer buf))) (list buf1 buf2)))))
+(ert-deftest ibuffer-save-filters ()
+ "Tests that `ibuffer-save-filters' saves in the proper format."
+ (skip-unless (featurep 'ibuf-ext))
+ (let ((ibuffer-save-with-custom nil)
+ (ibuffer-saved-filters nil)
+ (test1 '((mode . org-mode)
+ (or (size-gt . 10000)
+ (and (not (starred-name))
+ (directory . "\<org\>")))))
+ (test2 '((or (mode . emacs-lisp-mode) (file-extension . "elc?")
+ (and (starred-name) (name . "elisp"))
+ (mode . lisp-interaction-mode))))
+ (test3 '((size-lt . 100) (derived-mode . prog-mode)
+ (or (filename . "scratch")
+ (filename . "bonz")
+ (filename . "temp")))))
+ (ibuffer-save-filters "test1" test1)
+ (should (equal (car ibuffer-saved-filters) (cons "test1" test1)))
+ (ibuffer-save-filters "test2" test2)
+ (should (equal (car ibuffer-saved-filters) (cons "test2" test2)))
+ (should (equal (cadr ibuffer-saved-filters) (cons "test1" test1)))
+ (ibuffer-save-filters "test3" test3)
+ (should (equal (car ibuffer-saved-filters) (cons "test3" test3)))
+ (should (equal (cadr ibuffer-saved-filters) (cons "test2" test2)))
+ (should (equal (car (cddr ibuffer-saved-filters)) (cons "test1" test1)))
+ (should (equal (cdr (assoc "test1" ibuffer-saved-filters)) test1))
+ (should (equal (cdr (assoc "test2" ibuffer-saved-filters)) test2))
+ (should (equal (cdr (assoc "test3" ibuffer-saved-filters)) test3))))
+
(provide 'ibuffer-tests)
;; ibuffer-tests.el ends here
--
2.10.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* bug#25049: ibuffer bug when saving existing filter, with patches
2016-11-28 6:54 bug#25049: ibuffer bug when saving existing filter, with patches Christopher Genovese
@ 2016-11-29 15:06 ` Tino Calancha
2016-11-29 16:09 ` Christopher Genovese
0 siblings, 1 reply; 12+ messages in thread
From: Tino Calancha @ 2016-11-29 15:06 UTC (permalink / raw)
To: 25049; +Cc: Christopher Genovese, tino.calancha
Christopher Genovese <genovese@cmu.edu> writes:
> In at least Emacs 25+, the function 'ibuffer-save-filters'
> handles saving existing filters and new filters inconsistently.
Thank you for the bug report and the extensive analysis!
Below i add some comments.
> 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 also prefer the larger fix 2.
> I have attached a patch file with patches for three commits
1) (eval-when-compiler (require 'subr-x)) is missing
2) We might want to add :version "26.1" into
`ibuffer-saved-filter' definition.
3) When `ibuffer-old-saved-filters-warning' is shown i miss
that the buffer displaying it has the key binding
`TAB' for `forward-button'.
It might be a smarter way to do this, but i just put the buffer
in help-mode as follows:
@@ -11,12 +11,15 @@
(let ((fixed (ibuffer-update-saved-filters-format ibuffer-saved-filters)))
(prog1
(setq ibuffer-saved-filters (cdr fixed))
- (when-let (old-format-detected? (car fixed))
+ (when-let ((old-format-detected? (car fixed))
+ (buffer-name "*Warnings*"))
+ (with-current-buffer (get-buffer-create buffer-name)
+ (help-mode))
(let ((warning-series t)
(updated-form
(with-output-to-string
(pp `(setq ibuffer-saved-filters ',ibuffer-saved-filters)))))
(display-warning
'ibuffer
- (format ibuffer-old-saved-filters-warning updated-form))))))))
-
+ (format ibuffer-old-saved-filters-warning updated-form))
+ nil buffer-name))))))
4) [minor comment] It's not very common in Emacs to
call a var `old-format-detected?'. Adding 'p' instead
of '?' or even not suffix at all looks more usual.
5)
>* lisp/ibuf-ext.el (ibuffer-saved-filters): removed extra
^^^^^^^
>nesting level in the alist values and updated docstring.
>(ibuffer-save-filters): removed extra level of nesting
^^^^^^^
>in saved filter alist values when saving new filters.
Please, start sentences in capital letters. Imperative
form verb is prefered than pasive.
removed extra nesting --> Remove extra nesting
Thank you
Tino
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#25049: ibuffer bug when saving existing filter, with patches
2016-11-29 15:06 ` Tino Calancha
@ 2016-11-29 16:09 ` Christopher Genovese
2016-11-29 23:49 ` Tino Calancha
0 siblings, 1 reply; 12+ messages in thread
From: Christopher Genovese @ 2016-11-29 16:09 UTC (permalink / raw)
To: Tino Calancha; +Cc: 25049
[-- Attachment #1: Type: text/plain, Size: 4157 bytes --]
> 1) (eval-when-compiler (require 'subr-x)) is missing
Ah, sorry. I went back to master and then added in the changes,
but I missed that this time. Thanks.
> 3) When `ibuffer-old-saved-filters-warning' is shown i miss
> that the buffer displaying it has the key binding
> `TAB' for `forward-button'.
> It might be a smarter way to do this, but i just put the buffer
> in help-mode as follows:
That works for me.
> 4) [minor comment] It's not very common in Emacs to
> call a var `old-format-detected?'. Adding 'p' instead
> of '?' or even not suffix at all looks more usual.
OK, makes sense. I had thought of the -p convention
as being for predicate functions, and I do like the
readability of the '?'. But I get it.
> 5)
> Please, start sentences in capital letters. Imperative
> form verb is prefered than pasive.
Understood.
Do you want me to submit a new patch with these changes?
(And I know owe you new patches for the other changes, which I have
not forgotten. I'm just a bit behind but hope to submit those
today.)
Thanks again, Chris
On Tue, Nov 29, 2016 at 10:06 AM, Tino Calancha <tino.calancha@gmail.com>
wrote:
> Christopher Genovese <genovese@cmu.edu> writes:
>
>
> > In at least Emacs 25+, the function 'ibuffer-save-filters'
> > handles saving existing filters and new filters inconsistently.
> Thank you for the bug report and the extensive analysis!
> Below i add some comments.
>
> > 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 also prefer the larger fix 2.
>
> > I have attached a patch file with patches for three commits
>
> 1) (eval-when-compiler (require 'subr-x)) is missing
>
> 2) We might want to add :version "26.1" into
> `ibuffer-saved-filter' definition.
>
> 3) When `ibuffer-old-saved-filters-warning' is shown i miss
> that the buffer displaying it has the key binding
> `TAB' for `forward-button'.
> It might be a smarter way to do this, but i just put the buffer
> in help-mode as follows:
>
> @@ -11,12 +11,15 @@
> (let ((fixed (ibuffer-update-saved-filters-format
> ibuffer-saved-filters)))
> (prog1
> (setq ibuffer-saved-filters (cdr fixed))
> - (when-let (old-format-detected? (car fixed))
> + (when-let ((old-format-detected? (car fixed))
> + (buffer-name "*Warnings*"))
> + (with-current-buffer (get-buffer-create buffer-name)
> + (help-mode))
> (let ((warning-series t)
> (updated-form
> (with-output-to-string
> (pp `(setq ibuffer-saved-filters
> ',ibuffer-saved-filters)))))
> (display-warning
> 'ibuffer
> - (format ibuffer-old-saved-filters-warning
> updated-form))))))))
> -
> + (format ibuffer-old-saved-filters-warning updated-form))
> + nil buffer-name))))))
>
> 4) [minor comment] It's not very common in Emacs to
> call a var `old-format-detected?'. Adding 'p' instead
> of '?' or even not suffix at all looks more usual.
>
> 5)
> >* lisp/ibuf-ext.el (ibuffer-saved-filters): removed extra
> ^^^^^^^
> >nesting level in the alist values and updated docstring.
> >(ibuffer-save-filters): removed extra level of nesting
> ^^^^^^^
> >in saved filter alist values when saving new filters.
> Please, start sentences in capital letters. Imperative
> form verb is prefered than pasive.
>
> removed extra nesting --> Remove extra nesting
>
> Thank you
> Tino
>
[-- Attachment #2: Type: text/html, Size: 5625 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#25049: ibuffer bug when saving existing filter, with patches
2016-11-29 16:09 ` Christopher Genovese
@ 2016-11-29 23:49 ` Tino Calancha
2016-11-30 5:14 ` Christopher Genovese
0 siblings, 1 reply; 12+ messages in thread
From: Tino Calancha @ 2016-11-29 23:49 UTC (permalink / raw)
To: Christopher Genovese; +Cc: 25049, tino.calancha
Christopher Genovese <genovese@cmu.edu> writes:
> Do you want me to submit a new patch with these changes?
It would be useful. Someone other than me may try this patch
without reading all the bug report, and repeat some of my comments.
Once we update the patch we can wait like 1 week; if we don't
get further comments after that week, then it's OK for me to push
the fix to master branch.
Thank you
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#25049: ibuffer bug when saving existing filter, with patches
2016-11-29 23:49 ` Tino Calancha
@ 2016-11-30 5:14 ` Christopher Genovese
2016-11-30 7:02 ` Tino Calancha
0 siblings, 1 reply; 12+ messages in thread
From: Christopher Genovese @ 2016-11-30 5:14 UTC (permalink / raw)
To: Tino Calancha; +Cc: 25049
[-- Attachment #1.1: Type: text/plain, Size: 782 bytes --]
I've attached a modified patch file that includes
all your suggested changes. I did some squashing
and editing, so this patch has the same three parts
as before, with properly formatted Change Logs in
each.
Let me know if you think anything else is needed.
Thanks, Chris
On Tue, Nov 29, 2016 at 6:49 PM, Tino Calancha <tino.calancha@gmail.com>
wrote:
> Christopher Genovese <genovese@cmu.edu> writes:
>
> > Do you want me to submit a new patch with these changes?
> It would be useful. Someone other than me may try this patch
> without reading all the bug report, and repeat some of my comments.
>
> Once we update the patch we can wait like 1 week; if we don't
> get further comments after that week, then it's OK for me to push
> the fix to master branch.
>
> Thank you
>
[-- Attachment #1.2: Type: text/html, Size: 1305 bytes --]
[-- Attachment #2: ibuffer-saved-filters-bug-clean.patch --]
[-- Type: text/x-diff, Size: 18130 bytes --]
From 53efb62b6bddb40b5bf460c2cc6bdc8b294b27e9 Mon Sep 17 00:00:00 2001
From: "Christopher R. Genovese" <genovese@cmu.edu>
Date: Sun, 27 Nov 2016 23:34:50 -0500
Subject: [PATCH 1/3] ibuffer: one possible fix for bug when saving existing
filter
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.
This commit takes the first choice.
Change Log:
2016-11-27 Christopher R. Genovese <genovese@cmu.edu>
* lisp/ibuf-ext.el (ibuffer-save-filters): Add extra
level of nesting when saving filter to an existing name.
---
lisp/ibuf-ext.el | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lisp/ibuf-ext.el b/lisp/ibuf-ext.el
index 5ef0746..103232c 100644
--- a/lisp/ibuf-ext.el
+++ b/lisp/ibuf-ext.el
@@ -935,7 +935,7 @@ Interactively, prompt for NAME, and use the current filters."
(read-from-minibuffer "Save current filters as: ")
ibuffer-filtering-qualifiers)))
(ibuffer-aif (assoc name ibuffer-saved-filters)
- (setcdr it filters)
+ (setcdr it (list filters))
(push (list name filters) ibuffer-saved-filters))
(ibuffer-maybe-save-stuff))
--
2.10.0
From dd57dffcec0030861c4fb95340fbc081d233e5c2 Mon Sep 17 00:00:00 2001
From: "Christopher R. Genovese" <genovese@cmu.edu>
Date: Mon, 28 Nov 2016 00:33:36 -0500
Subject: [PATCH 2/3] ibuffer: another fix for bug when saving an existing
filter
As described in the previous commit, the function 'ibuffer-save-filters'
handles saving existing filters and new filters inconsistently,
and there are two paths to fixing the problem.
The previous commit fixed the problem while maintaining the format of
the variable 'ibuffer-saved-filters' with its extra level of nesting.
This commit instead takes the second path: removing the extra level of
nesting from the saved filter format.
Change Log:
2016-11-27 Christopher R. Genovese <genovese@cmu.edu>
* lisp/ibuf-ext.el (ibuffer-saved-filters): Remove extra
nesting level in the alist values and updated docstring.
(ibuffer-save-filters): Remove extra level of nesting
in saved filter alist values when saving new filters.
(ibuffer-included-in-filter-p): Change access of saved
filter data (cadr->cdr) to account for reduced nesting.
(ibuffer-decompose-filter): Change access of saved
filter data (cadr->cdr) to account for reduced nesting.
---
lisp/ibuf-ext.el | 67 ++++++++++++++++++++++++++++----------------------------
1 file changed, 33 insertions(+), 34 deletions(-)
diff --git a/lisp/ibuf-ext.el b/lisp/ibuf-ext.el
index 103232c..e4a7dfa 100644
--- a/lisp/ibuf-ext.el
+++ b/lisp/ibuf-ext.el
@@ -120,28 +120,32 @@ Buffers whose major mode is in this list, are not searched."
(defvar ibuffer-auto-buffers-changed nil)
(defcustom 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)))))
-
- "An alist of filter qualifiers to switch between.
-
-This variable should look like ((\"STRING\" QUALIFIERS)
- (\"STRING\" QUALIFIERS) ...), where
-QUALIFIERS is a list of the same form as
-`ibuffer-filtering-qualifiers'.
-See also the variables `ibuffer-filtering-qualifiers',
-`ibuffer-filtering-alist', and the functions
-`ibuffer-switch-to-saved-filters', `ibuffer-save-filters'."
+ (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))))
+
+ "An alist mapping saved filter names to filter specifications.
+
+Each element should look like (\"NAME\" . FILTER-LIST), where
+FILTER-LIST has the same structure as the variable
+`ibuffer-filtering-qualifiers', which see. The filters defined
+here are joined with an implicit logical `and' and associated
+with NAME. The combined specification can be used by name in
+other filter specifications via the `saved' qualifier (again, see
+`ibuffer-filtering-qualifiers'). They can also be switched to by
+name (see the functions `ibuffer-switch-to-saved-filters' and
+`ibuffer-save-filters'). The variable `ibuffer-save-with-custom'
+affects how this information is saved for future sessions. This
+variable can be set directly from lisp code."
:type '(repeat sexp)
:group 'ibuffer)
@@ -535,13 +539,11 @@ To evaluate a form without viewing the buffer, see `ibuffer-do-eval'."
(ibuffer-included-in-filter-p buf x))
(cdr filter))))
(`saved
- (let ((data
- (assoc (cdr filter)
- ibuffer-saved-filters)))
+ (let ((data (assoc (cdr filter) ibuffer-saved-filters)))
(unless data
(ibuffer-filter-disable t)
(error "Unknown saved filter %s" (cdr filter)))
- (ibuffer-included-in-filters-p buf (cadr data))))
+ (ibuffer-included-in-filters-p buf (cdr data))))
(_
(pcase-let ((`(,_type ,_desc ,func)
(assq (car filter) ibuffer-filtering-alist)))
@@ -849,15 +851,12 @@ turned into two separate filters [name: foo] and [mode: bar-mode]."
(cdr lim)
ibuffer-filtering-qualifiers)))
(`saved
- (let ((data
- (assoc (cdr lim)
- ibuffer-saved-filters)))
+ (let ((data (assoc (cdr lim) ibuffer-saved-filters)))
(unless data
(ibuffer-filter-disable)
(error "Unknown saved filter %s" (cdr lim)))
- (setq ibuffer-filtering-qualifiers (append
- (cadr data)
- ibuffer-filtering-qualifiers))))
+ (setq ibuffer-filtering-qualifiers
+ (append (cdr data) ibuffer-filtering-qualifiers))))
(`not
(push (cdr lim)
ibuffer-filtering-qualifiers))
@@ -935,8 +934,8 @@ Interactively, prompt for NAME, and use the current filters."
(read-from-minibuffer "Save current filters as: ")
ibuffer-filtering-qualifiers)))
(ibuffer-aif (assoc name ibuffer-saved-filters)
- (setcdr it (list filters))
- (push (list name filters) ibuffer-saved-filters))
+ (setcdr it filters)
+ (push (cons name filters) ibuffer-saved-filters))
(ibuffer-maybe-save-stuff))
;;;###autoload
--
2.10.0
From 2d8b0044deaf0570edbb97fd63519857696fde07 Mon Sep 17 00:00:00 2001
From: "Christopher R. Genovese" <genovese@cmu.edu>
Date: Mon, 28 Nov 2016 01:29:04 -0500
Subject: [PATCH 3/3] ibuffer: add support for saved filter format change and
test
As described in the previous commit, one fix to the
inconsistency in 'ibuffer-save-filters' involves simplifying
the format of 'ibuffer-saved-filters' to reduce the extra
nesting level. This adds some support for this transition,
including a customize setter to transparently update old
format values and a command to check and repair the saved
values if desired.
This also adds a test of 'ibuffer-save-filter'.
Change Log:
2016-11-27 Christopher R. Genovese <genovese@cmu.edu>
* lisp/ibuf-ext.el (ibuffer-saved-filters): Add more accurate
customization type and transparent setter to adjust old-format
values.
(ibuffer-update-saved-filters-format): Update old-format
for saved buffer data to new format with reduced nesting level.
(ibuffer-repair-saved-filters): Add new command to check and
repair saved filters format.
(ibuffer-old-saved-filters-warning): Add new variable with
clickable message with repair options to be displayed
as a warning if 'ibuffer-repair-saved-filters' detects
a format mismatch.
* test/lisp/ibuffer-tests.el (ibuffer-save-filters): Add
a test that filters are saved in the proper format.
---
lisp/ibuf-ext.el | 95 +++++++++++++++++++++++++++++++++++++++++++++-
test/lisp/ibuffer-tests.el | 29 ++++++++++++++
2 files changed, 122 insertions(+), 2 deletions(-)
diff --git a/lisp/ibuf-ext.el b/lisp/ibuf-ext.el
index e4a7dfa..3fa47c0 100644
--- a/lisp/ibuf-ext.el
+++ b/lisp/ibuf-ext.el
@@ -35,7 +35,8 @@
(eval-when-compile
(require 'ibuf-macs)
- (require 'cl-lib))
+ (require 'cl-lib)
+ (require 'subr-x))
;;; Utility functions
(defun ibuffer-delete-alist (key alist)
@@ -119,6 +120,26 @@ Buffers whose major mode is in this list, are not searched."
(defvar ibuffer-auto-buffers-changed nil)
+(defun ibuffer-update-saved-filters-format (filters)
+ "Transforms alist from old to new `ibuffer-saved-filters' format.
+
+Specifically, converts old-format alist with values of the
+form (STRING (FILTER-SPECS...)) to alist with values of the
+form (STRING FILTER-SPECS...), where each filter spec should be a
+cons cell with a symbol in the car. Any elements in the latter
+form are kept as is.
+
+Returns (OLD-FORMAT-DETECTED . UPDATED-SAVED-FILTERS-LIST)."
+ (when filters
+ (let* ((old-format-detected nil)
+ (fix-filter (lambda (filter-spec)
+ (if (symbolp (car (cadr filter-spec)))
+ filter-spec
+ (setq old-format-detected t) ; side-effect
+ (cons (car filter-spec) (cadr filter-spec)))))
+ (fixed (mapcar fix-filter filters)))
+ (cons old-format-detected fixed))))
+
(defcustom ibuffer-saved-filters '(("gnus"
(or (mode . message-mode)
(mode . mail-mode)
@@ -146,9 +167,79 @@ name (see the functions `ibuffer-switch-to-saved-filters' and
`ibuffer-save-filters'). The variable `ibuffer-save-with-custom'
affects how this information is saved for future sessions. This
variable can be set directly from lisp code."
- :type '(repeat sexp)
+ :version "26.1"
+ :type '(alist :key-type (string :tag "Filter name")
+ :value-type (repeat :tag "Filter specification" sexp))
+ :set (lambda (symbol value)
+ ;; Just set-default but update legacy old-style format
+ (set-default symbol (cdr (ibuffer-update-saved-filters-format value))))
:group 'ibuffer)
+(defvar ibuffer-old-saved-filters-warning
+ (concat "Deprecated format detected for variable `ibuffer-saved-filters'.
+
+The format has been repaired and the variable modified accordingly.
+You can save the current value through the customize system by
+either clicking or hitting return "
+ (make-text-button
+ "here" nil
+ 'face '(:weight bold :inherit button)
+ 'mouse-face '(:weight normal :background "gray50" :inherit button)
+ 'follow-link t
+ 'help-echo "Click or RET: save new value in customize"
+ 'action (lambda (b)
+ (if (not (fboundp 'customize-save-variable))
+ (message "Customize not available; value not saved")
+ (customize-save-variable 'ibuffer-saved-filters
+ ibuffer-saved-filters)
+ (message "Saved updated ibuffer-saved-filters."))))
+ ". See below for
+an explanation and alternative ways to save the repaired value.
+
+Explanation: For the list variable `ibuffer-saved-filters',
+elements of the form (STRING (FILTER-SPECS...)) are deprecated
+and should instead have the form (STRING FILTER-SPECS...), where
+each filter spec is a cons cell with a symbol in the car. See
+`ibuffer-saved-filters' for details. The repaired value fixes
+this format without changing the meaning of the saved filters.
+
+Alternative ways to save the repaired value:
+
+ 1. Do M-x customize-variable and entering `ibuffer-saved-filters'
+ when prompted.
+
+ 2. Set the updated value manually by copying the
+ following emacs-lisp form to your emacs init file.
+
+%s
+"))
+
+(defun ibuffer-repair-saved-filters ()
+ "Updates `ibuffer-saved-filters' to its new-style format, if needed.
+
+If this list has any elements of the old-style format, a
+deprecation warning is raised, with a button allowing persistent
+update. Any updated filters retain their meaning in the new
+format. See `ibuffer-update-saved-filters-format' and
+`ibuffer-saved-filters' for details of the old and new formats."
+ (interactive)
+ (when (and (boundp 'ibuffer-saved-filters) ibuffer-saved-filters)
+ (let ((fixed (ibuffer-update-saved-filters-format ibuffer-saved-filters)))
+ (prog1
+ (setq ibuffer-saved-filters (cdr fixed))
+ (when-let ((old-format-detected (car fixed))
+ (warning-buffer-name "*Warnings*"))
+ (with-current-buffer (get-buffer-create warning-buffer-name)
+ (help-mode))
+ (let ((warning-series t)
+ (updated-form
+ (with-output-to-string
+ (pp `(setq ibuffer-saved-filters ',ibuffer-saved-filters)))))
+ (display-warning
+ 'ibuffer
+ (format ibuffer-old-saved-filters-warning updated-form)
+ nil warning-buffer-name)))))))
+
(defvar ibuffer-filtering-qualifiers nil
"A list like (SYMBOL . QUALIFIER) which filters the current buffer list.
See also `ibuffer-filtering-alist'.")
diff --git a/test/lisp/ibuffer-tests.el b/test/lisp/ibuffer-tests.el
index 3a4def3..6d5187a 100644
--- a/test/lisp/ibuffer-tests.el
+++ b/test/lisp/ibuffer-tests.el
@@ -66,5 +66,34 @@
(mapc (lambda (buf) (when (buffer-live-p buf)
(kill-buffer buf))) (list buf1 buf2)))))
+(ert-deftest ibuffer-save-filters ()
+ "Tests that `ibuffer-save-filters' saves in the proper format."
+ (skip-unless (featurep 'ibuf-ext))
+ (let ((ibuffer-save-with-custom nil)
+ (ibuffer-saved-filters nil)
+ (test1 '((mode . org-mode)
+ (or (size-gt . 10000)
+ (and (not (starred-name))
+ (directory . "\<org\>")))))
+ (test2 '((or (mode . emacs-lisp-mode) (file-extension . "elc?")
+ (and (starred-name) (name . "elisp"))
+ (mode . lisp-interaction-mode))))
+ (test3 '((size-lt . 100) (derived-mode . prog-mode)
+ (or (filename . "scratch")
+ (filename . "bonz")
+ (filename . "temp")))))
+ (ibuffer-save-filters "test1" test1)
+ (should (equal (car ibuffer-saved-filters) (cons "test1" test1)))
+ (ibuffer-save-filters "test2" test2)
+ (should (equal (car ibuffer-saved-filters) (cons "test2" test2)))
+ (should (equal (cadr ibuffer-saved-filters) (cons "test1" test1)))
+ (ibuffer-save-filters "test3" test3)
+ (should (equal (car ibuffer-saved-filters) (cons "test3" test3)))
+ (should (equal (cadr ibuffer-saved-filters) (cons "test2" test2)))
+ (should (equal (car (cddr ibuffer-saved-filters)) (cons "test1" test1)))
+ (should (equal (cdr (assoc "test1" ibuffer-saved-filters)) test1))
+ (should (equal (cdr (assoc "test2" ibuffer-saved-filters)) test2))
+ (should (equal (cdr (assoc "test3" ibuffer-saved-filters)) test3))))
+
(provide 'ibuffer-tests)
;; ibuffer-tests.el ends here
--
2.10.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* bug#25049: ibuffer bug when saving existing filter, with patches
2016-11-30 5:14 ` Christopher Genovese
@ 2016-11-30 7:02 ` Tino Calancha
2016-11-30 14:07 ` npostavs
2016-12-07 10:56 ` Tino Calancha
0 siblings, 2 replies; 12+ messages in thread
From: Tino Calancha @ 2016-11-30 7:02 UTC (permalink / raw)
To: Christopher Genovese; +Cc: 25049, tino.calancha
Christopher Genovese <genovese@cmu.edu> writes:
> I've attached a modified patch file that includes
> all your suggested changes. I did some squashing
> and editing, so this patch has the same three parts
> as before, with properly formatted Change Logs in
> each.
Thank you very much fr your prompt replay!
I)
+ 'follow-link t
+ 'help-echo "Click or RET: save new value in customize"
+ 'action (lambda (b)
+ (if (not (fboundp 'customize-save-variable))
+ (message "Customize not available; value not saved")
+ (customize-save-variable 'ibuffer-saved-filters
+ ibuffer-saved-filters)
+ (message "Saved updated ibuffer-saved-filters."))))
The lambda form above doesn't use its 'b' argument, so i have dropped it.
II)
I see you define `ibuffer-update-saved-filters-format' right before
than `ibuffer-saved-filters', because this defcustom use that function
to set its default.
In the other hand `ibuffer-repair-saved-filters' isn't used by any
defcustom so i moved out after the variable definitions.
If we don't get further comments to address in 1 week, then let's
push this fix to the master branch.
Thank you
Tino
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#25049: ibuffer bug when saving existing filter, with patches
2016-11-30 7:02 ` Tino Calancha
@ 2016-11-30 14:07 ` npostavs
2016-11-30 15:03 ` Tino Calancha
2016-12-07 10:56 ` Tino Calancha
1 sibling, 1 reply; 12+ messages in thread
From: npostavs @ 2016-11-30 14:07 UTC (permalink / raw)
To: Tino Calancha; +Cc: Christopher Genovese, 25049
Tino Calancha <tino.calancha@gmail.com> writes:
> Christopher Genovese <genovese@cmu.edu> writes:
>
>> I've attached a modified patch file that includes
>> all your suggested changes. I did some squashing
>> and editing, so this patch has the same three parts
>> as before, with properly formatted Change Logs in
>> each.
You don't need the "Change Log: 2016-11-27 Christopher R. Genovese
<genovese@cmu.edu>" part, that information is extracted automatically
from the commit metadata.
> Thank you very much fr your prompt replay!
>
> I)
> + 'follow-link t
> + 'help-echo "Click or RET: save new value in customize"
> + 'action (lambda (b)
> + (if (not (fboundp 'customize-save-variable))
> + (message "Customize not available; value not saved")
> + (customize-save-variable 'ibuffer-saved-filters
> + ibuffer-saved-filters)
> + (message "Saved updated ibuffer-saved-filters."))))
> The lambda form above doesn't use its 'b' argument, so i have dropped
> it.
By "drop" I hope you meant "replaced it with `_'". The action function
receives 1 argument, so it has to accept it.
>
> If we don't get further comments to address in 1 week, then let's
> push this fix to the master branch.
I don't really agree with this switching *Warnings* to help-mode.
First, it's out of place for a particular warning to start manipulating
the *Warnings* buffer like that. And second, it would make more sense
to have a warnings-mode, that could provide more specialized bindings
(e.g., ignore warning at point). But that's a subject for another
thread. I don't think this patch should do anything about it.
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#25049: ibuffer bug when saving existing filter, with patches
2016-11-30 14:07 ` npostavs
@ 2016-11-30 15:03 ` Tino Calancha
2016-11-30 15:11 ` Tino Calancha
0 siblings, 1 reply; 12+ messages in thread
From: Tino Calancha @ 2016-11-30 15:03 UTC (permalink / raw)
To: npostavs; +Cc: Christopher Genovese, 25049, Tino Calancha
npostavs@users.sourceforge.net writes:
Hi Noa,
thanks for your time reading the report and for
bringing those points!
> Tino Calancha <tino.calancha@gmail.com> writes:
>
>> Christopher Genovese <genovese@cmu.edu> writes:
>>
>>> I've attached a modified patch file that includes
>>> all your suggested changes. I did some squashing
>>> and editing, so this patch has the same three parts
>>> as before, with properly formatted Change Logs in
>>> each.
>
> You don't need the "Change Log: 2016-11-27 Christopher R. Genovese
> <genovese@cmu.edu>" part, that information is extracted automatically
> from the commit metadata.
See the below patch. It doesn't include ChangeLog in the commit message.
>> Thank you very much fr your prompt replay!
>>
>> I)
>> + 'follow-link t
>> + 'help-echo "Click or RET: save new value in customize"
>> + 'action (lambda (b)
>> + (if (not (fboundp 'customize-save-variable))
>> + (message "Customize not available; value not saved")
>> + (customize-save-variable 'ibuffer-saved-filters
>> + ibuffer-saved-filters)
>> + (message "Saved updated ibuffer-saved-filters."))))
>> The lambda form above doesn't use its 'b' argument, so i have dropped
>> it.
>
> By "drop" I hope you meant "replaced it with `_'". The action function
> receives 1 argument, so it has to accept it
Thank you, i didn't know that. I updated to
'action (lambda (_b)
> I don't really agree with this switching *Warnings* to help-mode.
> First, it's out of place for a particular warning to start manipulating
> the *Warnings* buffer like that. And second, it would make more sense
> to have a warnings-mode, that could provide more specialized bindings
> (e.g., ignore warning at point). But that's a subject for another
> thread. I don't think this patch should do anything about it.
OK. I switched back to the OP version, i.e., without `help-mode' in the
buffer displaying the warning.
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
From b01ef57f393d24e3ed81630b8dfa69e19a9e6f98 Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha@gmail.com>
Date: Wed, 30 Nov 2016 23:49:53 +0900
Subject: [PATCH] ibuffer-saved-filters: Remove extra nesting level
* lisp/ibuf-ext.el (ibuffer-saved-filters): Remove extra
nesting level; add transparent setter to adjust old-format values;
update doc string.
(ibuffer-save-filters): Remove extra level of nesting
in ibuffer-saved-filters values when saving new filters (Bug#25049).
(ibuffer-old-saved-filters-warning): New variable with
clickable message with repair options to be displayed
as a warning if 'ibuffer-repair-saved-filters' detects
a format mismatch.
(ibuffer-repair-saved-filters): Add new command to check and
repair saved filters format.
(ibuffer-included-in-filter-p, ibuffer-decompose-filter):
Change access of saved filter data (cadr->cdr) to account
for reduced nesting.
* test/lisp/ibuffer-tests.el (ibuffer-save-filters):
New test; check that filters are saved in the proper format.
---
lisp/ibuf-ext.el | 156 +++++++++++++++++++++++++++++++++++----------
test/lisp/ibuffer-tests.el | 29 +++++++++
2 files changed, 150 insertions(+), 35 deletions(-)
diff --git a/lisp/ibuf-ext.el b/lisp/ibuf-ext.el
index 5ef0746..dbaafa6 100644
--- a/lisp/ibuf-ext.el
+++ b/lisp/ibuf-ext.el
@@ -35,7 +35,8 @@
(eval-when-compile
(require 'ibuf-macs)
- (require 'cl-lib))
+ (require 'cl-lib)
+ (require 'subr-x))
;;; Utility functions
(defun ibuffer-delete-alist (key alist)
@@ -119,32 +120,100 @@ ibuffer-tmp-show-regexps
(defvar ibuffer-auto-buffers-changed nil)
-(defcustom 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)))))
-
- "An alist of filter qualifiers to switch between.
+(defun ibuffer-update-saved-filters-format (filters)
+ "Transforms alist from old to new `ibuffer-saved-filters' format.
+
+Specifically, converts old-format alist with values of the
+form (STRING (FILTER-SPECS...)) to alist with values of the
+form (STRING FILTER-SPECS...), where each filter spec should be a
+cons cell with a symbol in the car. Any elements in the latter
+form are kept as is.
+
+Returns (OLD-FORMAT-DETECTED . UPDATED-SAVED-FILTERS-LIST)."
+ (when filters
+ (let* ((old-format-detected nil)
+ (fix-filter (lambda (filter-spec)
+ (if (symbolp (car (cadr filter-spec)))
+ filter-spec
+ (setq old-format-detected t) ; side-effect
+ (cons (car filter-spec) (cadr filter-spec)))))
+ (fixed (mapcar fix-filter filters)))
+ (cons old-format-detected fixed))))
-This variable should look like ((\"STRING\" QUALIFIERS)
- (\"STRING\" QUALIFIERS) ...), where
-QUALIFIERS is a list of the same form as
-`ibuffer-filtering-qualifiers'.
-See also the variables `ibuffer-filtering-qualifiers',
-`ibuffer-filtering-alist', and the functions
-`ibuffer-switch-to-saved-filters', `ibuffer-save-filters'."
- :type '(repeat sexp)
+(defcustom 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))))
+
+ "An alist mapping saved filter names to filter specifications.
+
+Each element should look like (\"NAME\" . FILTER-LIST), where
+FILTER-LIST has the same structure as the variable
+`ibuffer-filtering-qualifiers', which see. The filters defined
+here are joined with an implicit logical `and' and associated
+with NAME. The combined specification can be used by name in
+other filter specifications via the `saved' qualifier (again, see
+`ibuffer-filtering-qualifiers'). They can also be switched to by
+name (see the functions `ibuffer-switch-to-saved-filters' and
+`ibuffer-save-filters'). The variable `ibuffer-save-with-custom'
+affects how this information is saved for future sessions. This
+variable can be set directly from lisp code."
+ :version "26.1"
+ :type '(alist :key-type (string :tag "Filter name")
+ :value-type (repeat :tag "Filter specification" sexp))
+ :set (lambda (symbol value)
+ ;; Just set-default but update legacy old-style format
+ (set-default symbol (cdr (ibuffer-update-saved-filters-format value))))
:group 'ibuffer)
+(defvar ibuffer-old-saved-filters-warning
+ (concat "Deprecated format detected for variable `ibuffer-saved-filters'.
+
+The format has been repaired and the variable modified accordingly.
+You can save the current value through the customize system by
+either clicking or hitting return "
+ (make-text-button
+ "here" nil
+ 'face '(:weight bold :inherit button)
+ 'mouse-face '(:weight normal :background "gray50" :inherit button)
+ 'follow-link t
+ 'help-echo "Click or RET: save new value in customize"
+ 'action (lambda (_b)
+ (if (not (fboundp 'customize-save-variable))
+ (message "Customize not available; value not saved")
+ (customize-save-variable 'ibuffer-saved-filters
+ ibuffer-saved-filters)
+ (message "Saved updated ibuffer-saved-filters."))))
+ ". See below for
+an explanation and alternative ways to save the repaired value.
+
+Explanation: For the list variable `ibuffer-saved-filters',
+elements of the form (STRING (FILTER-SPECS...)) are deprecated
+and should instead have the form (STRING FILTER-SPECS...), where
+each filter spec is a cons cell with a symbol in the car. See
+`ibuffer-saved-filters' for details. The repaired value fixes
+this format without changing the meaning of the saved filters.
+
+Alternative ways to save the repaired value:
+
+ 1. Do M-x customize-variable and entering `ibuffer-saved-filters'
+ when prompted.
+
+ 2. Set the updated value manually by copying the
+ following emacs-lisp form to your emacs init file.
+
+%s
+"))
+
(defvar ibuffer-filtering-qualifiers nil
"A list like (SYMBOL . QUALIFIER) which filters the current buffer list.
See also `ibuffer-filtering-alist'.")
@@ -224,6 +293,28 @@ ibuffer-save-with-custom
:type 'boolean
:group 'ibuffer)
+(defun ibuffer-repair-saved-filters ()
+ "Updates `ibuffer-saved-filters' to its new-style format, if needed.
+
+If this list has any elements of the old-style format, a
+deprecation warning is raised, with a button allowing persistent
+update. Any updated filters retain their meaning in the new
+format. See `ibuffer-update-saved-filters-format' and
+`ibuffer-saved-filters' for details of the old and new formats."
+ (interactive)
+ (when (and (boundp 'ibuffer-saved-filters) ibuffer-saved-filters)
+ (let ((fixed (ibuffer-update-saved-filters-format ibuffer-saved-filters)))
+ (prog1
+ (setq ibuffer-saved-filters (cdr fixed))
+ (when-let (old-format-detected? (car fixed))
+ (let ((warning-series t)
+ (updated-form
+ (with-output-to-string
+ (pp `(setq ibuffer-saved-filters ',ibuffer-saved-filters)))))
+ (display-warning
+ 'ibuffer
+ (format ibuffer-old-saved-filters-warning updated-form))))))))
+
(defun ibuffer-ext-visible-p (buf all &optional ibuffer-buf)
(or
(ibuffer-buf-matches-predicates buf ibuffer-tmp-show-regexps)
@@ -535,13 +626,11 @@ ibuffer-included-in-filter-p-1
(ibuffer-included-in-filter-p buf x))
(cdr filter))))
(`saved
- (let ((data
- (assoc (cdr filter)
- ibuffer-saved-filters)))
+ (let ((data (assoc (cdr filter) ibuffer-saved-filters)))
(unless data
(ibuffer-filter-disable t)
(error "Unknown saved filter %s" (cdr filter)))
- (ibuffer-included-in-filters-p buf (cadr data))))
+ (ibuffer-included-in-filters-p buf (cadr data))))
(_
(pcase-let ((`(,_type ,_desc ,func)
(assq (car filter) ibuffer-filtering-alist)))
@@ -849,15 +938,12 @@ ibuffer-decompose-filter
(cdr lim)
ibuffer-filtering-qualifiers)))
(`saved
- (let ((data
- (assoc (cdr lim)
- ibuffer-saved-filters)))
+ (let ((data (assoc (cdr lim) ibuffer-saved-filters)))
(unless data
(ibuffer-filter-disable)
(error "Unknown saved filter %s" (cdr lim)))
- (setq ibuffer-filtering-qualifiers (append
- (cadr data)
- ibuffer-filtering-qualifiers))))
+ (setq ibuffer-filtering-qualifiers
+ (append (cdr data) ibuffer-filtering-qualifiers))))
(`not
(push (cdr lim)
ibuffer-filtering-qualifiers))
@@ -936,7 +1022,7 @@ ibuffer-save-filters
ibuffer-filtering-qualifiers)))
(ibuffer-aif (assoc name ibuffer-saved-filters)
(setcdr it filters)
- (push (list name filters) ibuffer-saved-filters))
+ (push (cons name filters) ibuffer-saved-filters))
(ibuffer-maybe-save-stuff))
;;;###autoload
diff --git a/test/lisp/ibuffer-tests.el b/test/lisp/ibuffer-tests.el
index 3a4def3..6d5187a 100644
--- a/test/lisp/ibuffer-tests.el
+++ b/test/lisp/ibuffer-tests.el
@@ -66,5 +66,34 @@
(mapc (lambda (buf) (when (buffer-live-p buf)
(kill-buffer buf))) (list buf1 buf2)))))
+(ert-deftest ibuffer-save-filters ()
+ "Tests that `ibuffer-save-filters' saves in the proper format."
+ (skip-unless (featurep 'ibuf-ext))
+ (let ((ibuffer-save-with-custom nil)
+ (ibuffer-saved-filters nil)
+ (test1 '((mode . org-mode)
+ (or (size-gt . 10000)
+ (and (not (starred-name))
+ (directory . "\<org\>")))))
+ (test2 '((or (mode . emacs-lisp-mode) (file-extension . "elc?")
+ (and (starred-name) (name . "elisp"))
+ (mode . lisp-interaction-mode))))
+ (test3 '((size-lt . 100) (derived-mode . prog-mode)
+ (or (filename . "scratch")
+ (filename . "bonz")
+ (filename . "temp")))))
+ (ibuffer-save-filters "test1" test1)
+ (should (equal (car ibuffer-saved-filters) (cons "test1" test1)))
+ (ibuffer-save-filters "test2" test2)
+ (should (equal (car ibuffer-saved-filters) (cons "test2" test2)))
+ (should (equal (cadr ibuffer-saved-filters) (cons "test1" test1)))
+ (ibuffer-save-filters "test3" test3)
+ (should (equal (car ibuffer-saved-filters) (cons "test3" test3)))
+ (should (equal (cadr ibuffer-saved-filters) (cons "test2" test2)))
+ (should (equal (car (cddr ibuffer-saved-filters)) (cons "test1" test1)))
+ (should (equal (cdr (assoc "test1" ibuffer-saved-filters)) test1))
+ (should (equal (cdr (assoc "test2" ibuffer-saved-filters)) test2))
+ (should (equal (cdr (assoc "test3" ibuffer-saved-filters)) test3))))
+
(provide 'ibuffer-tests)
;; ibuffer-tests.el ends here
--
2.10.2
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
In GNU Emacs 26.0.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.22.4)
of 2016-11-30
Repository revision: a283d655db88cdcc8cb53d8e2578e1cdf751c84b
^ permalink raw reply related [flat|nested] 12+ messages in thread
* bug#25049: ibuffer bug when saving existing filter, with patches
2016-11-30 15:03 ` Tino Calancha
@ 2016-11-30 15:11 ` Tino Calancha
2016-11-30 16:31 ` Christopher Genovese
0 siblings, 1 reply; 12+ messages in thread
From: Tino Calancha @ 2016-11-30 15:11 UTC (permalink / raw)
To: Tino Calancha; +Cc: Christopher Genovese, 25049, npostavs
On Wed, 30 Nov 2016, Tino Calancha wrote:
>From: Tino Calancha <tino.calancha@gmail.com>
>Date: Wed, 30 Nov 2016 23:49:53 +0900
>Subject: [PATCH] ibuffer-saved-filters: Remove extra nesting level
Of course the author of the patch is Chris, so the previous
should be read:
From: Christopher Genovese <genovese@cmu.edu>
Date: Wed, 30 Nov 2016 23:49:53 +0900
Subject: [PATCH] ibuffer-saved-filters: Remove extra nesting level
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#25049: ibuffer bug when saving existing filter, with patches
2016-11-30 15:11 ` Tino Calancha
@ 2016-11-30 16:31 ` Christopher Genovese
2016-12-01 2:52 ` Tino Calancha
0 siblings, 1 reply; 12+ messages in thread
From: Christopher Genovese @ 2016-11-30 16:31 UTC (permalink / raw)
To: Tino Calancha; +Cc: 25049, npostavs
[-- Attachment #1.1: Type: text/plain, Size: 1024 bytes --]
Thanks, Noa and Tino.
Tino, I noticed that in the patch you just sent in email the '?' on
old-format-detected was still there. The rest looked fine
on first glance, but you might want to edit that.
Alternatively, just in case it helps, I've attached a patch like the others
that drops the help-mode change, changes the b argument to _ to indicate
unused, and drops the names and dates from before the Change Logs -- while
keeping Tino's other suggested changes intact.
Best, Chris
On Wed, Nov 30, 2016 at 10:11 AM, Tino Calancha <tino.calancha@gmail.com>
wrote:
> On Wed, 30 Nov 2016, Tino Calancha wrote:
>
>> From: Tino Calancha <tino.calancha@gmail.com>
>> Date: Wed, 30 Nov 2016 23:49:53 +0900
>> Subject: [PATCH] ibuffer-saved-filters: Remove extra nesting level
>>
> Of course the author of the patch is Chris, so the previous
> should be read:
>
> From: Christopher Genovese <genovese@cmu.edu>
>
> Date: Wed, 30 Nov 2016 23:49:53 +0900
> Subject: [PATCH] ibuffer-saved-filters: Remove extra nesting level
>
>
[-- Attachment #1.2: Type: text/html, Size: 1831 bytes --]
[-- Attachment #2: ibuffer-saved-filters-bug-cleaner.patch --]
[-- Type: text/x-diff, Size: 18022 bytes --]
From ee8b2dc749455a96c3880cf2b29a95678159ffee Mon Sep 17 00:00:00 2001
From: "Christopher R. Genovese" <genovese@cmu.edu>
Date: Sun, 27 Nov 2016 23:34:50 -0500
Subject: [PATCH 1/3] ibuffer: one possible fix for bug when saving existing
filter
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.
This commit takes the first choice.
Change Log:
* lisp/ibuf-ext.el (ibuffer-save-filters): Add extra
level of nesting when saving filter to an existing name.
---
lisp/ibuf-ext.el | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lisp/ibuf-ext.el b/lisp/ibuf-ext.el
index 5ef0746..103232c 100644
--- a/lisp/ibuf-ext.el
+++ b/lisp/ibuf-ext.el
@@ -935,7 +935,7 @@ Interactively, prompt for NAME, and use the current filters."
(read-from-minibuffer "Save current filters as: ")
ibuffer-filtering-qualifiers)))
(ibuffer-aif (assoc name ibuffer-saved-filters)
- (setcdr it filters)
+ (setcdr it (list filters))
(push (list name filters) ibuffer-saved-filters))
(ibuffer-maybe-save-stuff))
--
2.10.0
From 06a94739c0ef00966e9f7105fd3b28d524cc36bf Mon Sep 17 00:00:00 2001
From: "Christopher R. Genovese" <genovese@cmu.edu>
Date: Mon, 28 Nov 2016 00:33:36 -0500
Subject: [PATCH 2/3] ibuffer: another fix for bug when saving an existing
filter
As described in the previous commit, the function 'ibuffer-save-filters'
handles saving existing filters and new filters inconsistently,
and there are two paths to fixing the problem.
The previous commit fixed the problem while maintaining the format of
the variable 'ibuffer-saved-filters' with its extra level of nesting.
This commit instead takes the second path: removing the extra level of
nesting from the saved filter format.
Change Log:
* lisp/ibuf-ext.el (ibuffer-saved-filters): Remove extra
nesting level in the alist values and updated docstring.
(ibuffer-save-filters): Remove extra level of nesting
in saved filter alist values when saving new filters.
(ibuffer-included-in-filter-p): Change access of saved
filter data (cadr->cdr) to account for reduced nesting.
(ibuffer-decompose-filter): Change access of saved
filter data (cadr->cdr) to account for reduced nesting.
---
lisp/ibuf-ext.el | 67 ++++++++++++++++++++++++++++----------------------------
1 file changed, 33 insertions(+), 34 deletions(-)
diff --git a/lisp/ibuf-ext.el b/lisp/ibuf-ext.el
index 103232c..e4a7dfa 100644
--- a/lisp/ibuf-ext.el
+++ b/lisp/ibuf-ext.el
@@ -120,28 +120,32 @@ Buffers whose major mode is in this list, are not searched."
(defvar ibuffer-auto-buffers-changed nil)
(defcustom 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)))))
-
- "An alist of filter qualifiers to switch between.
-
-This variable should look like ((\"STRING\" QUALIFIERS)
- (\"STRING\" QUALIFIERS) ...), where
-QUALIFIERS is a list of the same form as
-`ibuffer-filtering-qualifiers'.
-See also the variables `ibuffer-filtering-qualifiers',
-`ibuffer-filtering-alist', and the functions
-`ibuffer-switch-to-saved-filters', `ibuffer-save-filters'."
+ (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))))
+
+ "An alist mapping saved filter names to filter specifications.
+
+Each element should look like (\"NAME\" . FILTER-LIST), where
+FILTER-LIST has the same structure as the variable
+`ibuffer-filtering-qualifiers', which see. The filters defined
+here are joined with an implicit logical `and' and associated
+with NAME. The combined specification can be used by name in
+other filter specifications via the `saved' qualifier (again, see
+`ibuffer-filtering-qualifiers'). They can also be switched to by
+name (see the functions `ibuffer-switch-to-saved-filters' and
+`ibuffer-save-filters'). The variable `ibuffer-save-with-custom'
+affects how this information is saved for future sessions. This
+variable can be set directly from lisp code."
:type '(repeat sexp)
:group 'ibuffer)
@@ -535,13 +539,11 @@ To evaluate a form without viewing the buffer, see `ibuffer-do-eval'."
(ibuffer-included-in-filter-p buf x))
(cdr filter))))
(`saved
- (let ((data
- (assoc (cdr filter)
- ibuffer-saved-filters)))
+ (let ((data (assoc (cdr filter) ibuffer-saved-filters)))
(unless data
(ibuffer-filter-disable t)
(error "Unknown saved filter %s" (cdr filter)))
- (ibuffer-included-in-filters-p buf (cadr data))))
+ (ibuffer-included-in-filters-p buf (cdr data))))
(_
(pcase-let ((`(,_type ,_desc ,func)
(assq (car filter) ibuffer-filtering-alist)))
@@ -849,15 +851,12 @@ turned into two separate filters [name: foo] and [mode: bar-mode]."
(cdr lim)
ibuffer-filtering-qualifiers)))
(`saved
- (let ((data
- (assoc (cdr lim)
- ibuffer-saved-filters)))
+ (let ((data (assoc (cdr lim) ibuffer-saved-filters)))
(unless data
(ibuffer-filter-disable)
(error "Unknown saved filter %s" (cdr lim)))
- (setq ibuffer-filtering-qualifiers (append
- (cadr data)
- ibuffer-filtering-qualifiers))))
+ (setq ibuffer-filtering-qualifiers
+ (append (cdr data) ibuffer-filtering-qualifiers))))
(`not
(push (cdr lim)
ibuffer-filtering-qualifiers))
@@ -935,8 +934,8 @@ Interactively, prompt for NAME, and use the current filters."
(read-from-minibuffer "Save current filters as: ")
ibuffer-filtering-qualifiers)))
(ibuffer-aif (assoc name ibuffer-saved-filters)
- (setcdr it (list filters))
- (push (list name filters) ibuffer-saved-filters))
+ (setcdr it filters)
+ (push (cons name filters) ibuffer-saved-filters))
(ibuffer-maybe-save-stuff))
;;;###autoload
--
2.10.0
From 627444d059d845b91ccc4200eaf118918e758624 Mon Sep 17 00:00:00 2001
From: "Christopher R. Genovese" <genovese@cmu.edu>
Date: Mon, 28 Nov 2016 01:29:04 -0500
Subject: [PATCH 3/3] ibuffer: add support for saved filter format change and
test
As described in the previous commit, one fix to the
inconsistency in 'ibuffer-save-filters' involves simplifying
the format of 'ibuffer-saved-filters' to reduce the extra
nesting level. This adds some support for this transition,
including a customize setter to transparently update old
format values and a command to check and repair the saved
values if desired.
Also added a test of 'ibuffer-save-filter'.
Change Log:
* lisp/ibuf-ext.el (ibuffer-saved-filters): Add more accurate
customization type and transparent setter to adjust old-format
values.
(ibuffer-update-saved-filters-format): Update old-format
for saved buffer data to new format with reduced nesting level.
(ibuffer-repair-saved-filters): Add new command to check and
repair saved filters format.
(ibuffer-old-saved-filters-warning): Add new variable with
clickable message with repair options to be displayed
as a warning if 'ibuffer-repair-saved-filters' detects
a format mismatch.
* test/lisp/ibuffer-tests.el (ibuffer-save-filters): Add
a test that filters are saved in the proper format.
---
lisp/ibuf-ext.el | 91 +++++++++++++++++++++++++++++++++++++++++++++-
test/lisp/ibuffer-tests.el | 29 +++++++++++++++
2 files changed, 118 insertions(+), 2 deletions(-)
diff --git a/lisp/ibuf-ext.el b/lisp/ibuf-ext.el
index e4a7dfa..d1bf576 100644
--- a/lisp/ibuf-ext.el
+++ b/lisp/ibuf-ext.el
@@ -35,7 +35,8 @@
(eval-when-compile
(require 'ibuf-macs)
- (require 'cl-lib))
+ (require 'cl-lib)
+ (require 'subr-x))
;;; Utility functions
(defun ibuffer-delete-alist (key alist)
@@ -119,6 +120,26 @@ Buffers whose major mode is in this list, are not searched."
(defvar ibuffer-auto-buffers-changed nil)
+(defun ibuffer-update-saved-filters-format (filters)
+ "Transforms alist from old to new `ibuffer-saved-filters' format.
+
+Specifically, converts old-format alist with values of the
+form (STRING (FILTER-SPECS...)) to alist with values of the
+form (STRING FILTER-SPECS...), where each filter spec should be a
+cons cell with a symbol in the car. Any elements in the latter
+form are kept as is.
+
+Returns (OLD-FORMAT-DETECTED . UPDATED-SAVED-FILTERS-LIST)."
+ (when filters
+ (let* ((old-format-detected nil)
+ (fix-filter (lambda (filter-spec)
+ (if (symbolp (car (cadr filter-spec)))
+ filter-spec
+ (setq old-format-detected t) ; side-effect
+ (cons (car filter-spec) (cadr filter-spec)))))
+ (fixed (mapcar fix-filter filters)))
+ (cons old-format-detected fixed))))
+
(defcustom ibuffer-saved-filters '(("gnus"
(or (mode . message-mode)
(mode . mail-mode)
@@ -146,9 +167,53 @@ name (see the functions `ibuffer-switch-to-saved-filters' and
`ibuffer-save-filters'). The variable `ibuffer-save-with-custom'
affects how this information is saved for future sessions. This
variable can be set directly from lisp code."
- :type '(repeat sexp)
+ :version "26.1"
+ :type '(alist :key-type (string :tag "Filter name")
+ :value-type (repeat :tag "Filter specification" sexp))
+ :set (lambda (symbol value)
+ ;; Just set-default but update legacy old-style format
+ (set-default symbol (cdr (ibuffer-update-saved-filters-format value))))
:group 'ibuffer)
+(defvar ibuffer-old-saved-filters-warning
+ (concat "Deprecated format detected for variable `ibuffer-saved-filters'.
+
+The format has been repaired and the variable modified accordingly.
+You can save the current value through the customize system by
+either clicking or hitting return "
+ (make-text-button
+ "here" nil
+ 'face '(:weight bold :inherit button)
+ 'mouse-face '(:weight normal :background "gray50" :inherit button)
+ 'follow-link t
+ 'help-echo "Click or RET: save new value in customize"
+ 'action (lambda (_)
+ (if (not (fboundp 'customize-save-variable))
+ (message "Customize not available; value not saved")
+ (customize-save-variable 'ibuffer-saved-filters
+ ibuffer-saved-filters)
+ (message "Saved updated ibuffer-saved-filters."))))
+ ". See below for
+an explanation and alternative ways to save the repaired value.
+
+Explanation: For the list variable `ibuffer-saved-filters',
+elements of the form (STRING (FILTER-SPECS...)) are deprecated
+and should instead have the form (STRING FILTER-SPECS...), where
+each filter spec is a cons cell with a symbol in the car. See
+`ibuffer-saved-filters' for details. The repaired value fixes
+this format without changing the meaning of the saved filters.
+
+Alternative ways to save the repaired value:
+
+ 1. Do M-x customize-variable and entering `ibuffer-saved-filters'
+ when prompted.
+
+ 2. Set the updated value manually by copying the
+ following emacs-lisp form to your emacs init file.
+
+%s
+"))
+
(defvar ibuffer-filtering-qualifiers nil
"A list like (SYMBOL . QUALIFIER) which filters the current buffer list.
See also `ibuffer-filtering-alist'.")
@@ -228,6 +293,28 @@ Currently, this only applies to `ibuffer-saved-filters' and
:type 'boolean
:group 'ibuffer)
+(defun ibuffer-repair-saved-filters ()
+ "Updates `ibuffer-saved-filters' to its new-style format, if needed.
+
+If this list has any elements of the old-style format, a
+deprecation warning is raised, with a button allowing persistent
+update. Any updated filters retain their meaning in the new
+format. See `ibuffer-update-saved-filters-format' and
+`ibuffer-saved-filters' for details of the old and new formats."
+ (interactive)
+ (when (and (boundp 'ibuffer-saved-filters) ibuffer-saved-filters)
+ (let ((fixed (ibuffer-update-saved-filters-format ibuffer-saved-filters)))
+ (prog1
+ (setq ibuffer-saved-filters (cdr fixed))
+ (when-let (old-format-detected (car fixed))
+ (let ((warning-series t)
+ (updated-form
+ (with-output-to-string
+ (pp `(setq ibuffer-saved-filters ',ibuffer-saved-filters)))))
+ (display-warning
+ 'ibuffer
+ (format ibuffer-old-saved-filters-warning updated-form))))))))
+
(defun ibuffer-ext-visible-p (buf all &optional ibuffer-buf)
(or
(ibuffer-buf-matches-predicates buf ibuffer-tmp-show-regexps)
diff --git a/test/lisp/ibuffer-tests.el b/test/lisp/ibuffer-tests.el
index 3a4def3..6d5187a 100644
--- a/test/lisp/ibuffer-tests.el
+++ b/test/lisp/ibuffer-tests.el
@@ -66,5 +66,34 @@
(mapc (lambda (buf) (when (buffer-live-p buf)
(kill-buffer buf))) (list buf1 buf2)))))
+(ert-deftest ibuffer-save-filters ()
+ "Tests that `ibuffer-save-filters' saves in the proper format."
+ (skip-unless (featurep 'ibuf-ext))
+ (let ((ibuffer-save-with-custom nil)
+ (ibuffer-saved-filters nil)
+ (test1 '((mode . org-mode)
+ (or (size-gt . 10000)
+ (and (not (starred-name))
+ (directory . "\<org\>")))))
+ (test2 '((or (mode . emacs-lisp-mode) (file-extension . "elc?")
+ (and (starred-name) (name . "elisp"))
+ (mode . lisp-interaction-mode))))
+ (test3 '((size-lt . 100) (derived-mode . prog-mode)
+ (or (filename . "scratch")
+ (filename . "bonz")
+ (filename . "temp")))))
+ (ibuffer-save-filters "test1" test1)
+ (should (equal (car ibuffer-saved-filters) (cons "test1" test1)))
+ (ibuffer-save-filters "test2" test2)
+ (should (equal (car ibuffer-saved-filters) (cons "test2" test2)))
+ (should (equal (cadr ibuffer-saved-filters) (cons "test1" test1)))
+ (ibuffer-save-filters "test3" test3)
+ (should (equal (car ibuffer-saved-filters) (cons "test3" test3)))
+ (should (equal (cadr ibuffer-saved-filters) (cons "test2" test2)))
+ (should (equal (car (cddr ibuffer-saved-filters)) (cons "test1" test1)))
+ (should (equal (cdr (assoc "test1" ibuffer-saved-filters)) test1))
+ (should (equal (cdr (assoc "test2" ibuffer-saved-filters)) test2))
+ (should (equal (cdr (assoc "test3" ibuffer-saved-filters)) test3))))
+
(provide 'ibuffer-tests)
;; ibuffer-tests.el ends here
--
2.10.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* bug#25049: ibuffer bug when saving existing filter, with patches
2016-11-30 16:31 ` Christopher Genovese
@ 2016-12-01 2:52 ` Tino Calancha
0 siblings, 0 replies; 12+ messages in thread
From: Tino Calancha @ 2016-12-01 2:52 UTC (permalink / raw)
To: Christopher Genovese; +Cc: npostavs, 25049, Tino Calancha
Christopher Genovese <genovese@cmu.edu> writes:
> Thanks, Noa and Tino.
>
> Tino, I noticed that in the patch you just sent in email the '?' on
> old-format-detected was still there. The rest looked fine
> on first glance, but you might want to edit that.
>
> Alternatively, just in case it helps, I've attached a patch like the others
> that drops the help-mode change, changes the b argument to _ to indicate
> unused, and drops the names and dates from before the Change Logs -- while
> keeping Tino's other suggested changes intact.
Thank you very much.
I have updated the target patch with your latest changes:
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
From b63c1ab39ad8d1d4edb290707373d769c08b55a9 Mon Sep 17 00:00:00 2001
From: Christopher Genovese <genovese@cmu.edu>
Date: Thu, 1 Dec 2016 11:43:20 +0900
Subject: [PATCH] ibuffer-saved-filters: Remove extra nesting level
* lisp/ibuf-ext.el (ibuffer-saved-filters): Remove extra
nesting level; add transparent setter to adjust old-format values;
update doc string.
(ibuffer-save-filters): Remove extra level of nesting
in ibuffer-saved-filters values when saving new filters (Bug#25049).
(ibuffer-old-saved-filters-warning): New variable with
clickable message with repair options to be displayed
as a warning if 'ibuffer-repair-saved-filters' detects
a format mismatch.
(ibuffer-repair-saved-filters): Add new command to check and
repair saved filters format.
(ibuffer-included-in-filter-p, ibuffer-decompose-filter):
Change access of saved filter data (cadr->cdr) to account
for reduced nesting.
* test/lisp/ibuffer-tests.el (ibuffer-save-filters):
New test; check that filters are saved in the proper format.
---
lisp/ibuf-ext.el | 156 +++++++++++++++++++++++++++++++++++----------
test/lisp/ibuffer-tests.el | 29 +++++++++
2 files changed, 150 insertions(+), 35 deletions(-)
diff --git a/lisp/ibuf-ext.el b/lisp/ibuf-ext.el
index 5ef0746..d1bf576 100644
--- a/lisp/ibuf-ext.el
+++ b/lisp/ibuf-ext.el
@@ -35,7 +35,8 @@
(eval-when-compile
(require 'ibuf-macs)
- (require 'cl-lib))
+ (require 'cl-lib)
+ (require 'subr-x))
;;; Utility functions
(defun ibuffer-delete-alist (key alist)
@@ -119,32 +120,100 @@ ibuffer-tmp-show-regexps
(defvar ibuffer-auto-buffers-changed nil)
-(defcustom 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)))))
-
- "An alist of filter qualifiers to switch between.
+(defun ibuffer-update-saved-filters-format (filters)
+ "Transforms alist from old to new `ibuffer-saved-filters' format.
+
+Specifically, converts old-format alist with values of the
+form (STRING (FILTER-SPECS...)) to alist with values of the
+form (STRING FILTER-SPECS...), where each filter spec should be a
+cons cell with a symbol in the car. Any elements in the latter
+form are kept as is.
+
+Returns (OLD-FORMAT-DETECTED . UPDATED-SAVED-FILTERS-LIST)."
+ (when filters
+ (let* ((old-format-detected nil)
+ (fix-filter (lambda (filter-spec)
+ (if (symbolp (car (cadr filter-spec)))
+ filter-spec
+ (setq old-format-detected t) ; side-effect
+ (cons (car filter-spec) (cadr filter-spec)))))
+ (fixed (mapcar fix-filter filters)))
+ (cons old-format-detected fixed))))
-This variable should look like ((\"STRING\" QUALIFIERS)
- (\"STRING\" QUALIFIERS) ...), where
-QUALIFIERS is a list of the same form as
-`ibuffer-filtering-qualifiers'.
-See also the variables `ibuffer-filtering-qualifiers',
-`ibuffer-filtering-alist', and the functions
-`ibuffer-switch-to-saved-filters', `ibuffer-save-filters'."
- :type '(repeat sexp)
+(defcustom 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))))
+
+ "An alist mapping saved filter names to filter specifications.
+
+Each element should look like (\"NAME\" . FILTER-LIST), where
+FILTER-LIST has the same structure as the variable
+`ibuffer-filtering-qualifiers', which see. The filters defined
+here are joined with an implicit logical `and' and associated
+with NAME. The combined specification can be used by name in
+other filter specifications via the `saved' qualifier (again, see
+`ibuffer-filtering-qualifiers'). They can also be switched to by
+name (see the functions `ibuffer-switch-to-saved-filters' and
+`ibuffer-save-filters'). The variable `ibuffer-save-with-custom'
+affects how this information is saved for future sessions. This
+variable can be set directly from lisp code."
+ :version "26.1"
+ :type '(alist :key-type (string :tag "Filter name")
+ :value-type (repeat :tag "Filter specification" sexp))
+ :set (lambda (symbol value)
+ ;; Just set-default but update legacy old-style format
+ (set-default symbol (cdr (ibuffer-update-saved-filters-format value))))
:group 'ibuffer)
+(defvar ibuffer-old-saved-filters-warning
+ (concat "Deprecated format detected for variable `ibuffer-saved-filters'.
+
+The format has been repaired and the variable modified accordingly.
+You can save the current value through the customize system by
+either clicking or hitting return "
+ (make-text-button
+ "here" nil
+ 'face '(:weight bold :inherit button)
+ 'mouse-face '(:weight normal :background "gray50" :inherit button)
+ 'follow-link t
+ 'help-echo "Click or RET: save new value in customize"
+ 'action (lambda (_)
+ (if (not (fboundp 'customize-save-variable))
+ (message "Customize not available; value not saved")
+ (customize-save-variable 'ibuffer-saved-filters
+ ibuffer-saved-filters)
+ (message "Saved updated ibuffer-saved-filters."))))
+ ". See below for
+an explanation and alternative ways to save the repaired value.
+
+Explanation: For the list variable `ibuffer-saved-filters',
+elements of the form (STRING (FILTER-SPECS...)) are deprecated
+and should instead have the form (STRING FILTER-SPECS...), where
+each filter spec is a cons cell with a symbol in the car. See
+`ibuffer-saved-filters' for details. The repaired value fixes
+this format without changing the meaning of the saved filters.
+
+Alternative ways to save the repaired value:
+
+ 1. Do M-x customize-variable and entering `ibuffer-saved-filters'
+ when prompted.
+
+ 2. Set the updated value manually by copying the
+ following emacs-lisp form to your emacs init file.
+
+%s
+"))
+
(defvar ibuffer-filtering-qualifiers nil
"A list like (SYMBOL . QUALIFIER) which filters the current buffer list.
See also `ibuffer-filtering-alist'.")
@@ -224,6 +293,28 @@ ibuffer-save-with-custom
:type 'boolean
:group 'ibuffer)
+(defun ibuffer-repair-saved-filters ()
+ "Updates `ibuffer-saved-filters' to its new-style format, if needed.
+
+If this list has any elements of the old-style format, a
+deprecation warning is raised, with a button allowing persistent
+update. Any updated filters retain their meaning in the new
+format. See `ibuffer-update-saved-filters-format' and
+`ibuffer-saved-filters' for details of the old and new formats."
+ (interactive)
+ (when (and (boundp 'ibuffer-saved-filters) ibuffer-saved-filters)
+ (let ((fixed (ibuffer-update-saved-filters-format ibuffer-saved-filters)))
+ (prog1
+ (setq ibuffer-saved-filters (cdr fixed))
+ (when-let (old-format-detected (car fixed))
+ (let ((warning-series t)
+ (updated-form
+ (with-output-to-string
+ (pp `(setq ibuffer-saved-filters ',ibuffer-saved-filters)))))
+ (display-warning
+ 'ibuffer
+ (format ibuffer-old-saved-filters-warning updated-form))))))))
+
(defun ibuffer-ext-visible-p (buf all &optional ibuffer-buf)
(or
(ibuffer-buf-matches-predicates buf ibuffer-tmp-show-regexps)
@@ -535,13 +626,11 @@ ibuffer-included-in-filter-p-1
(ibuffer-included-in-filter-p buf x))
(cdr filter))))
(`saved
- (let ((data
- (assoc (cdr filter)
- ibuffer-saved-filters)))
+ (let ((data (assoc (cdr filter) ibuffer-saved-filters)))
(unless data
(ibuffer-filter-disable t)
(error "Unknown saved filter %s" (cdr filter)))
- (ibuffer-included-in-filters-p buf (cadr data))))
+ (ibuffer-included-in-filters-p buf (cdr data))))
(_
(pcase-let ((`(,_type ,_desc ,func)
(assq (car filter) ibuffer-filtering-alist)))
@@ -849,15 +938,12 @@ ibuffer-decompose-filter
(cdr lim)
ibuffer-filtering-qualifiers)))
(`saved
- (let ((data
- (assoc (cdr lim)
- ibuffer-saved-filters)))
+ (let ((data (assoc (cdr lim) ibuffer-saved-filters)))
(unless data
(ibuffer-filter-disable)
(error "Unknown saved filter %s" (cdr lim)))
- (setq ibuffer-filtering-qualifiers (append
- (cadr data)
- ibuffer-filtering-qualifiers))))
+ (setq ibuffer-filtering-qualifiers
+ (append (cdr data) ibuffer-filtering-qualifiers))))
(`not
(push (cdr lim)
ibuffer-filtering-qualifiers))
@@ -936,7 +1022,7 @@ ibuffer-save-filters
ibuffer-filtering-qualifiers)))
(ibuffer-aif (assoc name ibuffer-saved-filters)
(setcdr it filters)
- (push (list name filters) ibuffer-saved-filters))
+ (push (cons name filters) ibuffer-saved-filters))
(ibuffer-maybe-save-stuff))
;;;###autoload
diff --git a/test/lisp/ibuffer-tests.el b/test/lisp/ibuffer-tests.el
index 3a4def3..6d5187a 100644
--- a/test/lisp/ibuffer-tests.el
+++ b/test/lisp/ibuffer-tests.el
@@ -66,5 +66,34 @@
(mapc (lambda (buf) (when (buffer-live-p buf)
(kill-buffer buf))) (list buf1 buf2)))))
+(ert-deftest ibuffer-save-filters ()
+ "Tests that `ibuffer-save-filters' saves in the proper format."
+ (skip-unless (featurep 'ibuf-ext))
+ (let ((ibuffer-save-with-custom nil)
+ (ibuffer-saved-filters nil)
+ (test1 '((mode . org-mode)
+ (or (size-gt . 10000)
+ (and (not (starred-name))
+ (directory . "\<org\>")))))
+ (test2 '((or (mode . emacs-lisp-mode) (file-extension . "elc?")
+ (and (starred-name) (name . "elisp"))
+ (mode . lisp-interaction-mode))))
+ (test3 '((size-lt . 100) (derived-mode . prog-mode)
+ (or (filename . "scratch")
+ (filename . "bonz")
+ (filename . "temp")))))
+ (ibuffer-save-filters "test1" test1)
+ (should (equal (car ibuffer-saved-filters) (cons "test1" test1)))
+ (ibuffer-save-filters "test2" test2)
+ (should (equal (car ibuffer-saved-filters) (cons "test2" test2)))
+ (should (equal (cadr ibuffer-saved-filters) (cons "test1" test1)))
+ (ibuffer-save-filters "test3" test3)
+ (should (equal (car ibuffer-saved-filters) (cons "test3" test3)))
+ (should (equal (cadr ibuffer-saved-filters) (cons "test2" test2)))
+ (should (equal (car (cddr ibuffer-saved-filters)) (cons "test1" test1)))
+ (should (equal (cdr (assoc "test1" ibuffer-saved-filters)) test1))
+ (should (equal (cdr (assoc "test2" ibuffer-saved-filters)) test2))
+ (should (equal (cdr (assoc "test3" ibuffer-saved-filters)) test3))))
+
(provide 'ibuffer-tests)
;; ibuffer-tests.el ends here
--
2.10.2
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
In GNU Emacs 26.0.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.22.4)
of 2016-11-30
Repository revision: b63c1ab39ad8d1d4edb290707373d769c08b55a9
^ permalink raw reply related [flat|nested] 12+ messages in thread
* bug#25049: ibuffer bug when saving existing filter, with patches
2016-11-30 7:02 ` Tino Calancha
2016-11-30 14:07 ` npostavs
@ 2016-12-07 10:56 ` Tino Calancha
1 sibling, 0 replies; 12+ messages in thread
From: Tino Calancha @ 2016-12-07 10:56 UTC (permalink / raw)
To: 25049-done
Tino Calancha <tino.calancha@gmail.com> writes:
> If we don't get further comments to address in 1 week, then let's
> push this fix to the master branch.
Pushed to master branch as commit 20f5a5b.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2016-12-07 10:56 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-28 6:54 bug#25049: ibuffer bug when saving existing filter, with patches Christopher Genovese
2016-11-29 15:06 ` Tino Calancha
2016-11-29 16:09 ` Christopher Genovese
2016-11-29 23:49 ` Tino Calancha
2016-11-30 5:14 ` Christopher Genovese
2016-11-30 7:02 ` Tino Calancha
2016-11-30 14:07 ` npostavs
2016-11-30 15:03 ` Tino Calancha
2016-11-30 15:11 ` Tino Calancha
2016-11-30 16:31 ` Christopher Genovese
2016-12-01 2:52 ` Tino Calancha
2016-12-07 10:56 ` Tino Calancha
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).