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