Name the contexts a change cannot survive - #24
Open
phucnht wants to merge 1 commit into
Open
Conversation
Four constructs inside a change aborted compilation with an error from
TeX or xcolor that never mentioned this package, so a user had no way to
tell which construct offended or what to do instead:
\verb Argument of \XC@col@rlet has an extra }
verbatim environment Not allowed in LR mode
spanning an & tab Missing } inserted
\footnote Argument of \XC@col@rlet has an extra }
Each now stops with a message naming the construct and the way around
it. A local redefinition inside the change's own group intercepts \verb
and the verbatim-like environments, so the redefinitions vanish with the
group. Alignment tabs are found by scanning the argument for category
code 4, which is the same \tl_analysis_map_inline:nn technique the
report text already uses; matching on catcode rather than on the
character means an escaped \& in ordinary prose does not trip it.
The first three fail identically inside plain \textbf, so they are
properties of LaTeX rather than defects here, and the guards name them
rather than pretending to fix them.
The footnote case is different and was traced rather than assumed. It is
not a regression: the pre-PR-23 package fails the same way. The cause is
\addtocontents carrying a footnote in the change report line, reproduced
with no part of this package involved:
\addtocontents{x}{\protect\cmd{text\footnote{n}}}
Writing the text unexpanded does not help, and a report entry has no
sensible footnote of its own, so this is diagnosed rather than fixed.
The suggested workaround, placing the footnote immediately after the
change, was compiled and verified.
Citations were the premise of the roadmap item and turned out not to be
broken at all: \cite, \index, \ref, and a change inside a single table
cell all compile. The roadmap wording is corrected accordingly, and the
README's Limitations section, which still steered users away from
headings, floats and math after PR #23 made those work, is rewritten
against what the probes actually show.
case_error_fixtures gains a helper that asserts a fixture fails *and*
that the log carries the expected message. Asserting only failure would
pass even if the old cryptic error came back, since these documents
always stopped compilation; the message is what distinguishes a
diagnosed context from a crash. The helper joins the log before matching
because TeX wraps it at 79 columns and a line-based grep misses any
message long enough to be useful.
Verified each guard is not vacuous by disabling it in turn: without the
verb guard the verb fixture's assertion fails, without the alignment
scan the alignment fixture's does, without the footnote check the
footnote fixture's does, and no guard's removal disturbs another.
Restoring each passes. Across the 26 probe documents kept from this and
the previous slice, 21 compile clean, 4 stop with the new diagnostics,
and none broke unexpectedly. Full suite green at 23 cases.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Second slice of the roadmap's "Robust and accessible authoring" group, following #23. Measured with 26 probe documents rather than inferred, and two of the measurements changed what the work should be.
What was actually broken
Four constructs aborted compilation with an error from TeX or xcolor that never named this package:
\textbf\verbArgument of \XC@col@rlet has an extra }\verb illegal in argumentverbatimenvironmentNot allowed in LR modeFile ended while scanning use of \@xverbatim&tabMissing } insertedArgument of \check@nocorr@ has an extra }\footnoteArgument of \XC@col@rlet has an extra }The
\textbfcolumn is what decides fault. Three are properties of LaTeX, so the guards name them rather than pretending to fix them.Two findings that changed the plan
The roadmap item's premise was wrong. It read "unsupported citation contexts", but
\citeinside a change compiles, and so do\index,\ref, and a change within a single table cell. Nothing needed fixing there; the roadmap wording is corrected instead.The README was stale. It still told users to avoid headings, floats and math at a larger boundary, which #23 had already made work. Left alone it would steer people away from working features. Rewritten against the probe results.
The footnote case, traced rather than assumed
Not a regression: the pre-#23 package fails identically. Root cause is
\addtocontentscarrying a footnote in the report line, reproduced with zero texchanges involved:Writing the text unexpanded does not help, and a report entry has no sensible footnote of its own, so it is diagnosed rather than fixed. The suggested workaround, putting the footnote immediately after the change, was compiled and confirmed to work.
How it is implemented
A local redefinition inside the change's own group intercepts
\verband the verbatim-like environments, so the redefinitions vanish with the group. Feasibility was proven with a standalone probe first, since these failures happen while xparse reads the argument and interception could have been impossible.Alignment tabs are found by scanning for category code 4 using the same
\tl_analysis_map_inline:nntechnique\tx_report_text:nalready uses. Matching on catcode rather than the character means an escaped\&in ordinary prose does not trip it, which is verified by a probe.Tests
case_error_fixturesgains a helper asserting a fixture fails and that the log carries the expected message. Asserting only failure would pass even if the old cryptic error returned, since these documents always stopped compilation. The helper joins the log before matching, because TeX wraps at 79 columns and a line-based grep silently misses any message long enough to be useful — which it did on the first attempt.Verification
Each guard disabled in turn makes its own fixture's assertion fail and no other; restoring passes:
verb-in-changeassertion failsalignment-in-changeassertion failsfootnote-in-changeassertion failsAcross the 26 probes from this slice and #23: 21 compile clean, 4 stop with the new diagnostics, 0 broke unexpectedly.
\cite,\index,\refand in-cell changes still compile, so the guards do not over-fire. Full suite green at 23 cases,shellcheckclean, website build green.