[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-02 22:04:45
Message-ID: 201012022304.48298.mail () milianw ! de
[Download RAW message or body]

[Attachment #2 (multipart/signed)]


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.

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".

So your patch might be valid as well, but mine is imo just as ok and should 
not have been reverted.

Also, if you understand this code, comment it! Explain what the members are 
supposed to represent!

Bye

>  M  +3 -6      kateviewinternal.cpp
> 
> 
> --- trunk/KDE/kdelibs/kate/view/kateviewinternal.cpp #1202960:1202961
> @@ -409,7 +409,7 @@
>    {
>      KTextEditor::Cursor end(doc()->numVisLines() - 1,
> doc()->lineLength(doc()->getRealLine(doc()->numVisLines() - 1)));
> 
> -    if (m_minLinesVisible)
> +    if (m_view->config()->scrollPastEnd())
>        m_cachedMaxStartPos = viewLineOffset(end, -m_minLinesVisible);
>      else
>        m_cachedMaxStartPos = viewLineOffset(end, -(linesDisplayed() - 1));
> @@ -653,13 +653,13 @@
>  {
>    Q_UNUSED(clear_cache)
>    kDebug(13030);
> +  cache()->clear();
> +
>    m_cachedMaxStartPos.setLine(-1);
>    KTextEditor::Cursor max = maxStartPos();
>    if (startPos() > max)
>      scrollPos(max);
> 
> -  cache()->clear ();
> -
>    m_preserveX = true;
>    KTextEditor::Cursor newPos = toRealCursor(toVirtualCursor(m_cursor));
>    KateTextLayout newLine = cache()->textLayout(newPos);
> @@ -1290,9 +1290,6 @@
>    if (forwards) {
>      currentOffset = cache()->lastViewLine(realCursor.line()) -
> cursorViewLine; if (offset <= currentOffset) {
> -      // NOTE: if offset is zero the assert below won't hold, see also:
> -      // https://bugs.kde.org/show_bug.cgi?id=157754
> -      Q_ASSERT(offset);
>        // the answer is on the same line
>        KateTextLayout thisLine = cache()->textLayout(realCursor.line(),
> cursorViewLine + offset); Q_ASSERT(thisLine.virtualLine() ==
> virtualCursor.line());

-- 
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]