Excerpts from Rob Miller's message of Fri Jan 23 13:30:36 -0500 2009: > Nicholas Bergson-Shilcock wrote: > > While troubleshooting a bug with the recently added next/previous thread > > links, > > I noticed that getToplevelMessages[1] was not behaving as I expected it to. > > Specifically, there were two places where I wanted different behavior: > > > > 1. The function takes an optional boolean argument, recent_first. When true, > > the method sorts the threads by their modification dates, with the thread > > with > > the most recent response listed first. Unexpectedly (at least to me), when > > recent_first is false (the default), the threads are returned sorted by > > their > > dates, i.e., by when the initial message was sent. Thus this is *not* > > necessarily the reverse ordering from when specifying recent_first = True. > > > > 2. getToplevelMessages also takes optional start and end arguments for > > specifying a date range. Again, this range uses the dates and not the > > modification dates of the messages. This leads to confusion, particularly > > when > > sorting on the modification, since you will likely not get the results you > > expect (e.g., you'll likely get messages that you thought should have been > > filtered out). > > > > I'd like to modify the function to both sort and filter by > > modification_date. > > This seems to me to be more sensible behavior, particularly because it > > provides > > the expected behavior when dealing with threads. I've grep'd through the > > code > > and it doesn't look like it will break anything, but I wanted to shoot an > > e-mail out to the list to make sure no one was relying on the current > > behavior > > or had some reason to prefer it. > > i agree that we should consider some changes here, but i'm not completely > clear on what you're proposing. for instance, for #1 above, i would not > expect recent_first being False to necessarily reverse the order in which the > threads are returned, so if that's what you're thinking it would do i'm not > sure i like that idea. >
Yes, that is what I was thinking. I agree that setting recent_first to False does not *necessarily* (in the logical sense) need to return the reversed list, however, I still think it's a good idea. At a minimum, the interface needs to be more clearly defined, as right now the only way to know how things are ordered is to read the actual code. Is there a present use case for doing otherwise? > maybe what we need is a better API altogether. instead of a single boolean, > we might accept a 'sort_key' argument and an optional 'descending' or > 'reversed' boolean argument. this would allow for more expressiveness and > more clarity. would doing things this way meet your needs? does it > complicate things enough to make you balk at wanting to do so right now? > I'm +0 on this, mostly because I think adding this on top of recent_first would be clunky, and doing away with recent_first would change the established interface. My needs could definitely be met by this API, however, assuming there were both sort_on and filter_on (or something similar) keywords. -Nick > -r > -- Archive: http://www.openplans.org/projects/listen/lists/listen-dev/archive/2009/01/1232742265047 To unsubscribe send an email with subject "unsubscribe" to [email protected]. Please contact [email protected] for questions.
