Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 76 additions & 14 deletions .github/workflows/scripts/strip-non-invoking-markup.sh
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,51 @@ function run_len(s, ch, n) {
return n
}

# Whether `bare` is a CommonMark thematic break: three or more of a single one
# of `- * _`, with only spaces or tabs between. Written as a character count
# rather than a regex because the POSIX ERE that awk uses has no backreference,
# so a same-character-repeated pattern (a `\1` in PCRE) silently never matches.
function is_thematic_break(bare, ch, i, c, n) {
ch = substr(bare, 1, 1)
if (ch != "-" && ch != "*" && ch != "_") return 0
n = 0
for (i = 1; i <= length(bare); i++) {
c = substr(bare, i, 1)
if (c == ch) n++
else if (c != " " && c != "\t") return 0
}
return (n >= 3)
}

# Whether `bare` is a setext heading underline: a run of only `=` or only `-`,
# trailing spaces/tabs allowed. This is a *heading* underline only when the line
# before it is paragraph text, which the caller tracks separately -- the same
# characters are otherwise a thematic break (`- * _`, >=3), a list marker, or
# ordinary text. CommonMark makes the two underline characters symmetric, and an
# indented code block opens after either with no blank line, so the `-` case
# that `is_thematic_break` already covers for >=3 dashes needs its `=` twin here
# (plus 1-2 dash underlines, which are not thematic breaks).
function is_setext_underline(bare) {
return bare ~ /^=+[ \t]*$/ || bare ~ /^-+[ \t]*$/
}

# Whether this line is a leaf block -- an ATX heading or a thematic break --
# after which an indented code block may begin with no blank line. `bare` is the
# line with its indentation removed, `indent` its width. These two shapes need
# no context; a setext underline also qualifies but only after paragraph text,
# so the caller handles it. A list item deliberately does NOT qualify: an
# indented line after one is a list continuation, not code, and stripping it is
# the over-stripping error the mention gate must avoid (the list-continuation
# cases added in #345 guard exactly this). Both shapes are only valid at three
# columns of indentation or fewer; deeper, they are themselves code.
function heading_or_break(bare, indent) {
if (indent > 3) return 0
# ATX heading: 1-6 `#` then a space/tab or end of line.
if (bare ~ /^#{1,6}([ \t]|$)/) return 1
if (is_thematic_break(bare)) return 1
return 0
}

# Replace every closed inline code span in `text` with the placeholder.
#
# This runs once over the WHOLE remaining body rather than per line, because a
Expand Down Expand Up @@ -121,7 +166,8 @@ function strip_spans(text, out, i, n, run, j, closerun, found) {
BEGIN {
in_fence = 0; fence_char = ""; fence_len = 0
in_icode = 0
prev_blank = 1 # start of input behaves like a blank line
prev_opens_icode = 1 # start of input can open an indented code block
prev_para = 0 # ... but is not itself paragraph text (no setext under it)
kept = ""; nkept = 0
}

Expand All @@ -142,23 +188,26 @@ BEGIN {
substr(bare, run_len(bare, fence_char) + 1) ~ /^[ \t]*$/) {
in_fence = 0
}
prev_blank = blank
prev_opens_icode = 0; prev_para = 0
next
}

# --- indented code block ----------------------------------------------
# Four columns of indentation after a blank line opens one, and it runs
# until a non-blank line dedents to three columns or fewer. The blank-line
# precondition is what keeps an indented *list continuation* out of this
# branch: those follow their list item directly, so over-stripping them
# would drop a genuine request, which is the expensive error for the
# mention gate sharing this script.
# Four columns of indentation opens one when the preceding line does not leave
# an open paragraph or list for it to lazily continue. `prev_opens_icode`
# carries that from the previous iteration: it is set for a blank line, an ATX
# heading, a thematic break (`heading_or_break`), or a setext underline sitting
# under paragraph text. It runs until a non-blank line dedents to three columns
# or fewer. The precondition is what keeps an indented *list continuation* out
# of this branch: those follow their list item directly, so over-stripping them
# would drop a genuine request, which is the expensive error for the mention
# gate sharing this script.
if (in_icode) {
if (blank || indent >= 4) { prev_blank = blank; next }
if (blank || indent >= 4) { prev_opens_icode = 0; prev_para = 0; next }
in_icode = 0
} else if (prev_blank && !blank && indent >= 4) {
} else if (prev_opens_icode && !blank && indent >= 4) {
in_icode = 1
prev_blank = blank
prev_opens_icode = 0; prev_para = 0
next
}

Expand All @@ -167,17 +216,30 @@ BEGIN {
fence_char = lead
fence_len = run_len(bare, lead)
in_fence = 1
prev_blank = blank
prev_opens_icode = 0; prev_para = 0
next
}
if (lead == ">") { prev_blank = blank; next }
if (lead == ">") { prev_opens_icode = 0; prev_para = 0; next }
}

