On Mar 5, 2012 11:11 PM, "Dmitry Kurochkin" wrote: > > On Mon, 5 Mar 2012 22:55:54 +0200, Jani Nikula wrote: > > 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 < 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. > > > > > > > > > > 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). > > Valid point. > > > 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.) > > > > All of these are invalid usages of `notmuch-saved-search-sort-function'. > The function is meant for sorting only (hence the name). So the code > might assume that the function does only sorting. > > I do not understand why we need such functionality (renaming, > capitalizing, etc.). You can just rename the query itself if you want > to. Should be easier IMO. Just for the record, I have a few important searches capitalized in the saved searches and just use the regular sort. Capitalized entries sort before the lowercase ones. > But if we need such functionality, we should > not misuse sorting function for it. We can add `notmuch-saved-searches' > function which would return saved searches list (sorted, renamed and > mangled in any other way). By default it would return > `notmuch-saved-searches' variable as is. Agreed. As to the problem at hand, we should just fix the sort function not to modify its input. I wouldn't hold my breath waiting for someone to provide patches for the rest... BR, Jani.