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