hrw4u: add --error-format flag with pluggable formatters#13147
Open
samtes wants to merge 5 commits intoapache:masterfrom
Open
hrw4u: add --error-format flag with pluggable formatters#13147samtes wants to merge 5 commits intoapache:masterfrom
samtes wants to merge 5 commits intoapache:masterfrom
Conversation
Adds a check-antlr Make target that errors with an install hint (brew/dnf/apt/bootstrap.sh) before attempting parser generation, instead of surfacing a cryptic rule failure deep in gen-fwd on a fresh checkout.
Introduces an ErrorFormatter ABC with PlainText, JSON, and Markdown implementations. ErrorCollector now delegates rendering instead of building the output string inline, with PlainText as the default so existing callers see byte-for-byte identical output. The JSON schema is versioned (v1) and emitted on a single line so bulk-mode runs produce NDJSON on stderr.
Wires the formatter abstraction into the CLI with a new
--error-format {plain,json,markdown} option (default: plain, so
existing consumers are unaffected). Non-syntax errors (file I/O,
argument errors) now flow through the same formatter path via
new emit_fatal_message / emit_fatal_error helpers, so downstream
tools see one stable schema regardless of where the error came from.
When only one error is produced, the 'Found 1 error:' summary line adds noise without adding information. Skip it in that case and keep the 'Found N errors:' header for counts of two or more.
A trailing comma on the last parameter forced yapf to keep one argument per line, which clashes with the project's 132-column horizontal style. Drop the trailing commas in the new formatter signatures, call sites, and dict literals so yapf collapses them.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new --error-format {plain,json,markdown} flag to hrw4u / u4wrh so diagnostics can be consumed by editors/CI/bots, implemented via a pluggable formatter layer while keeping plain text as the default.
Changes:
- Introduces
ErrorFormatterimplementations (plain/JSON/Markdown) and a formatter registry. - Routes parser/visitor/file I/O failures through the selected formatter and updates tests to lock in output contracts.
- Improves
tools/hrw4ubuild behavior with an early ANTLR-on-PATH check.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/hrw4u/src/formatters.py | New formatter hierarchy + registry for plain/JSON/Markdown diagnostic output. |
| tools/hrw4u/src/errors.py | Extends ErrorCollector to delegate rendering to a formatter; stores raw error message on Hrw4uSyntaxError. |
| tools/hrw4u/src/common.py | Adds --error-format CLI flag and routes fatal/non-syntax errors through formatter-aware helpers. |
| tools/hrw4u/Makefile | Adds check-antlr target and wires it into generation. |
| tools/hrw4u/tests/test_errors.py | Adds unit tests for formatter behavior and schema expectations. |
| tools/hrw4u/tests/test_cli.py | Adds CLI-level tests for --error-format behavior across parse/I/O failures and --help. |
| tools/hrw4u/tests/test_units.py | Updates unit expectations for the removed single-error “Found 1 error:” preamble. |
Comments suppressed due to low confidence (1)
tools/hrw4u/Makefile:160
check-antlris wired intogen-fwd, butgen-invstill invokes$(ANTLR)without the fast-fail prerequisite. This meansmake gen-inv(or any target that depends only on inverse generation) will still fail with the original cryptic error when ANTLR is missing. Addcheck-antlras a dependency ofgen-invas well.
# Generate forward parser/lexer into build/hrw4u and build/hrw4u-lsp
gen-fwd: check-antlr $(ANTLR_FILES_FWD)
$(ANTLR_FILES_FWD): $(GRAMMAR_FWD)
@mkdir -p $(PKG_DIR_HRW4U)
cd grammar && $(ANTLR) -Dlanguage=Python3 -visitor -no-listener -o ../$(PKG_DIR_HRW4U) hrw4u.g4
# LSP no longer generates its own ANTLR files - it imports from hrw4u
# Generate inverse parser/lexer into build/u4wrh
gen-inv: $(ANTLR_FILES_INV)
Comment on lines
+116
to
+118
| def _build_formatter(error_format: str): | ||
| """Instantiate the configured error formatter, falling back to plain.""" | ||
| return FORMATTERS.get(error_format, FORMATTERS["plain"])() |
Contributor
There was a problem hiding this comment.
Yeah, we encourage the -> return types as much as possible
| return None, parser_obj, error_collector | ||
| else: | ||
| fatal(f"{filename}:0:0 - {error_prefix} error: {e}") | ||
| emit_fatal_message(error_format, f"{filename}:0:0 - {error_prefix} error: {e}", filename=filename) |
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.
Motivation
hrw4u/u4wrhonly emit human-readable error output, which makes it hard for editors, CI jobs, and PR bots to consume diagnostics. This PR introduces structured output formats (JSON and Markdown) while keeping the existing plain-text output as the default, and tightens a few rough edges around error reporting and the build.Summary
--error-format {plain,json,markdown}tohrw4u/u4wrh, backed by a new pluggableErrorFormatterhierarchy insrc/formatters.py.Found 1 error:preamble for single-error plain output.PATH.Test plan
cd tools/hrw4u && make testpasses.hrw4u --error-format plain <bad.hrw4u>output is byte-identical tomaster(minus theFound 1 error:line for single errors).hrw4u --error-format json <bad.hrw4u>parses as JSON and includesschema_version: 1.hrw4u --error-format markdown <bad.hrw4u>renders cleanly when pasted into a GitHub PR comment.antlrhidden fromPATH,make genexits with the install hint instead of a cryptic failure.