# Kept lines are buffered rather than printed, so the span scan below can
# see across line boundaries.
kept = (nkept == 0) ? line : kept "\n" line
nkept++
prev_blank = blank
# Decide what the NEXT line inherits. A blank line, an ATX heading, or a
# thematic break (`hd`) opens an indented code block after it with no context;
# a setext underline does too, but only when it is actually *consumed* as one
# -- a `=`/`-` run (`ul`) sitting under paragraph text (`prev_para`). A ul-shaped
# run with no paragraph above it is not consumed: CommonMark makes it an
# ordinary new paragraph, so `prev_para` must go to 1 for the next line (a later
# underline could underline it), which is why `prev_para` keys on `consumed`
# rather than the raw `ul` match. Otherwise a chain of underline-shaped lines
# (`===` / `===` / indented request) mistracks state and under-strips.
hd = heading_or_break(bare, indent)
ul = (indent <= 3 && is_setext_underline(bare))
consumed = (ul && prev_para)
prev_opens_icode = (blank || hd || consumed)
prev_para = (!blank && !hd && !consumed)
}

END {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,61 @@ check "indentation without a preceding blank line is not code" \
$'some prose\n @claude review' \
$'some prose\n @claude review'

# An indented code block also opens after a thematic break or an ATX heading
# with no blank line: neither leaves an open paragraph for the indented line to
# lazily continue (CommonMark 0.31.2, "Indented code blocks"). A request quoted
# this way must still be stripped (gha#356).
check "indented code block opens after a thematic break" \
$'---\n @claude review' \
'---'

check "indented code block opens after an ATX heading" \
$'## Accepted phrasing\n @claude review' \
'## Accepted phrasing'

check "indented code block opens after a spaced thematic break" \
$'* * *\n @claude review' \
'* * *'

# A list item is NOT such a predecessor: an indented line after it is a list
# continuation, not code, so it must survive -- and a lone-dash item must not
# be mistaken for a thematic break, which would over-strip a genuine request.
check "indented line after a list item is not code" \
$'- a point\n @claude review' \
$'- a point\n @claude review'

# A setext heading underline (a run of only `=` or only `-` under paragraph text)
# is also such a predecessor. CommonMark makes the two underline characters
# symmetric; the `-` case is caught by the thematic-break check only for >=3
# dashes, so the `=` twin and the 1-2 dash underlines are handled here.
check "indented code block opens after an = setext underline" \
$'Heading\n======\n @claude review' \
$'Heading\n======'

check "indented code block opens after a single-dash setext underline" \
$'Heading\n-\n @claude review' \
$'Heading\n-'

# But a run of `=` NOT under paragraph text is an ordinary paragraph, not a
# heading underline, so an indented line after it is a lazy continuation, not
# code -- stripping it would drop a genuine request.
check "= run at start of input is a paragraph, not a setext underline" \
$'======\n @claude review' \
$'======\n @claude review'

check "= run after a blank line is a paragraph, not a setext underline" \
$'intro\n\n======\n @claude review' \
$'intro\n\n======\n @claude review'

# A chain of underline-shaped lines: the first `===` has no paragraph above it so
# it is an ordinary paragraph, and the second `===` then underlines it into a
# heading -- after which the indented request is code and must be stripped. This
# only works if paragraph state tracks whether a line was actually *consumed* as
# an underline, not the raw match.
check "chain of two = runs forms a heading, stripping the indented request" \
$'===\n===\n @claude review' \
$'===\n==='

check "CRLF endings are normalized" \
$'@claude review\r\nthanks\r' \
$'@claude review\nthanks'
Expand Down
14 changes: 14 additions & 0 deletions changelog.d/strip-non-invoking-markup-block-predecessors.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
- **The `@claude review` matcher no longer fires on a mention quoted as an
indented code block that opens after a heading or a thematic break**
(gha#356).
`strip-non-invoking-markup.sh` only opened an indented code block after a
blank line, but CommonMark bars an indented block only from *interrupting a
paragraph* -- after a thematic break, an ATX heading, or a setext-heading
underline it opens with no blank line at all.
So a request quoted under a `---` or a `#`/`===` heading rendered as code on
GitHub yet slipped through the stripper and dispatched a review off quoted
text -- the same class of false dispatch the code-span and fence handling
already close.
A list item is deliberately still not treated as such a predecessor: an
indented line after one is a list continuation, not code, so stripping it
would drop a genuine request in the mention gate that shares this script.
Loading