unifying libdiff2

Jeremy Whiting jpwhiting at kde.org
Fri Oct 28 03:33:47 UTC 2011


On Thu, Oct 27, 2011 at 8:46 PM, Kevin Kofler <kevin.kofler at chello.at> wrote:
> 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
>

Ok, took out the komparemodellist.cpp changes.  Cached the constEnd()
and committed.

thanks,
Jeremy




More information about the KDevelop-devel mailing list