Skip to content

Add a Ctrl+Shift+X strikethrough shortcut - #15

Open
joshualambert wants to merge 2 commits into
omacom-io:masterfrom
joshualambert:strikethrough
Open

Add a Ctrl+Shift+X strikethrough shortcut#15
joshualambert wants to merge 2 commits into
omacom-io:masterfrom
joshualambert:strikethrough

Conversation

@joshualambert

Copy link
Copy Markdown

Adds a strikethrough shortcut alongside the existing bold/italic ones.

  • Ctrl+Shift+X wraps the selection in ~~ markers (the shortcut Slack, VS Code, and Obsidian use).
  • The highlighter renders ~~text~~ struck through and hides the markers exactly like ** and *, so the caret skips over them too.
  • The formatting shortcuts now toggle: pressing Ctrl+B, Ctrl+I, or Ctrl+Shift+X on already-wrapped text (or with the caret inside the markers) removes the markers instead of stacking another pair. The wrap/unwrap logic moved into EditorMutations.js so it's covered by the test harness.
  • Shortcut added to the Ctrl+? reference and the README.

Two new tests (findsStrikethroughRanges, togglesWrappedSelection); all 14 pass.

Strikethrough in Omawrite

Wrap the selection in ~~ markers, render the content struck through,
and hide the markers like the other inline markup. The formatting
shortcuts now also toggle: pressing Ctrl+B, Ctrl+I, or Ctrl+Shift+X
again removes the markers instead of stacking another pair.
@nalyDzzz

Copy link
Copy Markdown

Great idea, this would help me out too!

`*` is also the first half of `**`, so toggleWrap's unwrap branches matched the inner asterisk of a bold pair: Ctrl+I on `bold` selected inside `**bold**` rewrote it to `*bold*`, silently deleting the bold instead of adding italic. Both unwrap branches now refuse when the character just outside the marker repeats it, falling through to the wrap path and producing `***bold***`, which is what Ctrl+I did before this shortcut moved to toggleWrap.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@omarchybot

Copy link
Copy Markdown
Collaborator

Reviewed this and pushed one fix to the branch (d98a378). Moving the wrap logic into EditorMutations.js was the right call — it is where the other mutations live, and it is what made the bug below reproducible in a test.

Fixed: "*" is the first half of "**", so both unwrap branches mistook bold delimiters for italic ones. With **bold** in the buffer, Ctrl+I over a selection of bold produced *bold*, and over a selection of the whole **bold** also produced *bold* — either way the bold is silently deleted by a keystroke pressed to add italic. That is a regression against master, which produced ***bold***, and it is not confined to the new shortcut: it changes what Ctrl+I does to documents people already have. Both branches now refuse when the character just outside the marker repeats it, falling through to the wrap path. I extended togglesWrappedSelection rather than adding a parallel test; with the fix reverted it fails with *bold* against ***bold***.

Still open:

  • A strikethrough and a link can claim the same span, and the link wins. inlineMarkup returns entries in source order and highlightInline calls setFormat for each, which overwrites rather than merges, so for ~~[a link](url)~~ the list is [0] Strikethrough content=(2,13) then [1] Link content=(3,6) and the link is painted second. The result renders identically to a plain [a link](url) — no strike, and the ~~ hidden, so nothing on screen says the text is struck. That is exactly what you get by selecting a link and pressing Ctrl+Shift+X, so it is the first thing someone will try. The mirror case [~~a link~~](url) fails the other way: the tildes show literally while hiddenRangesAt still tells the caret to skip them, so the caret jumps over characters you can see. The same pathology already affects [**a**](url) on master, so this is a new instance of an existing problem rather than a new one — but resolving it means deciding how overlapping inline markup should compose, which is a bigger change than this PR, so I left it alone.
  • (~~)(.+?)(~~) takes delimiter pairs out of longer tilde runs: ~~~not~~~ strikes ~not and leaves a stray visible ~, and ~~~~~ reads as a one-character strikethrough of a tilde. GFM says runs of three or more tildes do not open strikethrough. Bare ~~~ and ~~~~ are fine, so code fences are not affected.
  • A selection spanning a line break gets markers it cannot use: Ctrl+Shift+X over one\ntwo writes ~~one\ntwo~~, but highlighting runs per QTextBlock, so neither half matches — nothing is struck and all four tildes stay visible. Bold and italic behave the same way on master, so this is inherited, and GFM would not strike it either.
  • The caret-only case in togglesWrappedSelection is vacuous: it toggles twice and asserts only that the text ends up empty, which an implementation that did nothing on both calls also satisfies. It never checks the intermediate ~~~~ or the caret landing between the markers.

The README line is accurate as written, and stays accurate with the fix — the fix restores what Ctrl+I did before rather than changing whether it toggles.

One thing worth flagging for the maintainer rather than changing here: this makes Ctrl+B and Ctrl+I toggle off instead of nest, which is a change to shipped behaviour riding along with a new shortcut. Your PR body says so plainly and it is a defensible choice, but it deserves an explicit yes.

Heads up that this overlaps #8 (link URL hover) and #17 (focus mode) on the same files. #17 is the one to watch: you both rewrite the Ctrl+? dialog's single string literal and your README bullets are adjacent, so that pair needs a real resolution rather than a keep-both. #17 also pins that dialog to 380px without wrapping, and Ctrl+Shift+X Strikethrough is the longest line it will contain, so whichever of you lands second will need that width resized.

Tests pass (14 passed, 0 failed).

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.

3 participants