Fix parser bugs: colon truncation, file:line:error support, Tectonic filenames - #32
Open
dpezto wants to merge 1 commit into
Open
Fix parser bugs: colon truncation, file:line:error support, Tectonic filenames#32dpezto wants to merge 1 commit into
dpezto wants to merge 1 commit into
Conversation
Five parser fixes, plus a fixture runner so behavior changes are reviewable.
test/run.sh feeds each test/*.log through pplatex and diffs against
test/expected/<name>.txt; --update regenerates them. Four fixtures are added
for cases nothing previously covered.
1. Errors and warnings lost text after their first colon. Both patterns read
".*:(.*)", and the greedy .* runs to the last colon on the line. Undefined
reference warnings and package errors quoting a filename or key always
contain a second colon and were consistently mangled:
before: intro' on page 1 undefined on input line 15.
after: Reference `sec:intro' on page 1 undefined on input line 15.
Same as PR stefanhepp#19, extended to the error pattern, with the space after the
colon optional so an empty message is not dropped.
2. file:line:error style logs parsed as nothing (issue #2). With
-file-line-error, errors read "./min.tex:3: Undefined control sequence."
and no pattern matched that shape; test/min.log reported zero errors. The
new detector takes file and line straight off the line and emits the item
immediately, since this format never prints the "l.<n>" the normal path
waits for.
3. Errors from Tectonic had no filename (issue stefanhepp#21). The parenthesis heuristic
pushes a filename only once it is fairly sure it has one, but pops on every
')'. A '(' that fails to push is still popped, so the stack drains; in the
log from stefanhepp#21 the source file is discarded 21 lines in, and every error is
then reported with no file. The trigger is several opens on one line:
) (tikz.sty (pgf.sty (pgfrcs.sty (pgfutil-common.tex
Only the last is pushed; the others are followed by a space and fall
through to fileExists(), which fails for distribution files. pdflatex
mostly escapes this by wrapping at 79 columns. A candidate with a short
alphanumeric extension is now accepted when it cannot be confirmed on disk.
This also covers PR stefanhepp#18's case of page markers glued onto a filename,
verified by test/pagemarkers.log.
4. Filler inside error contexts leaked through. addMessage() compares a line
against literals, but lines keep trailing spaces (their untrimmed length is
what detects wrapped words), so the "..." marker padded to the log width
was never equal to "...". The comparison is now right-trimmed.
5. Multi-line warnings never got a line number: the Warning-state branch
passed an always-empty local to the line-number scan. It now scans the
continuation line itself. This exposed a second bug in the wrapped-line
fallback "(.*)([0-9]+)\.$": the greedy group eats into the number,
reporting Line 3 for a line wrapped as "...on input lin" / "e 13.". The
first group must now end on a non-digit.
The existing pdflatex fixtures change only in the intended ways: messages keep
their text past a colon, "..." filler disappears, and eight warning headers
gain ", Line N", each N matching the number stated in its own message text.
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.
Five parser fixes, plus a fixture runner so the behavior changes are reviewable rather than asserted. Fixes #2, fixes #21, supersedes #18 and #19.
Test harness first.
test/run.shfeeds eachtest/*.logthrough pplatex and diffs againsttest/expected/<name>.txt(--updateregenerates). The expected files are committed, so each fix shows up as a reviewable diff. Four fixtures added for cases nothing covered.1. Errors and warnings lost text after their first colon. Both patterns read
.*:(.*)and the greedy.*runs to the last colon. Undefined-reference warnings and package errors quoting a filename or key always contain a second colon:Same fix as #19, extended to the error pattern, with the space after the colon optional so an empty message is not dropped.
2.
file:line:errorlogs parsed as nothing (#2, open since 2013). With-file-line-error, errors read./min.tex:3: Undefined control sequence.and no pattern matched;test/min.logreported zero errors. The new detector takes file and line straight off the line and emits immediately, since this format never prints thel.<n>the normal path waits for.3. Errors from Tectonic had no filename (#21). The paren heuristic pushes a filename only once it is fairly sure it has one, but pops on every
). A(that fails to push is still popped, so the stack drains — in the log from #21 the source file is discarded 21 lines in, after which every error reports no file. The trigger is several opens on one line:Only the last is pushed; the rest are followed by a space and fall through to
fileExists(), which fails for distribution files that don't sit next to the document. pdflatex mostly escapes this by wrapping at 79 columns so names land at end-of-line. A candidate with a short alphanumeric extension is now accepted when it can't be confirmed on disk; prose likesize optionis still rejected. This also covers #18's page-marker case (./chapter.tex [12] [13]glued into the filename), verified bytest/pagemarkers.log.4. Filler leaked into messages.
addMessage()compares lines against literals, but lines keep their trailing spaces (their untrimmed length is how wrapped words are detected), so the...marker padded to the log width never equalled"...". The comparison is now right-trimmed.5. Multi-line warnings never got a line number. The Warning-state branch passed an always-empty local to the line-number scan. It now scans the continuation line itself. This exposed a second bug in the wrapped-line fallback
(.*)([0-9]+)\.$: the greedy group eats into the number, reporting Line 3 for a line wrapped as...on input lin/e 13..The existing fixtures change only in the intended ways: messages keep their text past a colon,
...filler disappears, and eight warning headers gain, Line N— each N cross-checked against the number stated in its own message text.Independent of #30: this branch builds against PCRE1 as-is.