[mythtv] Ticket #1678: mythvideo sorting is case-sensitive

Anduin Withers awithers at anduin.com
Fri Apr 21 21:35:01 UTC 2006


> In a maintenance situation 75% or more of the effort is in
> understanding the existing code while the actual fixes are quite
> trivial. Certainly that was true with #1569 and I expect reviewing it
> will be the same.

True, but only to a point. My own Myth time comes in contiguous blocks that
are usually measured with an hour stick.

> Also, breaking up a diff into separate patches means complex hand
> editing of a file full of weird line offsets and hoping you got it
> right. More time for me and more frustration for the sucker who has to
> review it. Where is the gain?

Or it means maintaining your patch set with some external tool, or even
having a separate checkout where you apply, test, and submit patches. Hand
editing the diff is about the hardest way to implement sending in minimal
patches.

> As a general comment, I don't see a huge number of people who encounter
> problems and respond with thoughtful, tested patches so when that does
> happen the first response should be "thank you".

Thank you.

> That's why I'm
> surprised to get so much static because I'm not following some
> celestial ideal of patch creation.

Your static indicator may require some adjustments. People who encounter
problems and submit patches are also the only people capable of doing so
more than once. The incentive to get them to make bite-sized, style
consistent, patches is higher.

> Personally I prefer the phonebook ordering. But, the point is whatever
> you choose will be wrong for half the people so it's better to let
> those who care manipulate it through LC_COLLATE rather than hardcoding
> behaviour. Whatever. This is not a big issue for me.

In a world where tweaking locale settings is common knowledge I'd agree.
Weighing the uncommonness of that knowledge against a better reasonable
default still lands me in the case insensitive camp. Once a consistent sort
is applied changing this will be easy.
 
> However, as to the original report, the revision history for
> videolist.cpp doesn't go back far enough but I suspect there has always
> been a sql sort vs map sort difference and there was never universal
> case insensitivity.

I agree, not new breakage, just newly discoverable by some.

> One other comment, the patch Michael Dean submitted does key creation
> inside the compare function.

Yes, both of you have technically valid complaints about the relative
efficiencies of each others patches. 1) It is MythVideo 2) It is MythVideo
3) Those allocations and comparisons are meaningless in the face of other
inefficiencies in there. That isn't to say efficiency doesn't mater, but
simple things like lazy evaluation for genres, or even simpler things like
caching genres and countries are going to give noticeable returns while
summing the heap use and comparisons for X thousand entries will remain
academic.

-- 
Anduin Withers



More information about the mythtv-dev mailing list