@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/fa7025ba9d58fb4680f...

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