fix: keep cursor on the last line when output has no trailing newline

src/cursor-helpers.ts

@@ -19,18 +19,28 @@ export const cursorPositionChanged = (
 ): boolean => a?.x !== b?.x || a?.y !== b?.y;
 
 /**
-Build escape sequence to move cursor from bottom of output to the target position and show it.
-Assumes cursor is at (col 0, line visibleLineCount) — i.e. just after the last output line.
+Build escape sequence to move cursor from the bottom of the output to the target position and show it.
+
+`bottomLine` is the row the renderer left the cursor on, counted from the top of the output.
+That is always `lines.length - 1` for `lines = str.split('\n')`, whether or not the output ends
+with a newline:
+
+- With a trailing newline, `split` yields one extra empty element and the renderer stops just
+  past the last visible line — which is `lines.length - 1`.
+- Without one, there is no extra element and the renderer deliberately stops on the last visible
+  line instead of moving past it — which is also `lines.length - 1`.
+
+This is the same row basis `buildReturnToBottom` measures from, so the two stay in step.
 */
 export const buildCursorSuffix = (
-	visibleLineCount: number,
+	bottomLine: number,
 	cursorPosition: CursorPosition | undefined,
 ): string => {
 	if (!cursorPosition) {
 		return '';
 	}
 
-	const moveUp = visibleLineCount - cursorPosition.y;
+	const moveUp = bottomLine - cursorPosition.y;
 	return (
 		(moveUp > 0 ? ansiEscapes.cursorUp(moveUp) : '') +
 		ansiEscapes.cursorTo(cursorPosition.x) +
@@ -50,8 +60,9 @@ export const buildReturnToBottom = (
 		return '';
 	}
 
-	// PreviousLineCount includes trailing newline, so visible lines = previousLineCount - 1
-	// cursor is at previousCursorPosition.y, need to go to line (previousLineCount - 1)
+	// PreviousLineCount is the raw `split('\n')` length, so `previousLineCount - 1`
+	// is the row the cursor was left on regardless of a trailing newline — the same
+	// basis `buildCursorSuffix` takes as its `bottomLine`.
 	const down = previousLineCount - 1 - previousCursorPosition.y;
 	return (
 		(down > 0 ? ansiEscapes.cursorDown(down) : '') + ansiEscapes.cursorTo(0)
@@ -62,13 +73,15 @@ export type CursorOnlyInput = {
 	cursorWasShown: boolean;
 	previousLineCount: number;
 	previousCursorPosition: CursorPosition | undefined;
-	visibleLineCount: number;
 	cursorPosition: CursorPosition | undefined;
 };
 
 /**
 Build the escape sequence for cursor-only updates (output unchanged, cursor moved).
 Hides cursor if it was previously shown, returns to bottom, then repositions.
+
+`buildReturnToBottom` has just placed the cursor on row `previousLineCount - 1`, so the
+suffix measures from there rather than recomputing the row from the output.
 */
 export const buildCursorOnlySequence = (input: CursorOnlyInput): string => {
 	const hidePrefix = input.cursorWasShown ? hideCursorEscape : '';
@@ -77,7 +90,7 @@ export const buildCursorOnlySequence = (input: CursorOnlyInput): string => {
 		input.previousCursorPosition,
 	);
 	const cursorSuffix = buildCursorSuffix(
-		input.visibleLineCount,
+		input.previousLineCount - 1,
 		input.cursorPosition,
 	);
 	return hidePrefix + returnToBottom + cursorSuffix;

src/log-update.ts

@@ -72,16 +72,14 @@ const createStandard = (
 		}
 
 		const lines = str.split('\n');
-		const visibleCount = visibleLineCount(lines, str);
-		const cursorSuffix = buildCursorSuffix(visibleCount, activeCursor);
+		const cursorSuffix = buildCursorSuffix(lines.length - 1, activeCursor);
 
 		if (str === previousOutput && cursorChanged) {
 			stream.write(
 				buildCursorOnlySequence({
 					cursorWasShown,
 					previousLineCount,
 					previousCursorPosition,
-					visibleLineCount: visibleCount,
 					cursorPosition: activeCursor,
 				}),
 			);
@@ -151,9 +149,7 @@ const createStandard = (
 		}
 
 		if (activeCursor) {
-			stream.write(
-				buildCursorSuffix(visibleLineCount(lines, str), activeCursor),
-			);
+			stream.write(buildCursorSuffix(lines.length - 1, activeCursor));
 		}
 
 		previousCursorPosition = activeCursor ? {...activeCursor} : undefined;
@@ -224,7 +220,6 @@ const createIncremental = (
 					cursorWasShown,
 					previousLineCount: previousLines.length,
 					previousCursorPosition,
-					visibleLineCount: visibleCount,
 					cursorPosition: activeCursor,
 				}),
 			);
@@ -240,7 +235,10 @@ const createIncremental = (
 		);
 
 		if (str === '\n' || previousOutput.length === 0) {
-			const cursorSuffix = buildCursorSuffix(visibleCount, activeCursor);
+			const cursorSuffix = buildCursorSuffix(
+				nextLines.length - 1,
+				activeCursor,
+			);
 			stream.write(
 				returnPrefix +
 					ansiEscapes.eraseLines(previousLines.length) +
@@ -297,7 +295,7 @@ const createIncremental = (
 			);
 		}
 
-		const cursorSuffix = buildCursorSuffix(visibleCount, activeCursor);
+		const cursorSuffix = buildCursorSuffix(nextLines.length - 1, activeCursor);
 		buffer.push(cursorSuffix);
 
 		stream.write(buffer.join(''));
@@ -354,9 +352,7 @@ const createIncremental = (
 		}
 
 		if (activeCursor) {
-			stream.write(
-				buildCursorSuffix(visibleLineCount(lines, str), activeCursor),
-			);
+			stream.write(buildCursorSuffix(lines.length - 1, activeCursor));
 		}
 
 		previousCursorPosition = activeCursor ? {...activeCursor} : undefined;

test/cursor-helpers.tsx

@@ -84,7 +84,6 @@ test('buildCursorOnlySequence - builds full sequence with hide prefix when curso
 		cursorWasShown: true,
 		previousLineCount: 2,
 		previousCursorPosition: {x: 0, y: 0},
-		visibleLineCount: 1,
 		cursorPosition: {x: 3, y: 0},
 	});
 	const expected =
@@ -99,7 +98,6 @@ test('buildCursorOnlySequence - no hide prefix when cursor was not shown', t =>
 		cursorWasShown: false,
 		previousLineCount: 0,
 		previousCursorPosition: undefined,
-		visibleLineCount: 1,
 		cursorPosition: {x: 3, y: 0},
 	});
 	t.false(result.startsWith(hideCursorEscape));

test/cursor.tsx

@@ -644,3 +644,160 @@ test.serial(
 		unmount();
 	},
 );
+
+// Fullscreen frames are the only ones Ink renders without a trailing newline,
+// which is what makes the cursor suffix measure from the last visible line
+// rather than from one row past it. These drive that through the real
+// `render()` wiring — `outputToRender = isFullscreen ? output : output + '\n'`
+// — instead of handing log-update a hand-built string.
+
+const fullscreenLines = (count: number, marker: string): string[] =>
+	Array.from({length: count}, (_, index) =>
+		index === 1 ? `Line ${index}${marker}` : `Line ${index}`,
+	);
+
+function FullscreenCursorApp({
+	lineCount,
+	cursorY,
+	marker,
+}: {
+	readonly lineCount: number;
+	readonly cursorY: number;
+	readonly marker: string;
+}) {
+	const {setCursorPosition} = useCursor();
+	setCursorPosition({x: 3, y: cursorY});
+
+	return (
+		<Box flexDirection="column">
+			{fullscreenLines(lineCount, marker).map(line => (
+				<Text key={line}>{line}</Text>
+			))}
+		</Box>
+	);
+}
+
+// Both renderers need covering here. The trailing newline is omitted for
+// fullscreen in either mode, but only the incremental renderer skips the final
+// cursorNextLine to keep the cursor on the last line, so it is the one where
+// the row basis is easiest to get wrong.
+const inkRenderingModes = [
+	{name: 'standard rendering', incremental: false},
+	{name: 'incremental rendering', incremental: true},
+] as const;
+
+for (const {name, incremental} of inkRenderingModes) {
+	test.serial(
+		`${name} - fullscreen: cursor lands on the requested row across rerender and cursor-only update`,
+		async t => {
+			const stdout = createStdout();
+			// Output that exactly fills the viewport is fullscreen, so Ink omits
+			// the trailing newline and the renderer stops on the last visible line.
+			(stdout as any).rows = 5;
+
+			const {rerender, unmount, waitUntilRenderFlush} = render(
+				<FullscreenCursorApp lineCount={5} cursorY={2} marker="" />,
+				{stdout, incrementalRendering: incremental},
+			);
+			await waitUntilRenderFlush();
+
+			// 5 lines with no trailing newline: the cursor is left on row 4, so
+			// reaching y=2 is cursorUp(2). Measuring from the visible-line count
+			// instead would emit cursorUp(3) and land a row too high.
+			const expected =
+				ansiEscapes.cursorUp(2) + ansiEscapes.cursorTo(3) + showCursorEscape;
+			const overshoot =
+				ansiEscapes.cursorUp(3) + ansiEscapes.cursorTo(3) + showCursorEscape;
+
+			const firstRender = getWriteCalls(stdout).join('');
+			t.true(firstRender.includes(expected), 'first frame');
+			t.false(
+				firstRender.includes(overshoot),
+				'first frame does not overshoot',
+			);
+
+			const writesBeforeRerender = (stdout.write as any).callCount as number;
+			rerender(<FullscreenCursorApp lineCount={5} cursorY={2} marker="!" />);
+			await waitUntilRenderFlush();
+
+			const changedRerender = getWriteCalls(stdout)
+				.slice(writesBeforeRerender)
+				.join('');
+			t.true(changedRerender.includes('Line 1!'), 'content actually changed');
+			t.true(changedRerender.includes(expected), 'changed rerender');
+			t.false(
+				changedRerender.includes(overshoot),
+				'changed rerender does not overshoot',
+			);
+
+			const writesBeforeCursorMove = (stdout.write as any).callCount as number;
+			rerender(<FullscreenCursorApp lineCount={5} cursorY={0} marker="!" />);
+			await waitUntilRenderFlush();
+
+			// Output is unchanged, so this takes the cursor-only path, which derives
+			// the bottom row from previousLineCount (5) rather than from the output.
+			// Not on Windows: fullscreen frames there always take the clearing
+			// path instead, so the expected sequence comes from sync(). It works
+			// out to the same bytes, because both measure from lines.length - 1.
+			const cursorOnly = getWriteCalls(stdout)
+				.slice(writesBeforeCursorMove)
+				.join('');
+			t.true(
+				cursorOnly.includes(
+					ansiEscapes.cursorUp(4) + ansiEscapes.cursorTo(3) + showCursorEscape,
+				),
+				'cursor-only update',
+			);
+			t.false(
+				cursorOnly.includes(
+					ansiEscapes.cursorUp(5) + ansiEscapes.cursorTo(3) + showCursorEscape,
+				),
+				'cursor-only update does not overshoot',
+			);
+
+			unmount();
+		},
+	);
+}
+
+// Both renderers again: `sync()` is a separate implementation in each, so
+// identical behaviour today is not a reason to leave one of them untested.
+for (const {name, incremental} of inkRenderingModes) {
+	test.serial(
+		`${name} - fullscreen: cursor lands on the requested row on the sync path`,
+		async t => {
+			const stdout = createStdout();
+			(stdout as any).rows = 5;
+
+			// Output taller than the viewport is still fullscreen, and the second
+			// such frame clears the terminal and repositions through log.sync()
+			// rather than through the renderer's normal write path.
+			const {rerender, unmount, waitUntilRenderFlush} = render(
+				<FullscreenCursorApp lineCount={6} cursorY={2} marker="" />,
+				{stdout, incrementalRendering: incremental},
+			);
+			await waitUntilRenderFlush();
+
+			const writesBeforeRerender = (stdout.write as any).callCount as number;
+			rerender(<FullscreenCursorApp lineCount={6} cursorY={2} marker="!" />);
+			await waitUntilRenderFlush();
+
+			const synced = getWriteCalls(stdout).slice(writesBeforeRerender).join('');
+			t.true(synced.includes(ansiEscapes.clearTerminal), 'took the sync path');
+			// 6 lines with no trailing newline: the cursor is left on row 5, so y=2
+			// is cursorUp(3), not the cursorUp(4) a visible-line-count basis gives.
+			t.true(
+				synced.includes(
+					ansiEscapes.cursorUp(3) + ansiEscapes.cursorTo(3) + showCursorEscape,
+				),
+			);
+			t.false(
+				synced.includes(
+					ansiEscapes.cursorUp(4) + ansiEscapes.cursorTo(3) + showCursorEscape,
+				),
+			);
+
+			unmount();
+		},
+	);
+}