From cdc35c0a66701e90f732d456226b6352f3700f4c Mon Sep 17 00:00:00 2001 From: Jani Nikula Date: Mon, 5 Mar 2012 22:55:54 +0200 Subject: [PATCH] Re: [PATCH v2] emacs: Pass a copy to notmuch-saved-search-sort-function --- 40/16418e0cfd39709b41a7452a9871af01084104 | 238 ++++++++++++++++++++++ 1 file changed, 238 insertions(+) create mode 100644 40/16418e0cfd39709b41a7452a9871af01084104 diff --git a/40/16418e0cfd39709b41a7452a9871af01084104 b/40/16418e0cfd39709b41a7452a9871af01084104 new file mode 100644 index 000000000..01c2d7a82 --- /dev/null +++ b/40/16418e0cfd39709b41a7452a9871af01084104 @@ -0,0 +1,238 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +Received: from localhost (localhost [127.0.0.1]) + by olra.theworths.org (Postfix) with ESMTP id CFABC431FAF + for ; Mon, 5 Mar 2012 12:55:57 -0800 (PST) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: -0.699 +X-Spam-Level: +X-Spam-Status: No, score=-0.699 tagged_above=-999 required=5 + tests=[HTML_MESSAGE=0.001, RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled +Received: from olra.theworths.org ([127.0.0.1]) + by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) + with ESMTP id MH1XNHoACuj0 for ; + Mon, 5 Mar 2012 12:55:57 -0800 (PST) +Received: from mail-pw0-f53.google.com (mail-pw0-f53.google.com + [209.85.160.53]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) + (No client certificate requested) + by olra.theworths.org (Postfix) with ESMTPS id BFEFB431FAE + for ; Mon, 5 Mar 2012 12:55:56 -0800 (PST) +Received: by pbcuo1 with SMTP id uo1so2937889pbc.26 + for ; Mon, 05 Mar 2012 12:55:54 -0800 (PST) +Received-SPF: pass (google.com: domain of jani@nikula.org designates + 10.68.244.99 as permitted sender) client-ip=10.68.244.99; +Authentication-Results: mr.google.com; + spf=pass (google.com: domain of jani@nikula.org + designates 10.68.244.99 as permitted sender) + smtp.mail=jani@nikula.org +Received: from mr.google.com ([10.68.244.99]) + by 10.68.244.99 with SMTP id xf3mr31943011pbc.154.1330980954856 + (num_hops = 1); Mon, 05 Mar 2012 12:55:54 -0800 (PST) +MIME-Version: 1.0 +Received: by 10.68.244.99 with SMTP id xf3mr27794218pbc.154.1330980954646; + Mon, 05 Mar 2012 12:55:54 -0800 (PST) +Received: by 10.68.12.103 with HTTP; Mon, 5 Mar 2012 12:55:54 -0800 (PST) +Received: by 10.68.12.103 with HTTP; Mon, 5 Mar 2012 12:55:54 -0800 (PST) +In-Reply-To: <1330613059-5130-2-git-send-email-daniel@schoepe.org> +References: <1330613059-5130-1-git-send-email-daniel@schoepe.org> + <1330613059-5130-2-git-send-email-daniel@schoepe.org> +Date: Mon, 5 Mar 2012 22:55:54 +0200 +Message-ID: + +Subject: Re: [PATCH v2] emacs: Pass a copy to + notmuch-saved-search-sort-function +From: Jani Nikula +To: Dmitry Kurochkin +Content-Type: multipart/alternative; boundary=047d7b33cc1e01282304ba8527e8 +X-Gm-Message-State: + ALoCoQmjdANh73fr7xQuzX0jtrH17qlCe0u0zuScxGQOiJgDzjZRvtxlXGoWfEs0dF1p4aPbyQY1 +Cc: notmuch@notmuchmail.org +X-BeenThere: notmuch@notmuchmail.org +X-Mailman-Version: 2.1.13 +Precedence: list +List-Id: "Use and development of the notmuch mail system." + +List-Unsubscribe: , + +List-Archive: +List-Post: +List-Help: +List-Subscribe: , + +X-List-Received-Date: Mon, 05 Mar 2012 20:55:58 -0000 + +--047d7b33cc1e01282304ba8527e8 +Content-Type: text/plain; charset=UTF-8 + +On Mar 5, 2012 5:43 PM, "Dmitry Kurochkin" +wrote: +> +> On Mon, 05 Mar 2012 12:17:43 +0100, Daniel Schoepe +wrote: +> > On Mon, 05 Mar 2012 06:21:52 +0400, Dmitry Kurochkin < +dmitry.kurochkin@gmail.com> wrote: +> > > On Thu, 1 Mar 2012 21:24:38 +0100, Daniel Schoepe +wrote: +> > > > notmuch-saved-search-sort-function might destructively modify its +> > > > input (`sort' does that, for instance), so it should not be given +> > > > notmuch-saved-searches directly. +> > > > --- +> > > +> > > -1 +> > > +> > > I think we should require `notmuch-saved-search-sort-function' not to +> > > have side effects. Current documentation should be more clear about +> > > this. We need to fix `notmuch-sort-saved-searches' to copy the list +> > > before calling `sort'. But we should not do it in +> > > `notmuch-hello-insert-saved-searches' for any sorting function (which +> > > may not need this copying). +> > +> > My reasoning was that since sort is such a common function, many users +> > will probably use sort for their own sorting functions, not realizing +> > that it has side effects. This will lead to confusing behavior that's +> > not so easy to track down. +> > +> > Copying the list of saved searches when running notmuch-hello does not +> > seem be relevant to performance to me, since it's a) not called that +> > often and b) the list of saved searches will rarely exceed 30 elements. +> > +> > Hence, this way we can avoid some headaches for users who define their +> > own sorting functions at a negligible (performance) cost. Incidentally, +> > this is also how notmuch-hello did it before the user-defined sections +> > patches. +> > +> +> I do not buy the argument that we should help users who implement their +> own sorting functions but do not read documentation for functions they +> use. Apparently, those who implemented the `sort' function had similar +> ideas. And I do not think it is our job to add workarounds for it. +> +> An alternative (and IMO better) solution would be to allow customization +> of compare function used for sorting instead of the sorting function +> itself. + +Providing the customization of the sort function is more powerful than the +compare function. In the case of saved searches I can imagine people might +want to partially use the original order while sort the rest (e.g. +important ones first in predefined order, others sorted). In fact this also +allows dropping out some elements. And renaming. And changing the queries... + +(I had something like that in mind originally but then settled with just +capitalizing the important ones to show them first.) + +BR, +Jani. + +> +> Regards, +> Dmitry +> +> > Cheers, +> > Daniel +> _______________________________________________ +> notmuch mailing list +> notmuch@notmuchmail.org +> http://notmuchmail.org/mailman/listinfo/notmuch + +--047d7b33cc1e01282304ba8527e8 +Content-Type: text/html; charset=UTF-8 +Content-Transfer-Encoding: quoted-printable + +


+On Mar 5, 2012 5:43 PM, "Dmitry Kurochkin" <dmitry.kurochkin@gmail.com> wrote:
+>
+> On Mon, 05 Mar 2012 12:17:43 +0100, Daniel Schoepe <daniel@schoepe.org> wrote:
+> > On Mon, 05 Mar 2012 06:21:52 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> w= +rote:
+> > > On Thu, =C2=A01 Mar 2012 21:24:38 +0100, Daniel Schoepe <= +daniel@schoepe.org> wrote:
+> > > > notmuch-saved-search-sort-function might destructively = +modify its
+> > > > input (`sort' does that, for instance), so it shoul= +d not be given
+> > > > notmuch-saved-searches directly.
+> > > > ---
+> > >
+> > > -1
+> > >
+> > > I think we should require `notmuch-saved-search-sort-functio= +n' not to
+> > > have side effects. =C2=A0Current documentation should be mor= +e clear about
+> > > this. =C2=A0We need to fix `notmuch-sort-saved-searches'= + to copy the list
+> > > before calling `sort'. =C2=A0But we should not do it in<= +br> +> > > `notmuch-hello-insert-saved-searches' for any sorting fu= +nction (which
+> > > may not need this copying).
+> >
+> > My reasoning was that since sort is such a common function, many = +users
+> > will probably use sort for their own sorting functions, not reali= +zing
+> > that it has side effects. This will lead to confusing behavior th= +at's
+> > not so easy to track down.
+> >
+> > Copying the list of saved searches when running notmuch-hello doe= +s not
+> > seem be relevant to performance to me, since it's a) not call= +ed that
+> > often and b) the list of saved searches will rarely exceed 30 ele= +ments.
+> >
+> > Hence, this way we can avoid some headaches for users who define = +their
+> > own sorting functions at a negligible (performance) cost. Inciden= +tally,
+> > this is also how notmuch-hello did it before the user-defined sec= +tions
+> > patches.
+> >
+>
+> I do not buy the argument that we should help users who implement thei= +r
+> own sorting functions but do not read documentation for functions they= +
+> use. =C2=A0Apparently, those who implemented the `sort' function h= +ad similar
+> ideas. =C2=A0And I do not think it is our job to add workarounds for i= +t.
+>
+> An alternative (and IMO better) solution would be to allow customizati= +on
+> of compare function used for sorting instead of the sorting function +> itself.

+

Providing the customization of the sort function is more powerful than t= +he compare function. In the case of saved searches I can imagine people mig= +ht want to partially use the original order while sort the rest (e.g. impor= +tant ones first in predefined order, others sorted). In fact this also allo= +ws dropping out some elements. And renaming. And changing the queries... + +

(I had something like that in mind originally but then settled with just= + capitalizing the important ones to show them first.)

+

BR,
+Jani.

+

>
+> Regards,
+> =C2=A0Dmitry
+>
+> > Cheers,
+> > Daniel
+> _______________________________________________
+> notmuch mailing list
+> notmuch@notmuchmail.org= +
+> http://not= +muchmail.org/mailman/listinfo/notmuch
+

+ +--047d7b33cc1e01282304ba8527e8-- -- 2.26.2