KMessageBox runtime dependency on FrameworkIntegrationPlugin makes it useless

[Albert Astals Cid]

El dissabte, 2 de juliol de 2016, a les 12:23:31 CEST, David Faure va 
escriure:
> On mercredi 15 juin 2016 00:33:40 CEST Albert Astals Cid wrote:
> > El dimarts, 14 de juny de 2016, a les 10:57:19 CEST, David Faure va
> 
> escriure:
> > > On mardi 14 juin 2016 01:00:09 CEST Albert Astals Cid wrote:
> > > > El dilluns, 13 de juny de 2016, a les 11:27:53 CEST, David Faure va
> > > 
> > > escriure:
> > > > > On dimanche 12 juin 2016 22:58:34 CEST Albert Astals Cid wrote:
> > > > > > El diumenge, 12 de juny de 2016, a les 20:29:56 CEST, Christoph
> > > > > > Cullmann
> > > > > > va
> > > > > > 
> > > > > > escriure:
> > > > > > > Hi,
> > > > > > > 
> > > > > > > > Having KMessageBox "dontShowMeAgain" feature depend on an
> > > > > > > > integration
> > > > > > > > plugin is a very bad idea.
> > > > > > > > 
> > > > > > > > Basically it means programs that use KMessageBox can never
> > > > > > > > asusme
> > > > > > > > it
> > > > > > > > will
> > > > > > > > work so basically they have to use alternative methods to have
> > > > > > > > the
> > > > > > > > "dontShowMeAgain" feature or not have it at all.
> > > > > > > > 
> > > > > > > > I understand someone thought that it was a better idea having
> > > > > > > > a
> > > > > > > > feature
> > > > > > > > that may work or not randomly that increasing the dependency
> > > > > > > > chain
> > > > > > > > of
> > > > > > > > KMessageBox, but I disagree.
> > > > > > > > 
> > > > > > > > I don't think the status quo is good at all, my program
> > > > > > > > basically
> > > > > > > > gets
> > > > > > > > a
> > > > > > > > runtime dependency that is not specified anywhere and that
> > > > > > > > makes
> > > > > > > > some
> > > > > > > > features work or not randomly.
> > > > > > > > 
> > > > > > > > The options I can see are:
> > > > > > > > * Remove the "dontShowMeAgain" feature from KMessageBox
> > > > > > > > * Make the "dontShowMeAgain" feature use QSettings (always or
> > > > > > > > if
> > > > > > > > FrameworkIntegrationPlugin is not available)
> > > > > > > > * Show a KMessageBox warning when trying to use the
> > > > > > > > "dontShowMeAgain"
> > > > > > > > feature and the FrameworkIntegrationPlugin is not available
> > > > > > > > saying
> > > > > > > > the
> > > > > > > > user to install that package if he wants to get the
> > > > > > > > functionality.
> > > > > > > 
> > > > > > > I would go for the "just use QSettings always" solution.
> > > > > > 
> > > > > > This has two problems:
> > > > > >  * Someone needs to care about a way to read the KConfig and write
> > > > > >  it
> > > > > >  to
> > > > > > 
> > > > > > QSettings so it still works for people that had already checked
> > > > > > "don't
> > > > > > show
> > > > > > me again"
> > > > > > 
> > > > > >  * There's a KMessageBox::setDontShowAgainConfig(KConfig *cfg)
> > > > > >  that
> > > > > >  would
> > > > > > 
> > > > > > break, this affects only the kdialog app and KIO::JobUiDelegate
> > > > > > 
> > > > > > The second is probably "workaroundable" but i'm not sure how one
> > > > > > would
> > > > > > approach the first.
> > > > > > 
> > > > > > Maybe still using the FrameworkIntegrationPlugin to check if the
> > > > > > option
> > > > > > has
> > > > > > been set and if it has and it is not in qsettings, move it there?
> > > > > > 
> > > > > > This would "fail" if the FrameworkIntegrationPlugin is not there,
> > > > > > but
> > > > > > that
> > > > > > would mean that most probably the kconfig was never ever set
> > > > > > either
> > > > > > (unless
> > > > > > you installed FrameworkIntegrationPlugin used a kmessagebox and
> > > > > > then
> > > > > > uninstalled FrameworkIntegrationPlugin that seems very corner
> > > > > > case-y).
> > > > > > 
> > > > > > I'll try to work on this in the coming days if noone disagrees.
> > > > > 
> > > > > Not sure exactly what is your final approach, from the above.
> > > > > 
> > > > > I would go for:
> > > > >   Make the "dontShowMeAgain" feature use QSettings if
> > > > >   FrameworkIntegrationPlugin is not available
> > > > > 
> > > > > i.e. as a fallback. This preserves the KConfig benefits/integration
> > > > > on
> > > > > a
> > > > > more complete install, but doesn't lead to a non-working checkbox
> > > > > when
> > > > > the
> > > > > plugin is missing.
> > > > 
> > > > That is not what i suggested, I suggested using QSettings only.
> > > > 
> > > > What's the point of using KConfig for this?
> > > 
> > > I can think of the following:
> > > - not breaking setDontShowAgainConfig(KConfig *cfg)
> > > - not breaking application code that might be setting or clearing
> > > these settings directly using KConfig (e.g. to implement "show all
> > > messageboxes again")
> > 
> > That would still not work if only using QSettings, application is again
> > only half working because we support two different places to save the
> > same value.
> 
> Right, I meant that at least these apps would keep working when the
> integrationplugin is installed. If you use QSettings only, then the GUI code
> using KConfig (to re-enable all msgboxes) is broken, for everyone, in all
> cases, isn't it?
> 
> > > - (and then you don't have to handle any migration, so it's actually
> > > much
> > > simpler IHMO)
> > 
> > We still need to handle the migration when/if the user installs
> > FrameworksIntegration no? Suddenly all his MessageBoxes will be back
> > without any apparent reason.
> 
> True. Although this feels like a corner case without much harm (one can just
> click "don't show again" once more).
> 
> > I guess the other benefit that KConfig brings is that it can support
> > "system overrides" of the config options, is that really useful for
> > "dontShowMeAgain" settings?
> 
> QSettings supports that, actually.
> 
> I think we have two different questions at hand here.
> 
> 1) what would have been the right solution for this if KF 5.0 wasn't out
> yet? ---> there I would agree, using QSettings only, no plugin, would have
> been much simpler. We would have changed the API to remove or rework
> setDontShowAgainConfig(KConfig *), and we would have ported the apps with
> GUI code to re-enable all messages from KConfig to QSettings.
>      ---> alternatively, I would now say, we should just have moved
> KMessageBox to a framework with a higher tier.
> 
> 2) how do we fix this now, taking into account existing apps, i.e. our BC/SC
> constraints.
> 
> We can't come up with a good solution for that question, because things are
> broken right now (no plugin = no way to save) and whatever we change, we
> break something else (QSettings only = we break existing GUI code;
> QSettings as fallback = we introduce "migration" issues when
> installing/uninstalling the plugin).
> 
> I'm wondering if the best option wouldn't to ask all packagers to make
> kwidgetsaddons depend on frameworkintegration. Everything else leads to
> breakage.

That effectively breaks the tier of kwidgetsaddons to be "Tier a lot", no?

Also there's "a lot of packagers" it's hard to reach them all even by using 
the list we have for it :D

> My ideal solution (kmessagebox in a higher tier) can't be done anymore
> either, because that would break SC (apps using it, might not link to that
> other lib).

KF6 you said? ;)

Cheers,
  Albert