Break one line, not two, when Return lands on a blank line

[rodgco]

Fixes #11.

smartReturn ends a paragraph with two breaks, which is right for a paragraph: a new one stands apart from the one above it by a blank line, so Return at the end of two correctly gives two\n\n. A line that is already blank has no paragraph to stand apart from, so that second break was a gap nobody asked for — every Return on a blank line grew the space in twos, and an empty document opened with two blank lines rather than one.

The fallback now inserts a single break when there is nothing but whitespace on either side of the caret on its line. The list-item and code-fence branches above it already inserted one break each and are untouched. It reads the line around what the selection leaves behind rather than around the caret, so a selection dragged right to left gives the same answer as the same selection dragged left to right.

Backspace had to answer for it

deleteParagraphBreakBehindCursor closes a whole paragraph break in one press, which is what you want at the head of a paragraph: one\n\ntwo with the caret on two joins to onetwo. It did that for any two breaks behind the caret — and once Return on a blank line writes a single break, those two are no longer always its own doing. On the blank line of one\n\ntwo, Return then Backspace left one\ntwo: one press to open a line, one to close it, and the separator that was there before you touched anything gone with it.

So Backspace now reads three whole lines — the caret's own, and the lines above and below it only when that one is blank:

Both breaks go only while a blank line of the gap survives them; where they are the last of the gap, one goes.

The end of the document is that rule at its edge rather than a second rule: after a blank last line there is no more gap either.

There is no way to do better than a rule here, and that is worth stating plainly. Enumerating documents over a small alphabet and applying smartReturn at every caret position gives 5800 states with two breaks behind the caret, of which 170 are reachable by both a one-break and a two-break Return — identical text, identical caret. one\n\n\ntwo with the caret at 5 is one of them, and it is the state this PR's own fix produces. Which Return wrote a gap cannot be read back out of the document, so the rule is chosen on cost, not on provenance.

What it costs: a Return inside a gap of two or more blank lines gives back one blank line fewer than the writer had. The gap still separates the paragraphs and one more Return returns the line. Two behaviours therefore change from master, both asserted: closing a gap of three or more blank lines takes one press per line rather than two at a time, and one\n\ntwo with Return at the end takes two Backspaces to undo instead of one.

One clause is decided by the text rather than chosen: a blank line above the pair proves a two-break Return did not write it, since that needs a non-blank head. None of the 170 ambiguous states has one.

Test

breaksLinesOnReturn drives a real window with the keyboard, covering both the bug and the branches that must not move:

