unifying libdiff2

[Jeremy Whiting]

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.

BR,
Jeremy


























On Thu, Jul 18, 2013 at 2:04 PM, Jeremy Whiting <jpwhiting at kde.org> wrote:

> 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:
>
> 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.
>
> B) Have KomparePart get the actions from the KompareModelList and add them
> to it's own ActionCollection.
>
> 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.
>
> 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.
>
> thanks,
> Jeremy Whiting
>
>
> On Wed, Jul 17, 2013 at 5:28 PM, Kevin Kofler <kevin.kofler at chello.at>wrote:
>
>> On Wednesday 17 July 2013 at 13:02:27, Jeremy Whiting wrote:
>> > Ok, I've got libdiff2 with diffsettings.h|cpp settingsbase.h|cpp moved
>> over
>> > into it preserving history, and changed komparemodellist.cpp to not need
>> > kompare_part.h anymore by using the default KActionCollection (I'm not
>> sure
>> > if that will give the same KActionCollection, but will test once I get
>> > kompare to build against this new libdiff2) and adding an isReadWrite
>> > parameter to the KompareModelList ctor.  KomparePart can pass that in
>> when
>> > it creates the KompareModelList just fine.  The resulting libdiff2 is at
>> > kde:scratch/whiting/libdiff2 if anyone wants to take a look.  It does
>> still
>> > need to install headers though.
>> >
>> > When trying to build kompare with libdiff2 as a new includepath though I
>> > realized there are viewsettings and filesettings in
>> kompare/libdialogpages
>> > that derive from settingsbase also.  So my question is should we move
>> those
>> > into libdiff2 also?  Then the next question is should all of
>> libdialogpages
>> > be inside libdiff2 also? It's a bit odd to have filesettings in libdiff2
>> > but fileview in libdialogpages.
>>
>> IMHO, libdiff2 should only carry what is needed for diffing, i.e.
>> diffsettings,
>> and settingsbase because it's required by diffsettings. The other
>> *settings
>> (viewsettings and filesettings) should stay in libdialogpages (and of
>> course
>> reuse settingsbase from libdiff2 by depending on libdiff2).
>>
>>         Kevin Kofler
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20130718/a2b3caf2/attachment-0001.html>