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> wrote: > > On Thu, 1 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 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