Skip to content

Outdent heading markers - #5

Open
mateuszkowalczyk wants to merge 7 commits into
omacom-io:masterfrom
mateuszkowalczyk:outdent_heading_markers
Open

Outdent heading markers#5
mateuszkowalczyk wants to merge 7 commits into
omacom-io:masterfrom
mateuszkowalczyk:outdent_heading_markers

Conversation

@mateuszkowalczyk

Copy link
Copy Markdown

Reason for Changes

I personally find it more visually pleasing to have Markdown heading markers (like ###) outdented, while keeping the actual heading text aligned with regular paragraphs.

Feel free to use my implementation if it aligns with your vision for the app. Here's a screenshot for comparison:

screenshot-2026-08-08_14-35-05

Tests

Here's the list of tests I performed manually:

  • verified that various real-world notes render correctly (copied from my Obsidian vault)
  • resized the window from fullscreen to very small and tested all states in between
  • repeatedly changed the Omarchy monitor scaling
  • verified that Markdown headings above level 6 are not formatted
  • verified undo/redo behavior

Implementation approach

  • I asked GPT-5.6 Sol to implement the initial version
  • Then went through multiple iterations of manual review (done by me) and fixes (done by Sol) until the implementation felt reasonably simple IMO
  • Performed manual testing and made some additional fixes, mostly around undo/redo behavior

@omarchybot

Copy link
Copy Markdown
Collaborator

Reviewed this by driving the real editor on a disposable VM (Qt 6.11.1, offscreen) rather than reading the diff alone. ./bin/test is green: 15 passed, 0 failed.

What holds up. The gutter measures right: with the bundled font at 20px the cell is exactly 12px, and # , ## and ###### all put their heading text at x=84, the same column as body text, with the level-6 marker landing exactly on the block's left edge — level + 1 <= 7 is what keeps a marker on screen. Geometry holds too: across window widths 720/500/394/300/200 at 1x and 2x text scale, x never went negative and the TextEdit never overflowed the Flickable. And Ctrl+Z does not double-fire — the window-scoped Shortcut consumes the key before Keys.onPressed sees it, so one real Ctrl+Z is exactly one step, measured through the window's event pipeline rather than sendEvent.

Three defects, one root cause: setHeadingCellWidth()updateAllBlocksTypography() writes block formats into the document with its undo stack enabled, so a change of desktop text size becomes a user-visible undo command. applyDocumentTypography() deliberately turns undo off first; this path does not.

1. Redo is silently discarded, and the text is unrecoverable. Type hello, press Ctrl+Z, then change the desktop text size. Qt drops the redo branch the moment a new edit is recorded, so isRedoAvailable() flips true → false and Ctrl+Shift+Z gives back nothing.

2. One undo strips the document's typography. Open a document, change the desktop text size, press Ctrl+Z once. replayHistory consumes the format command looking for a text change, finds none, and stops — text untouched, every block back at the old margin (84 where the gutter is now 168). It never repairs itself: edit the body block afterwards and that block goes to 168 while the heading stays at 84, so the document ends up with mixed margins block by block.

3. Once any block is stale, one redo replays several edits. From the state in 2, make two cursor-separated edits and undo both — a single redo replays both, because documentHasExpectedTypography() scans the whole document and an untouched block keeps failing the check. A control run without the size change replays them one at a time, correctly.

I have not pushed a fix for these. Every option I can see is a design decision rather than a bug fix: disabling undo around the width change clears the entire stack instead of just the redo branch, and the alternative is keeping the gutter out of block formats altogether. That is your call and the maintainer's, not mine to make in your branch.

Two smaller things, both verified:

blockFormatWithTypography() indents by heading.level + 1 rather than the prefix width headingMarkup() already found, so a marker followed by more than one space drifts right. With the bundled font: ## Heading puts its text at 84 (aligned), ## Heading at 95.9, ## Heading at 107.9, ##\tHeading at 128. Worth saying that prefixLength is not a drop-in replacement — it can exceed 7 cells, which is exactly what would push a marker off the left edge.

At 720px with 3x text scale the body column is no longer centred: x clamps to 0, leaving 252px of space on the left against 60px on the right. laysOutAndEditsHeadings asserts only x >= 0 and containment, so it passes on that layout.

Finally, a note rather than a defect: the new StandardKey.Undo/Redo branches in Keys.onPressed never run in a real window, because the Shortcut gets there first. They are the only path undoingHeadingDoesNotExposeTypographyStep exercises, since QCoreApplication::sendEvent(editor, ...) bypasses the shortcut map — so the route users actually take is the one the test does not cover. It does work; it just is not what is being asserted.

Heads-up on overlap you did not cause: #12 also edits smartReturn immediately above your refreshCursorAfterHeading() call, and #18 fixes narrow-window clipping in the same editorWidth/x bindings you rewrote. Whichever lands second will need a rebase.

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