[mythtv] Ignore 'The|A|An' prefixes when sorting video directories

[David Hampton]

On Fri, 2016-12-23 at 13:28 -0500, Michael T. Dean wrote:
> On 12/22/2016 01:29 PM, David Hampton wrote:
> > On Thu, 2016-12-22 at 11:23 +0000, Stuart Auchterlonie wrote:
> > > On 22/12/16 06:01, Jean-Yves Avenard wrote:
> > > > _____________________________
> > > > From: David Hampton <mythtv at dhampton.net <mailto:mythtv at dhampto
> > > > n.ne
> > > > t>>
> > > > Sent: Thursday, December 22, 2016 4:32 am
> > > > Subject: [mythtv] Ignore 'The|A|An' prefixes when sorting video
> > > > directories
> > > > To: <mythtv-dev at mythtv.org <mailto:mythtv-dev at mythtv.org>>
> > > > 
> > > > 
> > > > Hi,
> > > > 
> > > > I'm a longtime (10+ year) user of MythTv, but I think this is
> > > > my
> > > > first
> > > > attempt at submitting a patch. The video library code currently
> > > > ignores
> > > > the 'The|A|An' prefixes when sorting video names, but it
> > > > doesn't do
> > > > so
> > > > when sorting directory names. This is most noticeable for me in
> > > > the
> > > > TV
> > > > section of my video library where the files are all grouped
> > > > into
> > > > folders by show name and season. I have a couple of line patch
> > > > to
> > > > 0.28
> > > > that adds support for ignoring common prefixes on directories.
> > > > I
> > > > wanted
> > > > to run it past this list to see if its acceptable before
> > > > creating a
> > > > Trac entry. Thanks.
> > > > 
> > > > David
> > > > 
> > > > 
> > > > diff --git a/mythtv/programs/mythfrontend/videolist.cpp
> > > > b/mythtv/programs/mythfrontend/videolist.cpp
> > > > index 7bcbda0..926c0a8 100644
> > > > --- a/mythtv/programs/mythfrontend/videolist.cpp
> > > > +++ b/mythtv/programs/mythfrontend/videolist.cpp
> > > > @@ -195,8 +195,9 @@ struct metadata_path_sort
> > > >   
> > > >       bool sort(const QString &lhs, const QString &rhs)
> > > >       {
> > > > -        QString lhs_comp(lhs);
> > > > -        QString rhs_comp(rhs);
> > > > +        const QRegExp prefixes = QRegExp(QObject::tr("^(The |A
> > > > |An
> > > > )"));
> > > > +        QString lhs_comp = QString(lhs).remove(prefixes);
> > > > +        QString rhs_comp = QString(rhs).remove(prefixes);
> > > >           if (m_ignore_case)
> > > >           {
> > > >               lhs_comp = lhs_comp.toLower();
> > > > _______________________________________________
> > > > 
> > > > The issue with your patch is that it will only work if you use
> > > > A or
> > > > An.
> > > > Any capitalisations will cause it to not work (eg an allied
> > > > directory)
> > > > You need to remove the strings after performing the
> > > > comp.to.lower
> > > > operation.
> > > > 
> > > > 
> > > 
> > > There is a modifier you can pass to the QRegExp to make it do
> > > case insensitive regexp's which will solve this issue.
> > 
> > Thanks. I added the Qt::CaseInsensitive argument, retested, and
> > filed a
> > ticket.
> 
> Haven't looked myself, but is yours done the same way it's done for
> the videos themselves and in Watch Recordings?  Would be nice to make
> sure they're all the same--even if the same is "wrong"--for the
> principle of least surprise.
> 
> So, basically, IMHO, the patch to add this feature to should make it 
> work like the rest.  If the rest are case-sensitive matching, a
> separate patch should be made to change all of them to be case-
> insensitive matching.

Its inconsistent today. There's one instance in libmythmetadata which
conditionally does case sensitive or case insensitive comparisons,
although all of its current callers request case insensitive
comparisons. There are three usages in mythfrontend (playbackbox.cpp,
proglist.cpp and programrecpriority.cpp), all of which are hard coded
to use case sensitive comparisons.

The metadata_path_sort structure in videolist.c that I was extending
already had a m_ignore_case boolean (which I should have used),
although all its current callers specify case insensitive comparisons.
I will update my patch to be case sensitive/insensitive based on the
existing m_ignore_case variable.

In order to be fully consistent, I would need to modify the three
regexps in the frontend code to be case insensitive. It would be nice
if all comparisons could all be keyed off the same variable, but I
can't find a good place for it. There doesn't seem to be any common
parent structure where it could go. The three frontend classes are all
derived from ScheduleCommon, while the callers to the other two
instances are from the VideoListImp class that has no parent. Would it
make more sense to make this a global variable in the frontend code? I
just started looking at globalsettings.h/cpp files which might be a
possible location to put this boolean, although it looks like I'm
drifting into code that creates the settings pages. Does this line of
thought make sense and am I looking in the right direction?

David

P.S. I know there's bug report (#12298) that says that sorting should
never ignore prefixes. I was thinking that an exception list would
solve the reported problem (i.e. don't strip prefixes from the string
"A to Z") while stripping prefixes for the normal case. I was
considering this as a follow-on project, although passing a list of
strings into the various sorting functions seems messier than the
current code that only passes a single boolean.