Review Request: ActionProperties merging fix

[David Faure]

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


Sorry, I let this one slip a bit... coming back to it.


/trunk/KDE/kdelibs/kdeui/xmlgui/kxmlguiversionhandler.cpp
<http://svn.reviewboard.kde.org/r/4531/#comment8568>

    This should use firstChildElement() / nextSiblingElement() to only iterate over elements.



/trunk/KDE/kdelibs/kdeui/xmlgui/kxmlguiversionhandler.cpp
<http://svn.reviewboard.kde.org/r/4531/#comment8569>

    Brace on the same line as the if (kdelibs/Qt coding style)



/trunk/KDE/kdelibs/kdeui/xmlgui/kxmlguiversionhandler.cpp
<http://svn.reviewboard.kde.org/r/4531/#comment8570>

    Maybe add a comment about what this is all about, for future readers of the code. If I understand correctly, something like "Keep action properties from global files, unless overriden locally".


- David


On 2010-07-06 20:58:39, Andras Mantia wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/4531/
> -----------------------------------------------------------
> 
> (Updated 2010-07-06 20:58:39)
> 
> 
> Review request for kdelibs and David Faure.
> 
> 
> Summary
> -------
> 
> When using replaceXMLFile (but it might happen in any case when setXMLFile is called on a gui client), the code tries to copy over the local ActionProperties section to the global one, so it doesn't get lost if the global file has a higher version number. Unfortunately when doing so, it removed all ActionProperties from the global file. This way it is not possible to have global xmlui files containing ActionProperties. 
> The patch fixes it, by only removing the properties of those actions that are present in the local file as well. This way user shortcut changes are preserved, even if the global file comes with different shortcuts, and global shortcuts are taken in account if they were not overwritten by the user. I suppose this was the intended behavior.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdelibs/kdeui/xmlgui/kxmlguiversionhandler.cpp 1146838 
> 
> Diff: http://svn.reviewboard.kde.org/r/4531/diff
> 
> 
> Testing
> -------
> 
> I did some basic testing with KDevelop, that has this code (view is a KTextEditor view, a KXMLGuiClient):
>         //in KDE >= 4.4 we can use KXMLGuiClient::replaceXMLFile to provide
>         //katepart with out own restructured UI configuration
>         QStringList katePartUIs = KGlobal::mainComponent().dirs()->findAllResources("data", "kdevelop/katepartui.rc");
>         const QString katePartUI = katePartUIs.last();
>         const QString katePartLocalUI = KStandardDirs::locateLocal("data", "kdevelop/katepartui.rc");
>         view->replaceXMLFile(katePartUI, katePartLocalUI);
> 
> 
> Thanks,
> 
> Andras
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20101021/9975e4b8/attachment.htm>