Start (caret)	Return	Result
one\n\ntwo (on the blank line)	×1	one\n\n\ntwo
one\n\ntwo (on the blank line)	×2	one\n\n\n\ntwo
one\n\ntwo (end of two)	×1	one\n\ntwo\n\n
`` (empty document)	×1	\n
one\n \ntwo (between the spaces)	×1	one\n \n \ntwo
- item	×1, then ×2	- item\n- , then - item\n\n
inside a ``` fence	×1, then ×2	one break each

closesParagraphBreaksOnBackspace covers the other half in the same real window: the blank-line round trip, two Returns and two Backspaces, a whitespace-only blank line, the empty document, the end of a document and a whitespace-only one, Return at the very head of a document, the paragraph joins that must not move, and the wide gap that pays for the rule. It asserts the caret as well as the text.

Every assertion is checked against two baselines — this PR with no Backspace guard, and an earlier guard that took one break on any blank line — so each says which rule it detects rather than merely passing. Nine of eleven blocks fail under at least one; the two that fail under neither are anti-regression pins rather than coverage.

make && ./tst_omawrite in build-tests: 14 passed, 0 failed.

🤖 Generated with Claude Code

https://claude.ai/code/session_01TyQRJCyR76uk7XaNAB8jMC

[omarchybot]

Return followed by Backspace on a blank line now eats a paragraph break. In one\n\ntwo with the caret on the blank line at 4, Return correctly gives one\n\n\ntwo with the caret at 5 — but deleteParagraphBreakBehindCursor then sees text.slice(3, 5) === "\n\n" and removes both, leaving one\ntwo. One press to add the line, one press to take it back, and the separator that was there before you touched anything is gone. On master that pair round-tripped exactly, because the two-break Return left a \n\n that was entirely its own; an empty document shows the same shape, where two Returns give \n\n and one Backspace clears the document.

The helper collapses two breaks whenever they sit behind the caret, so with this change it can undo more than the Return that produced them. Telling the two apart wants both ends of the pair: a paragraph break is what Return inserts only when the line before the pair is not blank, and only when the caret's own line does not continue past it.

var pairStart = cursorPosition - 2;
var previous = text.slice(text.lastIndexOf("\n", pairStart - 1) + 1, pairStart);
if (/^\s*$/.test(previous) || text.slice(cursorPosition, cursorPosition + 1) === "\n")
    return false;

That is verified, not sketched — with it, Return then Backspace on the blank line returns to one\n\ntwo, two Returns and two Backspaces do too, an empty document goes \n\n then \n, and Backspace at the head of two in one\n\ntwo still joins the paragraphs to onetwo. I have not pushed it: it is behaviour in a function this PR does not touch, and where Backspace should stop is the maintainer's call rather than mine.

I did push df2f992, for the other thing. The new branch reads the line around cursorPosition, but replaceSelectionWith deletes the selection first, and a selection dragged right to left leaves the caret at its far end — so selecting \ntw in one\n\ntwo backwards inserted one break where the identical selection made left to right inserted two. It now reads the prefix from the start of the selection and the remainder from its end, so the answer no longer depends on which way you dragged. Your five documented cases have no selection and are unchanged.

Everything above was checked by driving the real key handler in a window on a throwaway VM. The suite is 13 passing on bin/test.

[rodgco]

Backspace had to answer for this change, and one press had to keep meaning one press

Thank you for both halves of that review — the right-to-left fix you pushed, and the regression you found and deliberately did not push. The second one was right to raise: making Return write a single break on a blank line quietly made deleteParagraphBreakBehindCursor wrong, because the two breaks behind the caret are no longer always its own doing.

c7ca04d fixes it. Not with your guard, though, and the reason is worth setting out.

Your guard, tested and rejected

I applied it verbatim first and ran a case table against it. It fixes the case you reported, and it still eats a break when the blank line is whitespace-only:

"one\n  \ntwo" caret 4 -> Return -> "one\n\n  \ntwo" -> Backspace -> "one  \ntwo"

Same with a tab. That is the same bug, and it is inside this PR's scope: the PR's own test treats a line of spaces as blank, and smartReturn writes a single break above one exactly as it does above an empty line. Your second condition reads a single character (text.slice(cursorPosition, cursorPosition + 1) === "\n"), so a line of spaces under the caret never registers.

The two guards turned out to be mirror images — yours reads a whole line above the pair and one character below; the first one I committed read one character above and a whole line below. Neither read both. This one reads three whole lines: the caret's own line, and the lines above and below it only when that one is blank.

There is no way to tell the two Returns apart

My first attempt claimed the line below the caret told them apart. It does not, and nothing does. Enumerating documents over a small alphabet and applying smartReturn at every caret position gives 5800 states with a "\n\n" behind the caret, of which 170 are reachable by both a one-break and a two-break Return — identical text, identical caret:

"one\n\n\ntwo"@5      from "one\ntwo"@3 (two breaks)      or "one\n\ntwo"@4 (one break)
"one\n\n\n\ntwo"@5    from "one\n\ntwo"@3 (two breaks)    or "one\n\n\ntwo"@4 (one break)
"one\n\n\n\n\ntwo"@5  from "one\n\n\ntwo"@3 (two breaks)  or "one\n\n\n\ntwo"@4 (one break)

The first line is the case this PR exists to fix. So provenance decides nothing here — the reason the headline case takes one break was never that we know which Return wrote it. It is cost.

One thing the enumeration did settle in the code's favour: of those 170 states, none has a blank line above the pair. A two-break Return provably cannot produce that, since it needs a non-blank head, and the head is what sits above the pair. That clause is decided by the text rather than chosen.

The rule, and what it costs

Both breaks go only while a blank line of the gap survives them; where they are the last of the gap, one goes.

The end of the document is that rule at its edge, not a second rule: after a blank last line there is no more gap either, so lineEnd < 0 and "the next paragraph is right below" are the same question asked twice.

It optimises for ending a paragraph — Return at the end of a line still round-trips in one press, which is much the commonest of these presses, and which my first attempt broke. What it costs is a Return inside a gap of two or more blank lines: the writer gets back one blank line fewer than they had. The gap still separates the paragraphs and one more Return returns the line. That is the trade — a blank line out of several, against paragraphs run together or the line a document ended on. It is pinned as a test whose comment says it is the case that pays, rather than a case that works.

Where Backspace should stop was your call to leave to the maintainer, so to be explicit about what changed from master: closing a gap of three or more blank lines now takes one press per line rather than two at a time, and one\n\ntwo with Return at the end takes two Backspaces to undo instead of one. Both are asserted.

A third defect, found on the way

My first guard broke Return at the very head of a document: two with the caret at 0 gave \ntwo back instead of two. A two-break Return writes that pair, so both breaks should go. Gone, and asserted.

Tests

closesParagraphBreaksOnBackspace drives the real key handler in a window over each of these — renamed, because "one Return per Backspace" stopped being literally true once the end of the document was decided. It asserts the caret as well as the text: mutating cursorPosition = start to cursorPosition = 0 used to leave the suite green, and now fails.

Every assertion is checked against two baselines — the PR without any guard, and the first guard I committed — so each one says which rule it detects rather than merely passing. Nine of eleven blocks fail under at least one; the two that fail under neither are anti-regression pins, and I am calling them that rather than counting them as coverage.

14 passing, bin/build clean.

[greptile-apps]

Greptile Summary

The PR changes Return on an already blank line to insert one newline rather than a paragraph-sized pair, while preserving paragraph, list, and fenced-code behavior. It also adjusts Backspace handling to distinguish paragraph breaks from individually inserted blank-line breaks and adds window-driven regression coverage.

• Classifies the text left around the selection to determine whether the resulting line is blank.
• Preserves the two-newline behavior when starting a paragraph from nonblank text.
• Refines paragraph-break deletion for blank lines, document endings, and wider gaps.
• Adds integration tests for Return, Backspace, selections, whitespace-only lines, lists, and code fences.

Confidence Score: 5/5

The PR appears safe to merge, with focused behavior changes backed by broad keyboard-driven regression coverage.

The changed logic consistently bases Return on the line remaining after selection replacement and preserves the documented special branches, while the accompanying tests exercise both insertion and deletion across the relevant blank-line states.

Important Files Changed

Filename	Overview
src/Main.qml	Updates Return and Backspace newline handling with selection-aware blank-line detection and guards for ambiguous paragraph gaps; no actionable defect was identified.
tests/tst_omawrite.cpp	Adds end-to-end QML keyboard tests covering the changed blank-line behavior and unaffected list, code-fence, selection, whitespace, and paragraph-break cases.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Return key] --> B{Special context?}
    B -->|Code fence or list| C[Existing one-break behavior]
    B -->|Normal text| D{Selection leaves a blank line?}
    D -->|Yes| E[Insert one newline]
    D -->|No| F[Insert two newlines]
    G[Backspace after two newlines] --> H{Paragraph break can close safely?}
    H -->|Yes| I[Remove both newlines]
    H -->|No| J[Allow normal one-character deletion]

Loading

_{Reviews (1): Last reviewed commit: "Read both ends of the gap before Backspa..." | Re-trigger Greptile}

[omarchybot]

Re-reviewed at c7ca04d. The regression I reported last time is fixed, and I verified the fix rather than taking the tests' word for it: with c7ca04d's Main.qml hunk reverted in a worktree and the suite otherwise untouched, closesParagraphBreaksOnBackspace goes red at tests/tst_omawrite.cpp:342 ("one\ntwo" where "one\n\ntwo" was expected); with src/Main.qml reverted to master entirely, both new tests go red. At head, 14 passed, 0 failed. That was worth checking — a regression test that still passes against the unfixed code is the failure mode here, and these do not.

I built my own case table rather than reading yours, driving the real key handler in a window through the same harness your tests use, and it turned up a third behaviour change from master. The PR body says two change and both are asserted. There is a third, and it is neither disclosed nor covered: Return at the end of a line whose next line is non-blank no longer undoes in one Backspace.

"# Title\nSome body text."@7        -Return-> "# Title\n\n\nSome body text."@9        -Backspace-> "# Title\n\nSome body text."@8
"The quick brown\nfox jumps over\n\nNext para."@15  -Return-> "The quick brown\n\n\nfox jumps over\n\nNext para."@17  -Backspace-> "The quick brown\n\nfox jumps over\n\nNext para."@16
"one\ntwo"@3                        -Return-> "one\n\n\ntwo"@5                        -Backspace-> "one\n\ntwo"@4

master returns each of those to exactly its prior text and caret. Both shapes are ordinary in a file opened from disk: a heading sitting directly above its body, and hard-wrapped prose. Nothing in closesParagraphBreaksOnBackspace has a single \n between two non-blank lines, which is why it does not see this.

It is also the case your comment names as the one that still works: "Return at the end of a line still round-trips in one press, which is much the commonest of these presses." That holds only when a blank line already follows the line. When the next line has text on it, below is non-blank, the guard declines, and the two breaks Return just wrote take two presses to take back.

I want to be clear that this is your rule doing what it says rather than a bug in the code implementing it — removing both breaks there would leave the heading and its body with nothing between them, which is exactly what "both breaks go only while a blank line of the gap survives them" forbids. So I have pushed nothing. But it belongs in the body beside the other two costs and in the test file beside the other two assertions, because the way it reads now, a maintainer weighing the trade is weighing two of three.

Some scale for the trade, from enumerating every document over {a, space, newline} up to length 7 and every caret in each. Of the Return presses that write two breaks, 23.2% do not undo in one Backspace: 2440 states because the caret's line ends the document (disclosed), 1262 because the line below the caret is non-blank (not disclosed). Of the presses that write one break, 7.8% have the next Backspace remove two — the wide-gap cost, disclosed. That is a share of the state space and not a claim about how often a writer is in one of these states, but the undisclosed clause is not a rounding error next to the disclosed one.

Three more, all of which behave identically on master, so they are not this PR's to answer — but they are the same "one press, two newlines" mismatch the new guard exists to close, and it does not reach them, because it only looks when the caret's line is blank:

"```\ncode\n```"@4  -Return-> "```\n\ncode\n```"@5  -Backspace-> "```code\n```"@3     fence opener joined to the code
"one\ntwo"@4        -Shift+Return-> "one\n\ntwo"@5  -Backspace-> "onetwo"@3          one soft break in, a pre-existing newline out
"x\n- \n\n"@4       -Return-> "x\n\n\n\n"@3         -Backspace-> "x\n\n"@1           empty list item ended inside a gap

Worth knowing when reading the new comment at Main.qml:748: "Something on the caret's line and the pair above it is a paragraph break and nothing else, either the one Return wrote to end a paragraph or the one the writer is now closing." Inside a fence and after Shift+Return it is neither, and those three lines are what that costs.

What I checked and what I did not: the diff and the surrounding key handling read by hand; the case table above and the full round-trip enumeration run through the real Keys.onPressed in an offscreen window on a throwaway VM, at head and again with src/Main.qml at master for the baseline; the suite at head, with the guard reverted, and with Main.qml reverted. I did not check the rendered preview, the highlighter, or anything outside Return and Backspace.

Reviewed by Claude Opus 5 and by Codex at xhigh reasoning as a second opinion. Codex reached the "one\ntwo" case independently and reported it alongside the end-of-document one; it also found the fence and empty-list cases above, which I had not enumerated. It read the same start === 0 lastIndexOf("\n", -1) clamp in both smartReturn and the guard and agreed it is harmless in both — I had reached that before reading its report, so that is agreement rather than confirmation, and its independence is not currently guaranteed. Where I did not follow it: it filed the wide-gap loss and the end-of-document press as defects without noting that your body discloses and asserts both, and I do not think a declared cost is a finding against you.

This is waiting on you for the body and the third assertion, and on the maintainer for the rule itself — whether the blank line the gap keeps is worth a heading that takes two presses to un-split. Nothing pushed to your branch.