<p><br>
On Mar 5, 2012 11:11 PM, "Dmitry Kurochkin" <<a href="mailto:dmitry.kurochkin@gmail.com">dmitry.kurochkin@gmail.com</a>> wrote:<br>
><br>
> On Mon, 5 Mar 2012 22:55:54 +0200, Jani Nikula <<a href="mailto:jani@nikula.org">jani@nikula.org</a>> wrote:<br>
> > On Mar 5, 2012 5:43 PM, "Dmitry Kurochkin" <<a href="mailto:dmitry.kurochkin@gmail.com">dmitry.kurochkin@gmail.com</a>><br>
> > wrote:<br>
> > ><br>
> > > On Mon, 05 Mar 2012 12:17:43 +0100, Daniel Schoepe <<a href="mailto:daniel@schoepe.org">daniel@schoepe.org</a>><br>
> > wrote:<br>
> > > > On Mon, 05 Mar 2012 06:21:52 +0400, Dmitry Kurochkin <<br>
> > <a href="mailto:dmitry.kurochkin@gmail.com">dmitry.kurochkin@gmail.com</a>> wrote:<br>
> > > > > On Thu,  1 Mar 2012 21:24:38 +0100, Daniel Schoepe <<a href="mailto:daniel@schoepe.org">daniel@schoepe.org</a>><br>
> > wrote:<br>
> > > > > > notmuch-saved-search-sort-function might destructively modify its<br>
> > > > > > input (`sort' does that, for instance), so it should not be given<br>
> > > > > > notmuch-saved-searches directly.<br>
> > > > > > ---<br>
> > > > ><br>
> > > > > -1<br>
> > > > ><br>
> > > > > I think we should require `notmuch-saved-search-sort-function' not to<br>
> > > > > have side effects.  Current documentation should be more clear about<br>
> > > > > this.  We need to fix `notmuch-sort-saved-searches' to copy the list<br>
> > > > > before calling `sort'.  But we should not do it in<br>
> > > > > `notmuch-hello-insert-saved-searches' for any sorting function (which<br>
> > > > > may not need this copying).<br>
> > > ><br>
> > > > My reasoning was that since sort is such a common function, many users<br>
> > > > will probably use sort for their own sorting functions, not realizing<br>
> > > > that it has side effects. This will lead to confusing behavior that's<br>
> > > > not so easy to track down.<br>
> > > ><br>
> > > > Copying the list of saved searches when running notmuch-hello does not<br>
> > > > seem be relevant to performance to me, since it's a) not called that<br>
> > > > often and b) the list of saved searches will rarely exceed 30 elements.<br>
> > > ><br>
> > > > Hence, this way we can avoid some headaches for users who define their<br>
> > > > own sorting functions at a negligible (performance) cost. Incidentally,<br>
> > > > this is also how notmuch-hello did it before the user-defined sections<br>
> > > > patches.<br>
> > > ><br>
> > ><br>
> > > I do not buy the argument that we should help users who implement their<br>
> > > own sorting functions but do not read documentation for functions they<br>
> > > use.  Apparently, those who implemented the `sort' function had similar<br>
> > > ideas.  And I do not think it is our job to add workarounds for it.<br>
> > ><br>
> > > An alternative (and IMO better) solution would be to allow customization<br>
> > > of compare function used for sorting instead of the sorting function<br>
> > > itself.<br>
> ><br>
> > Providing the customization of the sort function is more powerful than the<br>
> > compare function. In the case of saved searches I can imagine people might<br>
> > want to partially use the original order while sort the rest (e.g.<br>
> > important ones first in predefined order, others sorted).<br>
><br>
> Valid point.<br>
><br>
> > In fact this also<br>
> > allows dropping out some elements. And renaming. And changing the queries...<br>
> ><br>
> > (I had something like that in mind originally but then settled with just<br>
> > capitalizing the important ones to show them first.)<br>
> ><br>
><br>
> All of these are invalid usages of `notmuch-saved-search-sort-function'.<br>
> The function is meant for sorting only (hence the name).  So the code<br>
> might assume that the function does only sorting.<br>
><br>
> I do not understand why we need such functionality (renaming,<br>
> capitalizing, etc.).  You can just rename the query itself if you want<br>
> to.  Should be easier IMO.  </p>
<p>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.</p>
<p>> But if we need such functionality, we should<br>
> not misuse sorting function for it.  We can add `notmuch-saved-searches'<br>
> function which would return saved searches list (sorted, renamed and<br>
> mangled in any other way).  By default it would return<br>
> `notmuch-saved-searches' variable as is.</p>
<p>Agreed.</p>
<p>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...</p>
<p>BR,<br>
Jani.<br>
</p>