<table><tr><td style="">jsalatas created this revision.<br />jsalatas added reviewers: Frameworks, Plasma.<br />jsalatas set the repository for this revision to R39 KTextEditor.<br />Restricted Application added projects: Plasma, Frameworks.<br />Restricted Application added subscribers: kwrite-devel, plasma-devel.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D4538" rel="noreferrer">View Revision</a></tr></table><br /><div><strong>REVISION SUMMARY</strong><div><p>This patch addresses 3 issues</p>

<ol class="remarkup-list">
<li class="remarkup-list-item">Currently if you try to convert the cursor to coordinates and then convert these coordinates back to a cursor again, it produces an invalid cursor (-1, -1) when the original cursor is located at the end of line</li>
</ol>

<ol class="remarkup-list" start="2">
<li class="remarkup-list-item">Converting back to cursor coordinates that have been produced by <tt style="background: #ebebeb; font-size: 13px;">cursorToCoordinate(view->cursorPosition())</tt> and <tt style="background: #ebebeb; font-size: 13px;">coordinatesToCursor(view->cursorPositionCoordinates())</tt> doesn't always produce the same results as the <tt style="background: #ebebeb; font-size: 13px;">includeBorder</tt> option doesn't seem to be used consistently.</li>
</ol>

<ol class="remarkup-list" start="3">
<li class="remarkup-list-item">Although I cannot find a test case for this, it doen't seem to make any sense the line <tt style="background: #ebebeb; font-size: 13px;">scrollPos(max, max.column(), calledExternally);</tt> in <tt style="background: #ebebeb; font-size: 13px;">KateViewInternal::makeVisible</tt> function.   Was it supposed to be that way (ie force when the last line is not empty and not force when it is), or is it just a typo? :\  I would appreciate any reasoning about this!</li>
</ol></div></div><br /><div><strong>TEST PLAN</strong><div><ol class="remarkup-list">
<li class="remarkup-list-item">Create a new app with a single <tt style="background: #ebebeb; font-size: 13px;">KTextEditor::View</tt> widget (similar to kwrite).</li>
</ol>

<ol class="remarkup-list" start="2">
<li class="remarkup-list-item">connect the view's <tt style="background: #ebebeb; font-size: 13px;">cursorPositionChanged</tt> signal with a slot in the window which contains the following code</li>
</ol>

<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">qDebug() << " original cursor" << view->cursorPosition();
QPoint p1 = view->cursorToCoordinate(view->cursorPosition());
KTextEditor::Cursor c1 = view->coordinatesToCursor(p1);
qDebug() << "converted cursor" << c1;
qDebug() << "converted cursor 2" << view->coordinatesToCursor(view->cursorPositionCoordinates());</pre></div>



<ol class="remarkup-list" start="3">
<li class="remarkup-list-item">Run the application and start typing (I typed 52 characters in the first line) and then pressed the <Enter> key sometimes)</li>
</ol>

<ol class="remarkup-list" start="4">
<li class="remarkup-list-item">Press Back Arrow key until you are in the beginning of the document.</li>
</ol>

<p>In steps 3 and 4 the three cursors are the same (this wasn't the case before).</p>

<p>Before you get something like the following</p>

<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">original cursor (0, 1)
converted cursor (-1, -1)
converted cursor 2 (-1, -1)
 original cursor (0, 2)
converted cursor (-1, -1)
converted cursor 2 (-1, -1)
 original cursor (0, 3)
converted cursor (-1, -1)
converted cursor 2 (-1, -1)
 original cursor (0, 4)
converted cursor (-1, -1)
converted cursor 2 (-1, -1)
 original cursor (0, 5)
converted cursor (-1, -1)
converted cursor 2 (-1, -1)
 original cursor (0, 6)
converted cursor (-1, -1)
converted cursor 2 (-1, -1)
 original cursor (0, 7)
converted cursor (-1, -1)
......
 original cursor (0, 48)
converted cursor (-1, -1)
converted cursor 2 (-1, -1)
 original cursor (0, 49)
converted cursor (-1, -1)
converted cursor 2 (-1, -1)
 original cursor (0, 50)
converted cursor (-1, -1)
converted cursor 2 (-1, -1)
 original cursor (0, 51)
converted cursor (-1, -1)
converted cursor 2 (-1, -1)
 original cursor (0, 52)
converted cursor (-1, -1)
converted cursor 2 (-1, -1)
 original cursor (1, 0)
converted cursor (-1, -1)
converted cursor 2 (-1, -1)
 original cursor (2, 0)
converted cursor (-1, -1)
converted cursor 2 (-1, -1)
 original cursor (3, 0)
converted cursor (-1, -1)
converted cursor 2 (-1, -1)
.....
 original cursor (0, 52)
converted cursor (-1, -1)
converted cursor 2 (-1, -1)
 original cursor (0, 51)
converted cursor (0, 51)
converted cursor 2 (-1, -1)
 original cursor (0, 50)
converted cursor (0, 50)
converted cursor 2 (-1, -1)
 original cursor (0, 49)
converted cursor (0, 49)
converted cursor 2 (-1, -1)
 original cursor (0, 48)
converted cursor (0, 48)
converted cursor 2 (-1, -1)
 original cursor (0, 47)
converted cursor (0, 47)
converted cursor 2 (0, 51)
 original cursor (0, 46)
converted cursor (0, 46)
converted cursor 2 (0, 50)
 original cursor (0, 45)
converted cursor (0, 45)
converted cursor 2 (0, 49)
 original cursor (0, 44)
converted cursor (0, 44)
converted cursor 2 (0, 48)
 original cursor (0, 43)
converted cursor (0, 43)
converted cursor 2 (0, 47)
 original cursor (0, 42)
converted cursor (0, 42)
converted cursor 2 (0, 46)
 original cursor (0, 41)
converted cursor (0, 41)
converted cursor 2 (0, 45)
.....
 original cursor (0, 6)
converted cursor (0, 6)
converted cursor 2 (0, 10)
 original cursor (0, 5)
converted cursor (0, 5)
converted cursor 2 (0, 9)
 original cursor (0, 4)
converted cursor (0, 4)
converted cursor 2 (0, 8)
 original cursor (0, 3)
converted cursor (0, 3)
converted cursor 2 (0, 7)
 original cursor (0, 2)
converted cursor (0, 2)
converted cursor 2 (0, 6)
 original cursor (0, 1)
converted cursor (0, 1)
converted cursor 2 (0, 5)
 original cursor (0, 0)
converted cursor (0, 0)
converted cursor 2 (0, 4)</pre></div>

<p>after this patch is applied the converted cursor are consistent with the original (correct) cursor.</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R39 KTextEditor</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D4538" rel="noreferrer">https://phabricator.kde.org/D4538</a></div></div><br /><div><strong>AFFECTED FILES</strong><div><div>src/view/kateview.cpp<br />
src/view/kateviewinternal.cpp</div></div></div><br /><div><strong>EMAIL PREFERENCES</strong><div><a href="https://phabricator.kde.org/settings/panel/emailpreferences/" rel="noreferrer">https://phabricator.kde.org/settings/panel/emailpreferences/</a></div></div><br /><div><strong>To: </strong>jsalatas, Frameworks, Plasma<br /><strong>Cc: </strong>plasma-devel, kwrite-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol<br /></div>