<div dir="ltr">Hmm, after taking another look, updateModelListActions uses some internal data to the KompareModelList, so moving that out wouldn't make much sense. I'll see if there's a way to get the KomparePart's actioncollection without knowing about KomparePart possibly. Otherwise we'll have to go with option B, getting the actions from the KompareModelList and adding them to the KomparePart's actionCollection.<div>
<br></div><div>BR,</div><div>Jeremy<br><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div><div>
<br></div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">
On Thu, Jul 18, 2013 at 2:04 PM, Jeremy Whiting <span dir="ltr"><<a href="mailto:jpwhiting@kde.org" target="_blank">jpwhiting@kde.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr">Ok, I've got it working and building that way. I had to add DIFF2_EXPORT to SettingsBase but otherwise it's good. There's a problem though because the actions aren't created in the KomparePart but in the KompareModelList. Thus all the actions don't appear in the Difference menu, or on the toolbar. In order to solve this we could do one of the following:<div>
<br></div><div>A) Move the actions into KomparePart itself, and attach them to the KompareModelList's slots after it is created from within KomparePart. This has the advantage that KompareModelList doesn't need to update the actions anymore and thus doesn't need to get isReadWrite in it's constructor either to know if it can enable the modification actions.</div>
<div><br></div><div>B) Have KomparePart get the actions from the KompareModelList and add them to it's own ActionCollection.</div><div><br></div><div>I prefer A over B, but will do either way after your feedback. Once that's done I'll put my changes to kompare itself on a branch and up on review board, as well as push my local changes to my libdiff2 repo.</div>
<div><br></div><div>One other question, do we want the library to be called libdiff2 as the directory is named? or libkomparediff2 as the .so is named? I'd like to make them match, so I'd change one or the other to make them match.</div>
<div><br></div><div>thanks,</div><div>Jeremy Whiting</div></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Jul 17, 2013 at 5:28 PM, Kevin Kofler <span dir="ltr"><<a href="mailto:kevin.kofler@chello.at" target="_blank">kevin.kofler@chello.at</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>On Wednesday 17 July 2013 at 13:02:27, Jeremy Whiting wrote:<br>
> Ok, I've got libdiff2 with diffsettings.h|cpp settingsbase.h|cpp moved over<br>
> into it preserving history, and changed komparemodellist.cpp to not need<br>
> kompare_part.h anymore by using the default KActionCollection (I'm not sure<br>
> if that will give the same KActionCollection, but will test once I get<br>
> kompare to build against this new libdiff2) and adding an isReadWrite<br>
> parameter to the KompareModelList ctor. KomparePart can pass that in when<br>
> it creates the KompareModelList just fine. The resulting libdiff2 is at<br>
> kde:scratch/whiting/libdiff2 if anyone wants to take a look. It does still<br>
> need to install headers though.<br>
><br>
> When trying to build kompare with libdiff2 as a new includepath though I<br>
> realized there are viewsettings and filesettings in kompare/libdialogpages<br>
> that derive from settingsbase also. So my question is should we move those<br>
> into libdiff2 also? Then the next question is should all of libdialogpages<br>
> be inside libdiff2 also? It's a bit odd to have filesettings in libdiff2<br>
> but fileview in libdialogpages.<br>
<br>
</div>IMHO, libdiff2 should only carry what is needed for diffing, i.e. diffsettings,<br>
and settingsbase because it's required by diffsettings. The other *settings<br>
(viewsettings and filesettings) should stay in libdialogpages (and of course<br>
reuse settingsbase from libdiff2 by depending on libdiff2).<br>
<span><font color="#888888"><br>
Kevin Kofler<br>
</font></span></blockquote></div><br></div>
</div></div></blockquote></div><br></div>