--nextPart2542190.hkKLZBWbfW
Content-Type: Text/Plain;
  charset="utf-8"
Content-Transfer-Encoding: quoted-printable

Hi!

Taking this to the mailing list as suggested.

=46irst to clarify: My comment does not concern your patch that directly. Y=
our=20
patch does not change really change anything in that respect. But your patc=
h=20
addresses the case of making changes using setDomDocument(), i.e. changes=20
which are not (necessarily) written to disk, and that's the thing I am=20
concerned about. Sorry for causing this confusion.

On Friday 25 September 2009, David Faure wrote:
> I think the confusion is about which "kxmlguiclient changes" we're talking
>  about. The use case I see for setDomDocument() (which means, dynamic
>  changes not on disk), is the case where applications hardcode stuff like
>  "remove those actions that should not appear in this context" (e.g.
>  kaddressbookpart-inside-kontact).

In fact, as of now, I'm doing the same in RKWard. (See also this (submitted=
)=20
request of mine: http://reviewboard.kde.org/r/1229/). This is all perfectly=
=20
fine as long as we're talking about the "top" level of embedding, i.e. ther=
e is=20
no expectation that the modified xmlguiclient will be embedded anywhere els=
e,=20
directly or indirectly. I guess this is another thing I should have made cl=
ear=20
from the beginning...

>  This is not a user-configured change, it
>  should not be written to disk upfront (if it was, it would mess up
>  kaddressbook-standalone, while at least now it only does that when the
>  user configure toolbars; corner case).

Writing to disk would not necessarily mean writing to the original location=
,=20
though. Annoyed by the need to re-apply my local xml modifications time and=
=20
again, when changes happened (well, aesthetically annoyed - it's not really=
=20
that difficult, just ugly), I was considering to write out the modified xml=
 to=20
disk in a local/temporary path, then call replaceXMLFile() to use this modi=
fied=20
file on future reloads.

Note that there appear to be a number of occasions when the client's xml is=
=20
reloaded besides editing toolbars. E.g.=20
KXMLGUIFactory::refreshActionProperties() (called when switching shortcut=20
schemes, IIRC). I don't currently have a source tree attached to grep throu=
gh,=20
but I guess all instances where KXMLGUIClient::reloadXML() is used may need=
=20
similar patches as well.

> In fact
>  this isn't the only thing that needs to be done after rebuilding the GUI
>  (from edittoolbars), the other thing is plugging back any ActionLists. F=
or
>  that reason, the "syncer" probably breaks actionlists right now...

Eww. I'll need to check this.

>  With
>  the current infrastructure, it sounds like what we need is a
>  kxmlguifactory signal that apps would connect to that same
>  slotNewToolbarConfig slot, which is where the code for "stuff that has to
>  be done after createGUI" is located. It would be emitted by the method y=
ou
>  mention it should have, for removing+readding all clients, moving that
>  code out of kedittoolbarwidget.

Indeed, that would make things easier. But this sounds like apps would need=
 to=20
adjust to this, so not possible before KDE 5?

Even without this, though, couldn't/shouldn't all this code from=20
kedittoolbarwidget be moved into a proper function=20
KXMLGuiFactory::rebuildTheWholeGuiWithAllBellsAndWhistles()?

> PS: I don't understand your "Alternative" suggestion.

Well, overall it seems we have three problems, here. That "alternative"=20
suggestion concerns number 2, only:

1) Detecting when there are changes in other instances of the same=20
XMLGUIClient that should be propagated. Solvable by simply monitoring the=20
localXMLFile() for changes.

2) Merging only the changes (configured shortcuts / toolbars), without thro=
wing=20
away customizations. I admit, I'm not quite sure the solution is this simpl=
e,=20
but my idea was roughly: "Well, so instead of reloading the original xmlFil=
e()=20
*and* then merging it with the localXMLFile(), why don't we just skip the f=
irst=20
step and only merge the new version of the localXMLFile()". So basically in=
=20
most places where reloadXML() is called right now, reloadLocalXML() or=20
something like that would be called. Wouldn't it be possible to preserve=20
applicaiton customizations while still merging user changes this way?

3) Properly rebuilding the GUI after the changes have been merged. The prob=
lem=20
with ActionLists you pointed out.

Regards
Thomas

--nextPart2542190.hkKLZBWbfW
Content-Type: application/pgp-signature; name=signature.asc 
Content-Description: This is a digitally signed message part.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)

iEYEABECAAYFAkq8wcwACgkQEKRv+5DVNhhZlQCgqLyqYoZVfdCJX3I1r0fKCOqn
C9kAoMFF5Kjf2qAlZUJCatQ8snio4V8w
=xVwi
-----END PGP SIGNATURE-----

--nextPart2542190.hkKLZBWbfW--