unifying libdiff2

Kevin Kofler kevin.kofler at chello.at
Fri Oct 28 02:46:56 UTC 2011


On Friday 28 October 2011, Jeremy Whiting wrote:
> That said in comparing the two versions I merged some things locally
> from kdevelop version to kdesdk version.  I've attached a patch to
> kdesdk version with those improvements.  I suggest these changes get
> applied to kdesdk version and after the dependency issue is resolved
> the forked version go away.

Reviewing those changes:

* Removing the endl after kDebug is an obvious bugfix, so obviously OK.

* Re these komparemodellist.cpp changes:
-       
-       if( !temp.open() ) {
+       bool correct=temp.open();
+
+       if( !correct ) {
and:
+    bool opened=m_diffTemp->open();
 
-       if( !m_diffTemp->open() ) {
+       if( !opened ) {
Why the extra variables? In fact, I removed those variables when I fixed the 
inverted login in revision 1145313 because they're redundant. They clearly 
didn't help preventing inverted logic. ;-)

* Moving the operator == on Marker inside the class is OK. (I don't think 
there was anything wrong per se with the non-member version, but making it a 
member is more common.)

* Removing spaces from the blank lines in kompareprocess is of course OK. (I 
hate spaces in blank lines!)

* The optimization of begin and end to constBegin and constEnd in 
kompareprocess is also OK. That said, while we are at optimizing things, 
shouldn't constEnd also be saved to a variable? As the code stands now, we're 
calling constEnd() at every loop iteration.

        Kevin Kofler




More information about the KDevelop-devel mailing list