Review Request: Make KXMLGUIClient::set[Local]XMLFile() public

[Thomas Friedrichsmeier]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/1229/
-----------------------------------------------------------

Review request for kdelibs.


Summary
-------

--- What this patch does:
This patch makes KXMLGUIClient::setXMLFile() and KXMLGUIClient::setLocalXMLFile() public, instead of protected.

--- What this is good for:
The problem I'm trying to address is to allow flexibility in merging the GUI of KXMLGUIClients, and esp. of KParts. Currently an embedding application has the following options for merging a KParts GUI:

1) Do not insert the part into the KXMLGUIFactory at all.
  1b) Individual actions can be inserted, manually, if their names are known.
2) Merge the full GUI of the part.
  2b) Individual actions can be removed from the actionCollection() manually, if their names are known, and if the part does not rely on having the actions in the collection.

Yet in some cases more control over GUI merging would be nice. This applies in particular to applications that
a) already come with a complex menu structure of their own, and/or
b) embed more than one part, and/or
c) embed one of the more heavy kparts like khtmlpart of katepart, while possibly needing only a limited subset of the functionality of those parts.

The solution that I'm trying to enable is to simply override the entire ui.rc of the embedded client/part (before adding it to the KXMLGUIFactory, obviously). This does require knowledge of the internal names of the KActions in question, but so do solutions 1b and 2b, above. Apart from that it allows complete control in a straight-forward manner.

--- Does this break things?
Definitely not as long as the embedding application does not make use of the new public functions. But let's look at what may break, when it does:

------ In the embedded client:
Obviously, the embedded client is only affected in the context of the embedding application. It does still have an intact actionCollection(). Also, a kpart cannot rely on being added to a KXMLGUIFactory at all. I don't see any potential for breakage, here.

------ In the embedding application:
Things may break, here, if the KAction names change in the embedded client. This, however is a problem that is inherent in the wish to achieve this sort of control (also in solutions 1b and 2b, above).

Things would also break, if the embedded client were to call set[Local]XMLFile (or one of the other setters) *after* the embedding client has tried to override the ui-definition. Yet, this usage is highly atypical. In general setXMLFile() and friends are called only during the construction phase, and in fact they do not usually have any effect at all if called after the client was added to the factory (unless the factory is told to rebuild, which the embedding application could try to detect, if needed).

------ What about that "virtual" keyword? Won't things break, when setXMLFile() is re-implemented, in a client?
I have no idea, why these functions are declared virtual at all, or what would be the point of re-implementing them. lxr.kde.org does not show any re-implementations. Using wcgrep I did not find relevant matches to "void setXMLFile", either. I suggest, that should be scheduled for removal in the next BIC release, but that's beyond the scope of this review request.

--- Overall:
I admit, this solution is not terribly elegant in some respects, esp. in requiring knowledge of the KAction names. Yet, I think the problem is real, and this solution provides a decent way to cope with it.

--- Related reading:
I've been fighting this problem for a while, now. See http://lists.kde.org/?l=kde-devel&m=112911871111633&w=2 (and follow-ups) and http://techbase.kde.org/Development/Architecture/KDE4/XMLGUI_Technology#stripping_down_a_KPart.27s_GUI for an approach using setXMLGUIBuildDocument() (which is much hackier for obvious reasons).


Diffs
-----

  trunk/KDE/kdelibs/kdeui/xmlgui/kxmlguiclient.h 1006783 

Diff: http://reviewboard.kde.org/r/1229/diff


Testing
-------


Thanks,

Thomas