Review Request 120363: proposal to use the NOGUI switch in CMake files to set the default value for GUIenabled
thomas.luebking at gmail.com
Fri Sep 26 12:33:20 BST 2014
On Freitag, 26. September 2014 10:41:39 CEST, Ben Cooksley wrote:
> Applying a solution to a single OS is inherently wrong
> On Fri, Sep 26, 2014 at 5:01 PM, Thiago Macieira <thiago at kde.org> wrote:
>> And still it needs to be studied for Qt5
Afaics there are two major issues with this patch (and actually they make everyone right in this thread ;-)
1. It's Qt4 only
2. It's the broadsword
We can skip 1 (which is not "fixable", the API doesn't exist anymore) and should focus on 2 - and the foil may implicitly handle this as well ;-)
It is too broadsword as, as mentioned by Thiago, the GUIEnabled parameter has several implications on all systems (let alone that the patch attempts to control it by the so far unrelated NOGUI cmake switch).
Altering it (everywhere) to fix an issue with one system only, (which is also only indirectly caused by it) is dangerous (read: "wrong in an LTS")
But limiting modifications to one system only is just as wrong, since it will diversify behavior, thus harden future maintainance.
Luckily the behavior whether there's a menubar and stuff on OSX can be controlled by a QCoreApplication attribute that *is* OSX specific:
Stops the Qt mac application from doing specific initializations that do not necessarily make sense when using Qt to author a plugin. This includes avoiding loading our nib for the main menu and not taking possession of the native menu bar. When setting this attribute to true will also set the AA_DontUseNativeMenuBar attribute to true.
So my very first attempt would be to set that (unconditionally, ie. on all systems - it should be idempotent outside OSX anyway) before instantiating the application in the main funcs of the problematic applications AND LEAVE THE GUIEnabled PARAMETER UNTOUCHED.
Charmingly, such change would be Qt5 compatible =)
Personally I doubt that this needs to or should be injected by a cmake parameter, but added directly (notably since this will only affect a fistful of applications in KDE anyway)
More information about the kde-core-devel