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

[George Nassas]

On 21-Apr-06, at 4:00 AM, Anduin Withers wrote:

> Smaller patches, incrementally adding features, are preferred. In the 
> end it
> saves everyone time and frustration.

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. I figured one review is quicker than a bunch where 
you go back to step 1 each time. Plus, none of the changes are 
objectionable so it's not like some will be accepted and some rejected. 
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?

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". That's why I'm 
surprised to get so much static because I'm not following some 
celestial ideal of patch creation. Look, normally I do try to work that 
way but in this case it didn't play out like that so you got what you 
got. Deal with it.

The #1569 changes are a big improvement for me and I've gotten some 
off-list mail that tells me it works well for others too. I'd like to 
see those changes be a part of the next release. If there's something 
about the patch that's difficult for you to follow then drop me a note 
and I'll help you understand it.

> equals latin1_swedish_ci). The difference between (AAA, aaa, All About 
> Eve)
> and (AAA, All About Eve, aaa) is noticeably annoying in longer lists.

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.

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. Perhaps his DB environment changed around the time 
he upgraded myth. Also whatever. This is not a big issue for me.

One other comment, the patch Michael Dean submitted does key creation 
inside the compare function. With 1,000 videos that code will get hit 
3,000 times (the sort is NlogN). I'm not sure if that and the 
accompanying heap activity is significant but it can't help. With #1569 
applied mythvideo entry is quite snappy so I'd hate to see regression 
on that front. I recommend adding a sortkey member variable to the 
Metadata class and assigning that during the accumulation loop in 
VideoList::buildDbList.

- George