Skip to content

fix: Split undo transactions before line breaks, rather than after them - #5992

Open
lhmouse wants to merge 1 commit into
rizonesoft:masterfrom
lhmouse:master
Open

fix: Split undo transactions before line breaks, rather than after them#5992
lhmouse wants to merge 1 commit into
rizonesoft:masterfrom
lhmouse:master

Conversation

@lhmouse

@lhmouse lhmouse commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Previously, an undo transaction was started after a line break, so the line break itself was in the end of the previous transaction, and a subsequent auto indention that was triggered by the line break started a new transaction. Pressing Ctrl+Z undid the auto indention, and pressing Ctrl+Z a second time undid the line break with previous inputs.

In order to be consistent with other editors, a new undo transaction should start after the line break, so they are in the same transaction.


Steps to reproduce

  1. Open empty document.
  2. Turn 'auto indention' and 'split undo transaction at line breaks' on.
  3. Type aa (space space A A).
  4. Press Enter. This moves the cursor to the next line and audo-indent it by two spaces.
  5. Press Ctrl+Z. This undoes the auto indention, but does not undo the line break.
  6. Press Ctrl+Z again, this undoes aa with the line break.

Expected behavior

Like in Visual Studio, step 5 should undo the auto indention and the line break, and step 6 should undo aa (since there's no longer a line break to undo).

Copilot AI lite review requested due to automatic review settings August 31, 2026 14:38

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new newline check dereferences scn->text without a NULL guard, which can lead to a crash if an insert-check notification arrives without text.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adjusts Notepad3’s Scintilla notification handling so undo transaction boundaries are split before a typed line break, aligning undo behavior with common editors by grouping the newline together with the subsequent auto-indentation.

Changes:

  • Move the “split undo transaction on line break” logic from the post-modification linesAdded path into the pre-insert SC_MOD_INSERTCHECK path when the inserted text begins with \r/\n.
  • Remove the old split behavior that triggered after any insertion that increased linesAdded.
File summaries
File Description
src/Notepad3.c Shifts undo-transaction splitting to occur before newline insertion so newline + auto-indent land in the same undo step.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/Notepad3.c
bool const bInUndoRedoStep = (iModType & (SC_PERFORMED_UNDO | SC_PERFORMED_REDO));
if (iModType & SC_MOD_INSERTCHECK) {
if (!bInUndoRedoStep) {
if (Settings.SplitUndoTypingSeqOnLnBreak && (scn->length > 0) && ((scn->text[0] == '\r') || (scn->text[0] == '\n'))) {
Comment thread src/Notepad3.c Outdated
Previously, an undo transaction was started after a line break, so the line break
itself was in the end of the previous transaction, and a subsequent auto indentation
that was triggered by the line break started a new transaction. Pressing Ctrl+Z
undid the auto indentation, and pressing Ctrl+Z a second time undid the line break
with previous inputs.

In order to be consistent with other editors, a new undo transaction should start
after the line break, so they are in the same transaction.

Signed-off-by: LIU Hao <lh_mouse@126.com>
@lhmouse

lhmouse commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

@RaiKoHoff When scn->length > 0, is it possible that scn->text is null? I think this can be omitted.

@hpwamr
hpwamr requested a review from RaiKoHoff August 31, 2026 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants