@techee commented on this pull request.
> @@ -40,42 +40,53 @@ void cmd_goto_right(CmdContext *c, CmdParams *p)
SET_POS(p->sci, pos, TRUE);
}
+static gint doc_line_from_visible_delta(CmdParams *p, gint line, gint delta,
gint *previous)
+{
+ gint step = delta < 0 ? -1 : 1;
+ gint new_line = p->line;
Should be `line` instead of `p->line` - we don't always pass `p->line` as the
parameter of this function.
> /* Calling SCI_LINEUP/SCI_LINEDOWN in a loop for num lines leads to
> visible
* slow scrolling. On the other hand, SCI_LINEUP preserves the value of
* SCI_CHOOSECARETX which we cannot read directly from Scintilla and
which
* we want to keep - perform jump to previous/following line and add
* one final SCI_LINEUP/SCI_LINEDOWN which recovers SCI_CHOOSECARETX
for us. */
- one_above = p->line - p->num - 1;
- if (one_above >= 0 && SSM(p->sci, SCI_GETLINEVISIBLE, one_above, 0))
- {
- /* Every case except for the first line - go one line above and
perform
- * SCI_LINEDOWN. This ensures that even with wrapping on, we
get the
- * caret on the first line of the wrapped line */
- pos = SSM(p->sci, SCI_GETLINEENDPOSITION, one_above, 0);
+
+ if (previous > -1) {
+ guint pos = SSM(p->sci, SCI_POSITIONFROMLINE, previous, 0);
I think you didn't understand the purpose of this strange "one above" and then
`SCI_LINEDOWN` dance. While you get to the correct line alright, you lose the
cursor's `x` position on the line this way. Notice how, when moving cursor up
and down, it remembers the maximum `x` coordinate on the line and even after
bing on lines where the maximum `x` is lower *like e.g. an empty line where `x`
is 0), it returns back to the right position on longer lines.
Unfortunately this "maximum x" is not possible to obtain from Scintilla but
Scintilla automatically recovers it when doing `SCI_LINEDOWN` or `SCI_LINEUP`.
So the trick here is to first go to the line above or below, and then perform
`SCI_LINEDOWN` or `SCI_LINEUP` to get us to the right line and get the `x`
position of the cursor. When you remove this code, you'll always end with the
`x` position at the beginning of the line.
> return;
- /* see cmd_goto_up() for explanation */
- one_above = p->line + num - 1;
- one_above = one_above < last_line ? one_above : last_line - 1;
- pos = SSM(p->sci, SCI_GETLINEENDPOSITION, one_above, 0);
- SET_POS_NOX(p->sci, pos, FALSE);
- SSM(p->sci, SCI_LINEDOWN, 0, 0);
+ new_line = doc_line_from_visible_delta(p, p->line, num, &previous);
+
+ if (previous > -1) {
+ guint pos = SSM(p->sci, SCI_GETLINEENDPOSITION, previous, 0);
+ SET_POS_NOX(p->sci, pos, FALSE);
+ }
+
+ if (new_line > p->line) SSM(p->sci, SCI_LINEDOWN, 0, 0);
Is all this code necessary? Maybe I'm missing something but I did just this:
https://github.com/geany/geany-plugins/pull/1338/commits/fa7025ba9d58fb4680fb47e13bd5c05c6f1f1059#diff-15a3a15a958bc6f85e0f1fae419e23c60bbee47bf617ef133f7539fc1f29b277R128
`doc_line_from_visible_delta()` only returns a line from the valid line range
and when you are on the first line, this is already the "line above" for which
you then perform `SCI_LINEDOWN` so it should be OK on this side. On the other
end of the document when you are on the last line, `SCI_LINEDOWN` just won't do
anything so there's no need for some special handling there.
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany-plugins/pull/1326#pullrequestreview-2045509286
You are receiving this because you are subscribed to this thread.
Message ID: <geany/geany-plugins/pull/1326/review/2045509...@github.com>
