[mythtv-users] Version 30.0 and toggle bookmarks confusion

[Tom Dexter]

On 4/24/19, Tom Dexter <digitalaudiorock at gmail.com> wrote:
> On 4/24/19, David Hampton <mythtv at love2code.net> wrote:
>> On Tue, 2019-04-23 at 23:00 -0400, David Hampton wrote:
>>> On Tue, 2019-04-23 at 21:28 +0100, Paul Harrison wrote:
>>> > On 23/04/2019 20:53, Peter Bennett wrote:
>>> >
>>> >
>>> > Those of us using master got the fix, at least I don't remember
>>> > having
>>> > to fix the keybindings manually, but the code to do the fixup was
>>> > removed before fixes/30 was released. Don't quite understand the
>>> > logic
>>> > behind that.
>>> >
>>> > https://github.com/MythTV/mythtv/commit/453178430d86f7
>>>
>>> Well, I was just doing what the comment at the top of the diffs for
>>> tv_play.cpp said to do.  It said to delete that code block when
>>> consolidating database version 1348, so I did.  I'll revert it since
>>> I committed it, and then let the original author figure out when/how
>>> to eventually remove it.
>>>
>>> David
>>
>> Code restored to master and fixes/30 branches.
>>
>> David
>>
>>
>
> I just checked out the current fixes/30. Maybe I'm missing something
> but I don't understand this change. I'd assumed that the change was a
> database update to physically add the new key bindings for existing
> frontends or something to that effect.
>
> The change you're referring to if I'm not mistaken is the one in
> mythtv/libs/libmythtv/tv_play.cpp(?). That appears to be dependent on
> the AltClearSavedPosition setting which will have been deleted during
> the backend update. Wouldn't that default to SET as apposed to TOGGLE
> regardless? Even if that weren't the case, I'm not understanding how
> that change wouldn't completely override actual key bindings...though
> there may be a lot more to that that I'm not aware of.
>
> Tom
>

It appears that the original reason for removing that code was that it
won't work correctly after the update from schema update from 1348 to
1349, which deletes the AltClearSavedPosition entry from the settings
table. Looking at this part of the new code in tv_play.cpp:

    QString testKey = GetMythMainWindow()->GetKey("TV Playback", ACTION_PAUSE);
    if (testKey != "?")
    {
        int alternate = gCoreContext->GetNumSetting("AltClearSavedPosition",0);
        QString selectKeys = GetMythMainWindow()->GetKey("Global",
ACTION_SELECT);
        if (selectKeys != "?")
        {
            if (alternate)
                togBkmKeys = selectKeys;
            else
                bkmKeys = selectKeys;
        }
    }
    REG_KEY("TV Playback", ACTION_SETBOOKMARK, QT_TRANSLATE_NOOP("MythControls",
            "Add Bookmark"), bkmKeys);
    REG_KEY("TV Playback", ACTION_TOGGLEBOOKMARK,
QT_TRANSLATE_NOOP("MythControls",
            "Toggle Bookmark"), togBkmKeys);

Any frontend that runs this after the backend has updated the schema
will end up with alternate set to that default 0 even if it was
originally set to 1 (as it is for mine), which will set the select
keys to the set bookmark keys in all cases. Beyond that I'm not clear
what would happen if the toggle key bindings had been
entered(?)...that is if it would attempt to set conflicting key
bindings for both(?).

Again, forgive me if there's something I'm missing, but this seems
almost surely broken to me.

Thanks
Tom