[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 22:33:52
Message-ID: 201012021733.52492.pascal.letourneau () gmail ! com
[Download RAW message or body]

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

Pascal Létourneau

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