Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#12503 closed Bug Report - General (fixed)

Mythfilldatabase translates all program genres to lower case

Reported by: John Bergqvist <JohnLBergqvist@…> Owned by: Nicolas Riendeau
Priority: minor Milestone: 0.28
Component: MythTV - Mythfilldatabase Version: 0.27-fixes
Severity: low Keywords:
Cc: Ticket locked: no

Description

When using the atlas or radio times XMLTV grabbers, mythfilldatabase is translating all genres/categories (and possibly category_type too) for each program into lowercase when they are inserted into the program table.

When running the grabbers manually & inspecting the XML files, the genres are in their correct case & capitalised as appropriate, but when mythfilldatabase inserts them into the database, any capitalisation is lost. This does not occur for genres of programs inserted via EIT, so it looks to me that this is mythfilldatabase's doing.

Change History (17)

comment:1 Changed 3 years ago by JohnLBergqvist@…

comment:2 Changed 3 years ago by William L. DeRieux IV <williamderieux@…>

Why not just use a case-insensitive comparison?

if ((cat.compare(QObject::tr("movie"),Qt::CaseSensitivity::CaseInsensitive) == 0) ||
        (cat.compare(QObject::tr("film"),Qt::CaseSensitivity::CaseInsensitive) == 0))
{
    // Hack for tv_grab_uk_rt
    pginfo->categoryType = ProgramInfo::kCategoryMovie;

comment:3 in reply to:  2 Changed 3 years ago by William L. DeRieux IV <williamderieux@…>

Replying to William L. DeRieux IV <williamderieux@…>:

Why not just use a case-insensitive comparison?

if ((cat.compare(QObject::tr("movie"),Qt::CaseSensitivity::CaseInsensitive) == 0) ||
        (cat.compare(QObject::tr("film"),Qt::CaseSensitivity::CaseInsensitive) == 0))
{
    // Hack for tv_grab_uk_rt
    pginfo->categoryType = ProgramInfo::kCategoryMovie;

updated wrong use of Qt::CaseSensitivity::CaseInsensitive?

if ((cat.compare(QObject::tr("movie"),Qt::CaseInsensitive) == 0) ||
        (cat.compare(QObject::tr("film"),Qt::CaseInsensitive) == 0))
{
    // Hack for tv_grab_uk_rt
    pginfo->categoryType = ProgramInfo::kCategoryMovie;

comment:4 Changed 3 years ago by JohnLBergqvist@…

My change was solely built on the existing code.

comment:5 in reply to:  4 Changed 3 years ago by William L. DeRieux IV <williamderieux@…>

Replying to JohnLBergqvist@…:

My change was solely built on the existing code.

True I realize that. But a case-insensitive comparison will future proof the code should the category case be changed.

comment:6 Changed 3 years ago by Gary Buhrmaster <gary.buhrmaster@…>

Note for any potential committer: This pull request appears to add/change translated strings (which may have an impact on any proposed back-porting and the translators workload and processes (*)).

(*) I admit to knowing very little about the translators work flow, but I do know there is work there.

comment:7 Changed 3 years ago by Nicolas Riendeau

Hi Gary!

Changing string at this point in time is still acceptable if it fixes a problem but will no longer be in a few weeks time when we go into full string freeze (we are in soft string freeze right now).

Don't expect any updates from our translators against 0.27 though...

Since it doesn't change the strings *and* as William said it's more future proof I personally much rather prefer the case-insensitive comparison...

Thank you and have a nice day!

Nicolas Riendeau MythTV developer and translation maintainer

comment:8 Changed 3 years ago by Nicolas Riendeau

I was contacted about this so I better explain what I said above a little better...

Assuming it does what it is supposed to, I am definitely AGAINST backporting it to 0.27 as it would break translations, while it might fix one person's problem it would create one for someone else...

Our translators currently work against master and the upcoming 0.28, we very rarely receive updates for the current -fixes version so it would create a problem which would not get fixes until the next -fixes version.

However, if the translatable strings are left as they initially were and the comparison is made case-insensitive and assuming this does what it is supposed to I have no problem with it being backported to 0.27.

I also think that since we have no control over the data we are feeded we should make the comparison case-insensitive and not depend on it being of a specific case, this could likely break in the future...

John, could you post a revised patch?

Thank you and have a nice day!

Nicolas

comment:9 in reply to:  8 Changed 3 years ago by William L. DeRieux IV <williamderieux@…>

Replying to knight:

I was contacted about this so I better explain what I said above a little better...

Assuming it does what it is supposed to, I am definitely AGAINST backporting it to 0.27 as it would break translations, while it might fix one person's problem it would create one for someone else...

Our translators currently work against master and the upcoming 0.28, we very rarely receive updates for the current -fixes version so it would create a problem which would not get fixes until the next -fixes version.

However, if the translatable strings are left as they initially were and the comparison is made case-insensitive and assuming this does what it is supposed to I have no problem with it being backported to 0.27.

I also think that since we have no control over the data we are feeded we should make the comparison case-insensitive and not depend on it being of a specific case, this could likely break in the future...

John, could you post a revised patch?

Thank you and have a nice day!

Nicolas

maybe this should marked as 'info needed' since an updated patch has been requested.

comment:10 Changed 3 years ago by JohnLBergqvist@…

Updated patch posted into pull request: https://github.com/MythTV/mythtv/pull/108

comment:11 in reply to:  8 Changed 3 years ago by John Bergqvist <JohnLBergqvist@…>

Replying to knight:

I was contacted about this so I better explain what I said above a little better...

Assuming it does what it is supposed to, I am definitely AGAINST backporting it to 0.27 as it would break translations, while it might fix one person's problem it would create one for someone else...

Our translators currently work against master and the upcoming 0.28, we very rarely receive updates for the current -fixes version so it would create a problem which would not get fixes until the next -fixes version.

However, if the translatable strings are left as they initially were and the comparison is made case-insensitive and assuming this does what it is supposed to I have no problem with it being backported to 0.27.

I also think that since we have no control over the data we are feeded we should make the comparison case-insensitive and not depend on it being of a specific case, this could likely break in the future...

John, could you post a revised patch?

Thank you and have a nice day!

Nicolas

I've posted an updated patch into my pull request: ​https://github.com/MythTV/mythtv/pull/108

comment:12 Changed 3 years ago by Nicolas Riendeau

Milestone: unknown0.28
Owner: changed from stuartm to Nicolas Riendeau
Status: newassigned

comment:13 Changed 3 years ago by JohnBergqvist <John@…>

Resolution: fixed
Status: assignedclosed

In c8eecc119925fc26749750619db23623b335615f/mythtv:

Fixes #12503 Prevents program categories being converted to lower-case to bring inline with categories imported over EIT - Patched to use case-insensitive comparison

comment:14 Changed 3 years ago by Nicolas Riendeau <nriendeau@…>

In 702d0238cf7d8e4c7d6d4d749cc098a02a6616d1/mythtv:

Stop mythfilldatabase from lowercasing all program genres

Closes #12503

Merge remote-tracking branch 'JohnLBergqvist/johnlbergqvist'

comment:15 Changed 3 years ago by Nicolas Riendeau <nriendeau@…>

In a655a8762f53944be8588db9b9321e0be5edebdf/mythtv:

Stop mythfilldatabase from lowercasing all program genres

Closes #12503

Merge remote-tracking branch 'JohnLBergqvist/johnlbergqvist'

comment:16 Changed 3 years ago by Nicolas Riendeau <nriendeau@…>

In d76c3518c23f2e8abbdb7debac56b521e3a2f53e/mythtv:

Stop mythfilldatabase from lowercasing all program genres

Closes #12503

Merge remote-tracking branch 'JohnLBergqvist/johnlbergqvist'

comment:17 Changed 3 years ago by Nicolas Riendeau

Thank you John!

Note: See TracTickets for help on using tickets.