[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    Re: KDE/kdelibs/kate/view
From:       Pascal =?iso-8859-15?q?L=E9tourneau?= <pascal.letourneau () gmail ! com>
Date:       2010-12-02 23:03:46
Message-ID: 201012021803.46448.pascal.letourneau () gmail ! com
[Download RAW message or body]

On December 2, 2010 17:33:52 Pascal Létourneau wrote:
> On December 2, 2010 17:04:45 Milian Wolff wrote:
> > On Thursday 02 December 2010 22:57:58 Pascal Létourneau wrote:
> > > SVN commit 1202961 by pletourn:
> > >
> > > Revert commit 1202930
> > > Instead clear the cache earlier
> > >
> > > CCBUG:157754
> >
> > Hey Pascal,
> >
> > please comment on this patch and explain *why* this is better.
> >
> > a) the assert I added is valid - if offset is zero the
> > thisLine.virtualLine() == virtualCursor.line() assert will *always* be
> > hit. Imo it's much better to assert earlier on the offset.
>
> The assert was placed where offset>0 so it would never have been hit
> If offset is zero the assert (thisLine.virtualLine() ==
> virtualCursor.line()) will *never* be hit
> A zero offfset has a trivial effect, it seems completly wrong to assert on
> it
>
> > b) why should you check m_minLinesVisible with scrollPastEnd - they have
> > no relationship as far as I could understand the code. m_minLinesVisible
> > is only related to "autoCenterLines".
>
> With your patchm, if you have scrollPastEnd == true and m_minLinesVisible >
> 0 the user *can't* scroll past the end
That should be:
With your patch, if you have scrollPastEnd == true and m_minLinesVisible ==
0, the user *can't* scroll past the end

Pascal Létourneau



[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic