From ee8b2dc749455a96c3880cf2b29a95678159ffee Mon Sep 17 00:00:00 2001 From: "Christopher R. Genovese" 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" 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" 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 . "\"))))) + (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