Package: emacs;
Reported by: Christopher Genovese <genovese <at> cmu.edu>
Date: Mon, 28 Nov 2016 06:57:02 UTC
Severity: normal
Done: Tino Calancha <tino.calancha <at> gmail.com>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 25049 in the body.
You can then email your comments to 25049 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
View this report as an mbox folder, status mbox, maintainer mbox
bug-gnu-emacs <at> gnu.org
:bug#25049
; Package emacs
.
(Mon, 28 Nov 2016 06:57:02 GMT) Full text and rfc822 format available.Christopher Genovese <genovese <at> cmu.edu>
:bug-gnu-emacs <at> gnu.org
.
(Mon, 28 Nov 2016 06:57:02 GMT) Full text and rfc822 format available.Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
From: Christopher Genovese <genovese <at> cmu.edu> To: bug-gnu-emacs <at> gnu.org Cc: Tino Calancha <tino.calancha <at> gmail.com> Subject: ibuffer bug when saving existing filter, with patches Date: Mon, 28 Nov 2016 01:54:39 -0500
[Message part 1 (text/plain, inline)]
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
[Message part 2 (text/html, inline)]
[ibuffer-saved-filters-bug.patch (text/x-diff, attachment)]
bug-gnu-emacs <at> gnu.org
:bug#25049
; Package emacs
.
(Tue, 29 Nov 2016 15:08:02 GMT) Full text and rfc822 format available.Message #8 received at 25049 <at> debbugs.gnu.org (full text, mbox):
From: Tino Calancha <tino.calancha <at> gmail.com> To: 25049 <at> debbugs.gnu.org Cc: Christopher Genovese <genovese <at> cmu.edu>, tino.calancha <at> gmail.com Subject: Re: bug#25049: ibuffer bug when saving existing filter, with patches Date: Wed, 30 Nov 2016 00:06:43 +0900
Christopher Genovese <genovese <at> 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
bug-gnu-emacs <at> gnu.org
:bug#25049
; Package emacs
.
(Tue, 29 Nov 2016 16:10:01 GMT) Full text and rfc822 format available.Message #11 received at 25049 <at> debbugs.gnu.org (full text, mbox):
From: Christopher Genovese <genovese <at> cmu.edu> To: Tino Calancha <tino.calancha <at> gmail.com> Cc: 25049 <at> debbugs.gnu.org Subject: Re: bug#25049: ibuffer bug when saving existing filter, with patches Date: Tue, 29 Nov 2016 11:09:28 -0500
[Message part 1 (text/plain, inline)]
> 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 <at> gmail.com> wrote: > Christopher Genovese <genovese <at> 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 >
[Message part 2 (text/html, inline)]
bug-gnu-emacs <at> gnu.org
:bug#25049
; Package emacs
.
(Tue, 29 Nov 2016 23:50:01 GMT) Full text and rfc822 format available.Message #14 received at 25049 <at> debbugs.gnu.org (full text, mbox):
From: Tino Calancha <tino.calancha <at> gmail.com> To: Christopher Genovese <genovese <at> cmu.edu> Cc: 25049 <at> debbugs.gnu.org, tino.calancha <at> gmail.com Subject: Re: bug#25049: ibuffer bug when saving existing filter, with patches Date: Wed, 30 Nov 2016 08:49:22 +0900
Christopher Genovese <genovese <at> 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
bug-gnu-emacs <at> gnu.org
:bug#25049
; Package emacs
.
(Wed, 30 Nov 2016 05:15:02 GMT) Full text and rfc822 format available.Message #17 received at 25049 <at> debbugs.gnu.org (full text, mbox):
From: Christopher Genovese <genovese <at> cmu.edu> To: Tino Calancha <tino.calancha <at> gmail.com> Cc: 25049 <at> debbugs.gnu.org Subject: Re: bug#25049: ibuffer bug when saving existing filter, with patches Date: Wed, 30 Nov 2016 00:14:03 -0500
[Message part 1 (text/plain, inline)]
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 <at> gmail.com> wrote: > Christopher Genovese <genovese <at> 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 >
[Message part 2 (text/html, inline)]
[ibuffer-saved-filters-bug-clean.patch (text/x-diff, attachment)]
bug-gnu-emacs <at> gnu.org
:bug#25049
; Package emacs
.
(Wed, 30 Nov 2016 07:04:01 GMT) Full text and rfc822 format available.Message #20 received at 25049 <at> debbugs.gnu.org (full text, mbox):
From: Tino Calancha <tino.calancha <at> gmail.com> To: Christopher Genovese <genovese <at> cmu.edu> Cc: 25049 <at> debbugs.gnu.org, tino.calancha <at> gmail.com Subject: Re: bug#25049: ibuffer bug when saving existing filter, with patches Date: Wed, 30 Nov 2016 16:02:53 +0900
Christopher Genovese <genovese <at> 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
bug-gnu-emacs <at> gnu.org
:bug#25049
; Package emacs
.
(Wed, 30 Nov 2016 14:07:01 GMT) Full text and rfc822 format available.Message #23 received at 25049 <at> debbugs.gnu.org (full text, mbox):
From: npostavs <at> users.sourceforge.net To: Tino Calancha <tino.calancha <at> gmail.com> Cc: Christopher Genovese <genovese <at> cmu.edu>, 25049 <at> debbugs.gnu.org Subject: Re: bug#25049: ibuffer bug when saving existing filter, with patches Date: Wed, 30 Nov 2016 09:07:13 -0500
Tino Calancha <tino.calancha <at> gmail.com> writes: > Christopher Genovese <genovese <at> 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 <at> 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.
bug-gnu-emacs <at> gnu.org
:bug#25049
; Package emacs
.
(Wed, 30 Nov 2016 15:04:01 GMT) Full text and rfc822 format available.Message #26 received at 25049 <at> debbugs.gnu.org (full text, mbox):
From: Tino Calancha <tino.calancha <at> gmail.com> To: npostavs <at> users.sourceforge.net Cc: Christopher Genovese <genovese <at> cmu.edu>, 25049 <at> debbugs.gnu.org, Tino Calancha <tino.calancha <at> gmail.com> Subject: Re: bug#25049: ibuffer bug when saving existing filter, with patches Date: Thu, 01 Dec 2016 00:03:37 +0900
npostavs <at> users.sourceforge.net writes: Hi Noa, thanks for your time reading the report and for bringing those points! > Tino Calancha <tino.calancha <at> gmail.com> writes: > >> Christopher Genovese <genovese <at> 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 <at> 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 <at> 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
bug-gnu-emacs <at> gnu.org
:bug#25049
; Package emacs
.
(Wed, 30 Nov 2016 15:12:02 GMT) Full text and rfc822 format available.Message #29 received at 25049 <at> debbugs.gnu.org (full text, mbox):
From: Tino Calancha <tino.calancha <at> gmail.com> To: Tino Calancha <tino.calancha <at> gmail.com> Cc: Christopher Genovese <genovese <at> cmu.edu>, 25049 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net Subject: Re: bug#25049: ibuffer bug when saving existing filter, with patches Date: Thu, 1 Dec 2016 00:11:24 +0900 (JST)
On Wed, 30 Nov 2016, Tino Calancha wrote: >From: Tino Calancha <tino.calancha <at> 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 <at> cmu.edu> Date: Wed, 30 Nov 2016 23:49:53 +0900 Subject: [PATCH] ibuffer-saved-filters: Remove extra nesting level
bug-gnu-emacs <at> gnu.org
:bug#25049
; Package emacs
.
(Wed, 30 Nov 2016 16:33:02 GMT) Full text and rfc822 format available.Message #32 received at 25049 <at> debbugs.gnu.org (full text, mbox):
From: Christopher Genovese <genovese <at> cmu.edu> To: Tino Calancha <tino.calancha <at> gmail.com> Cc: 25049 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net Subject: Re: bug#25049: ibuffer bug when saving existing filter, with patches Date: Wed, 30 Nov 2016 11:31:55 -0500
[Message part 1 (text/plain, inline)]
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 <at> gmail.com> wrote: > On Wed, 30 Nov 2016, Tino Calancha wrote: > >> From: Tino Calancha <tino.calancha <at> 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 <at> cmu.edu> > > Date: Wed, 30 Nov 2016 23:49:53 +0900 > Subject: [PATCH] ibuffer-saved-filters: Remove extra nesting level > >
[Message part 2 (text/html, inline)]
[ibuffer-saved-filters-bug-cleaner.patch (text/x-diff, attachment)]
bug-gnu-emacs <at> gnu.org
:bug#25049
; Package emacs
.
(Thu, 01 Dec 2016 02:53:01 GMT) Full text and rfc822 format available.Message #35 received at 25049 <at> debbugs.gnu.org (full text, mbox):
From: Tino Calancha <tino.calancha <at> gmail.com> To: Christopher Genovese <genovese <at> cmu.edu> Cc: 25049 <at> debbugs.gnu.org, Tino Calancha <tino.calancha <at> gmail.com>, npostavs <at> users.sourceforge.net Subject: Re: bug#25049: ibuffer bug when saving existing filter, with patches Date: Thu, 01 Dec 2016 11:52:01 +0900
Christopher Genovese <genovese <at> 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 <at> 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
Tino Calancha <tino.calancha <at> gmail.com>
:Christopher Genovese <genovese <at> cmu.edu>
:Message #40 received at 25049-done <at> debbugs.gnu.org (full text, mbox):
From: Tino Calancha <tino.calancha <at> gmail.com> To: 25049-done <at> debbugs.gnu.org Subject: Re: bug#25049: ibuffer bug when saving existing filter, with patches Date: Wed, 07 Dec 2016 19:56:39 +0900
Tino Calancha <tino.calancha <at> 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.
Debbugs Internal Request <help-debbugs <at> gnu.org>
to internal_control <at> debbugs.gnu.org
.
(Wed, 04 Jan 2017 12:24:04 GMT) Full text and rfc822 format available.
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.