From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Christopher Genovese Newsgroups: gmane.emacs.bugs Subject: bug#25049: ibuffer bug when saving existing filter, with patches Date: Tue, 29 Nov 2016 11:09:28 -0500 Message-ID: References: <8737ianx4s.fsf@gmail.com> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/alternative; boundary=001a1141f18c5614b8054272d185 X-Trace: blaine.gmane.org 1480436945 12566 195.159.176.226 (29 Nov 2016 16:29:05 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Tue, 29 Nov 2016 16:29:05 +0000 (UTC) Cc: 25049@debbugs.gnu.org To: Tino Calancha Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Tue Nov 29 17:29:01 2016 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cBlH9-0002Qq-5f for geb-bug-gnu-emacs@m.gmane.org; Tue, 29 Nov 2016 17:29:00 +0100 Original-Received: from localhost ([::1]:37972 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cBlHC-0000nU-U6 for geb-bug-gnu-emacs@m.gmane.org; Tue, 29 Nov 2016 11:29:02 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:45412) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cBkyu-0001zA-Gm for bug-gnu-emacs@gnu.org; Tue, 29 Nov 2016 11:10:14 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cBkyo-0004I6-BS for bug-gnu-emacs@gnu.org; Tue, 29 Nov 2016 11:10:08 -0500 Original-Received: from debbugs.gnu.org ([208.118.235.43]:59137) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1cBkyo-0004Hy-5N for bug-gnu-emacs@gnu.org; Tue, 29 Nov 2016 11:10:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1cBkyn-0007oQ-Ui for bug-gnu-emacs@gnu.org; Tue, 29 Nov 2016 11:10:01 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Christopher Genovese Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 29 Nov 2016 16:10:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 25049 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 25049-submit@debbugs.gnu.org id=B25049.148043579630015 (code B ref 25049); Tue, 29 Nov 2016 16:10:01 +0000 Original-Received: (at 25049) by debbugs.gnu.org; 29 Nov 2016 16:09:56 +0000 Original-Received: from localhost ([127.0.0.1]:46303 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cBkyi-0007o2-Gd for submit@debbugs.gnu.org; Tue, 29 Nov 2016 11:09:56 -0500 Original-Received: from mail-io0-f182.google.com ([209.85.223.182]:35829) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cBkyh-0007nn-1q for 25049@debbugs.gnu.org; Tue, 29 Nov 2016 11:09:55 -0500 Original-Received: by mail-io0-f182.google.com with SMTP id a124so297178803ioe.2 for <25049@debbugs.gnu.org>; Tue, 29 Nov 2016 08:09:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=2KkuvpX7u51wNHKKh9kOAj+QJwbwmnUR8oSv147lsqU=; b=cCFpfKuY4D7vVvAQN8dhpnalYM0DrPs63quyIBAy/9m8EYoHG0kIRc0jvEKPZP6msT PDcmggm6uwPaEcG4kQiNyuLkmPF9Udi3eOKEh+FttBNDtLZWS6c7jo6c6vnmyyMy1wsn /o2uyEAmlpg0MYXZ7KIiTl4pkM9ufNuQbALCTabccegVRBBM7Rd3p/gCvwBog9C/3s9a uXoHbmg0jLtKiDLKeRaShGVymovwdfqPzBI/sH8yHME0C0FD5ukv5G8cAqakcZz3lOWR W+E2qUw5A98GkvR7/jfmAtFjsBpuLh0yETxB2Ehl072UEFkYGxcWBOaIa/Bzqfh8RsQA jCkw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=2KkuvpX7u51wNHKKh9kOAj+QJwbwmnUR8oSv147lsqU=; b=BbbgHIZTcATgDFwh+bIFo1OrQXz1Yg4IrB5BBgNYvJAOKa0XYslqlC55DrDHhkW3zr obUWXIcDAs+UNDRxFYTXQ+aq/VRXVzeNkXXzjUfhlqOCHHf+br3bcoyL+6YemKa4D6xp 8WwL7fWxFxLp96WnWLDZ+FkYp5Uv0UOzt+yFN1o2TjaaJPsPniFCoHUW6wwzfMcjqeuk /BjQQjdWR0+mLMjfiQPUYAPQmDlwbPMD+FFZK5gQ9ODuKayEzqhpez0SYdpjhk2o0S2F vTFhhKDH03g9l5qMc2u1Ttkq5c8L3W/DAOEJ1xlkQ03vvz8xnuqwRyXz0iySOkogNbnr svLg== X-Gm-Message-State: AKaTC02yG12M/Ya5HKeqaJqzr/554iheskMRUnw69IJMSzl/ejN1zJP49msukUDAqaisCKMaHKePFeXryvDr4w== X-Received: by 10.107.165.139 with SMTP id o133mr22757548ioe.42.1480435789336; Tue, 29 Nov 2016 08:09:49 -0800 (PST) Original-Received: by 10.107.59.143 with HTTP; Tue, 29 Nov 2016 08:09:28 -0800 (PST) In-Reply-To: <8737ianx4s.fsf@gmail.com> X-Google-Sender-Auth: rG7QmIvPgCd97Ahy3mYIjVCcC5o X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:126247 Archived-At: --001a1141f18c5614b8054272d185 Content-Type: text/plain; charset=UTF-8 > 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 wrote: > Christopher Genovese 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 > --001a1141f18c5614b8054272d185 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
> 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 t= ime.=C2=A0 Thanks.

