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

List:       kwrite-devel
Subject:    Re: KDE/kdelibs/kate/view
From:       Milian Wolff <mail () milianw ! de>
Date:       2010-12-03 10:41:43
Message-ID: 201012031141.43400.mail () milianw ! de
[Download RAW message or body]

[Attachment #2 (multipart/signed)]


On Friday 03 December 2010 11:24:27 Milian Wolff wrote:
> On Thursday 02 December 2010 23: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
> 
> True, but just below (!forwards) the same assert is done and the offset
> should be tested there, since otherwise it will crash on the line-assert.
> Maybe an offset of zero should be handled gracefully in viewLineOffset?
> 
> > > 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

The contrary is the case: If scrollPastEnd == false and m_minLinesVisible 
(i.e. auto-center-lines = true), then the user *can* scroll past the end ;-)

I still don't get why m_minLinesVisible is related to scrollPastEnd...

I'll test your patch now, thanks. The offset = 0 case should still be discussed 
/ handled in viewLineOffset though.

Bye
-- 
Milian Wolff
mail@milianw.de
http://milianw.de

["signature.asc" (application/pgp-signature)]

_______________________________________________
KWrite-Devel mailing list
KWrite-Devel@kde.org
https://mail.kde.org/mailman/listinfo/kwrite-devel


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