[PATCH v2] emacs: Pass a copy to notmuch-saved-search-sort-function

[Tomi Ollila]

On Mon, 05 Mar 2012 12:17:43 +0100, Daniel Schoepe <daniel at schoepe.org> wrote:
> On Mon, 05 Mar 2012 06:21:52 +0400, Dmitry Kurochkin <dmitry.kurochkin at gmail.com> wrote:
> > On Thu,  1 Mar 2012 21:24:38 +0100, Daniel Schoepe <daniel at schoepe.org> 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.

Hard to say -- maybe the alternative:

 (defun notmuch-sort-saved-searches (alist)
   "Generate an alphabetically sorted saved searches alist."
-  (sort alist (lambda (a b) (string< (car a) (car b)))))
+  (sort (copy-sequence alist) (lambda (a b) (string< (car a) (car b)))))

matches better with the current documentation 
(of notmuch-saved-search-sort-function).

Both sort and copy-sequence are blazingly fast...

For more information, read
http://www.emacswiki.org/emacs/DestructiveOperations

> Cheers,
> Daniel

Tomi