unifying libdiff2

[Jeremy Whiting]

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/kompare-devel/attachments/20130718/d1d955fb/attachment.html>