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

In vimode/src/cmds/motion.c:

>  	/* 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.

In vimode/src/cmds/motion.c:

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

fa7025b#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, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.