Switch semantics of filterLongerThan and filterShorterThan

[JoelCourtney]

When I made the filterShortThan and filterLongerThan methods in the timeline library, I had a choice of semantics, either "filterShorterThan filters OUT intervals shorter than" or "filterShorterThan filters SUCH THAT all intervals are shorter than". I chose the first, but in practice I get confused every time I use these functions. This is because with all other filter-like functions such as highlightEqualTo or isolateTrue, I named them in the pattern <operation><predicate>. So in highlightEqualTo("x"), if the segment is equal to "x", it is highlighted. Unfortunately I broke that pattern with filterLonger/ShorterThan; in filterShorterThan the object is retained if it doesn't match the shorter-than predicate.

I understand that this is the absolute worst kind of breaking change. It doesn't just invalidate existing code, it completely reverses what it does. But I can't think of another way that isn't more work than it's worth. I'm hoping the impact is pretty small since procedural scheduling is still a new feature.

[Mythicaeda]

I think the naming of these functions needs to be revisited, as if I use filterShorterThan("X"), I would expect the filter to remove everything shorter than X, which is the current behavior on develop AFAIU.

[dandelany]

the naming of these functions needs to be revisited

This PR is the revisitation 🙂

if I use filterShorterThan("X"), I would expect the filter to remove everything shorter than X

I disagree, I think the proposed change is more intuitive & closer to standard - as @JoelCourtney notes, it makes it more consistent with things like highlightEqualTo or isolateTrue. A stronger argument IMHO is that pretty much all languages that have a filter function use a syntax like filter(predicate, subject) where predicate is the thing you're including. So eg. if you had a isOdd function, filter(isOdd, myArr) will return all things that are odd, not filter out (remove) things that are odd, so devs who commonly use filter will likely expect this convention.

[dandelany]

I asked a few more people for opinions on this today (@mattdailis @joswig and @jmorton ) to try and reach consensus, and where we landed was:

• filterX in general can be a confusing name, in future we should consider naming things like this keepX or removeX
• However, doing ^this would a more far-reaching change than this PR needs right now & would take some more time/buy-in
• Given the choice between filterX meaning "keep X" or "filter out all X's", everyone I asked thought that "keep X" was the more intuitive choice given the syntax of filter() mentioned above
• Therefore I think we should approve & merge this PR