unifying libdiff2

[Jeremy Whiting]

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.

thanks,
Jeremy




On Fri, Jul 12, 2013 at 3:36 AM, Bruggeman, Otto G <
otto.g.bruggeman at intel.com> wrote:

> Sorry outlook here at work. Trying to fake some sane formatting...
>
> Kevin Kofler (KK) wrote:
>
> On Thursday 11 July 2013 at 20:18:37, Jeremy Whiting wrote:
> > Ok, it looks like the kdevplatform copy of libdiff gets around needing
> > diffsettings by copying diffsettings.h|cpp and settingsbase.h|cpp also.
> >  Which makes me wonder if that ought to just be part of libdiff2 and
> > kompare's libdialogchanges could get it from libdiff2 itself rather
> > than owning those classes and code.
>
> KK> +1, makes sense.
>
> OB> Yes full ack, this is how it was meant to be. Never found the time to
> finish this.
> OB> I also wanted to find a way to interface with the diff code directly
> so I would
> OB> not have to parse the diff output and use the internal representation
> of diff :(
> OB> If only there were 100 hours per day and I would only need 8 hours
> sleep...
>
> > The other difference and dependency of libdiff2 is kompare_part.h used
> > in komparemodellist.cpp to get the actions from the action collection
> > (can't this be done without casting the parent to komparePart though?)
> > and to get isReadWrite from the parent.  Maybe isReadWrite should be a
> > property of the komparemodellist rather than the kompare part? Then
> > when the kompare part isReadWrite changes, it just changes it on it's
> modellist also?
>
> KK> Yes, that dependency also needs to go away. I'm fine with whatever
> change is made
> KK> to make it unnecessary (as long as it doesn't break Kompare, of
> course).
>
> OB> The isReadWrite is determined when the Part is instantiated and does
> not change
> OB> during the lifetime of a Part if I understood this correctly back in
> the KDE3 days.
> OB> Not sure if this changed during KDE4 and later versions. David Faure
> would be able
> OB> to tell without any doubt whether my recollection is right.
>
> > thoughts, questions, etc. if not I'll make these changes and put up a
> > review request.
>
> KK> Go ahead. I agree with the ideas in principle, I'll review the
> implementation on
> KK> ReviewBoard. (I expect it will be fine, but I'd still like to see the
> code
> KK> before it goes in.)
>
>
> Intel GmbH
> Dornacher Strasse 1
> 85622 Feldkirchen/Muenchen, Deutschland
> Sitz der Gesellschaft: Feldkirchen bei Muenchen
> Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
> Registergericht: Muenchen HRB 47456
> Ust.-IdNr./VAT Registration No.: DE129385895
> Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kompare-devel/attachments/20130717/0c28b0db/attachment.html>