> 3) When `ibuffer-old-saved-filters-warn= ing' is shown i miss
>=C2=A0 =C2=A0 that the buffer displaying it= has the key binding
>=C2=A0 =C2=A0 `TAB' for `forward-button'= ;.
>=C2=A0 =C2=A0 It might be a smarter way to do this, but i just pu= t the buffer
>=C2=A0 =C2=A0 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?'.=C2=A0 Adding 'p' instea= d
> of '?' or even not suffix at all looks more usual.
OK, makes sense. I had thought of the -p convention
as bein= g for predicate functions, and I do like the
readability of t= he '?'.=C2=A0 But I get it.

> 5)
> Please, start sentences in capital letters.=C2=A0 Imper= ative
> form verb is prefered than pasive.

Understood.


Do you want me to submit a new patch with t= hese changes?

(And I know owe you new patches for the oth= er changes, which I have
not forgotten. I'm just a bit be= hind but hope to submit those
today.)

Thank= s again, Chris


<= br>
On Tue, Nov 29, 2016 at 10:06 AM, Tino Calanc= ha <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:
>
>=C2=A0 =C2=A01. Change the setcdr to add the extra level of nesting. >=C2=A0 =C2=A02. Change the format of 'ibuffer-saved-filters' to= remove
>=C2=A0 =C2=A0 =C2=A0 the level of testing, change the push (list->co= ns) and
>=C2=A0 =C2=A0 =C2=A0 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.=C2=A0 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
=C2=A0 =C2=A0`ibuffer-saved-filter' definition.

3) When `ibuffer-old-saved-filters-warning' is shown i miss
=C2=A0 =C2=A0that the buffer displaying it has the key binding
=C2=A0 =C2=A0`TAB' for `forward-button'.
=C2=A0 =C2=A0It might be a smarter way to do this, but i just put the buffe= r
=C2=A0 =C2=A0in help-mode as follows:

@@ -11,12 +11,15 @@
=C2=A0 =C2=A0 =C2=A0(let ((fixed (ibuffer-update-saved-filters-format = ibuffer-saved-filters)))
=C2=A0 =C2=A0 =C2=A0 =C2=A0(prog1
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(setq ibuffer-saved-filters (cdr f= ixed))
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 (when-let (old-format-detected? (car fixed)) +=C2=A0 =C2=A0 =C2=A0 =C2=A0 (when-let ((old-format-detected? (car fixed))<= br> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(buff= er-name "*Warnings*"))
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (with-current-buffer (get-buffer-create= buffer-name)
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (help-mode))
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(let ((warning-series t)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(updated-form=
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (with-output= -to-string
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (pp `= (setq ibuffer-saved-filters ',ibuffer-saved-filters)))))
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(display-warning
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 'ibuffer
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(format ibuffer-old-saved-= filters-warning updated-form))))))))
-
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(format ibuffer-old-saved-= filters-warning updated-form))
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 nil buffer-name))))))

4) [minor comment] It's not very common in Emacs to
call a var `old-format-detected?'.=C2=A0 Adding 'p' instead
of '?' or even not suffix at all looks more usual.

5)
>* lisp/ibuf-ext.el (ibuffer-saved-filters): removed extra
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0^^^^^^^
>nesting level in the alist values and updated docstring.
>(ibuffer-save-filters): removed extra level of nesting
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0^^^^^^^
>in saved filter alist values when saving new filters.
Please, start sentences in capital letters.=C2=A0 Imperative
form verb is prefered than pasive.

removed extra nesting --> Remove extra nesting

Thank you
Tino

--001a1141f18c5614b8054272d185--