Repository navigation
fix(mecha): cli and diagnostics fixes (caret position, --json, utf-8, --minecraft) - #524
Open
tickwarden wants to merge 5 commits into
Open
tickwarden wants to merge 5 commits into
tickwarden wants to merge 5 commits into
Conversation
…/eof errors For errors reported on a newline or eof token (for example an incomplete command such as `execute as @A run`), `underline_code` drew the caret under the first column of the *next* line, or drew no caret at all when the error was on the last line of the file. The reported `line:column` was correct, only the rendered snippet was misleading. The span of a newline token ends at column 1 of the following line, so the loop underlined one character there, while on the line that actually contains the error the range was clamped by `rstrip()` down to nothing. - Skip a line when the span ends at its first column (it covers none of its characters). - When the clamped range on the first line is empty because the span starts after the last visible character, keep a single caret at the original column instead of drawing nothing. Add tests/test_utils.py. The three newline/eof tests fail without this change; the other two guard the existing token and multi-line behaviour. The full mecha test suite still passes. AI disclosure: this patch (code, tests and this message) was written by Claude, an AI model made by Anthropic, at the request of the PR author. It is marked in the code with a comment in utils.py and tests/test_utils.py. Co-authored-by: Claude <noreply@anthropic.com>
`mecha -j out.json foo.mcfunction` silently did nothing: the statistics plugin that writes the json file was only loaded when `--stats` was passed, so no file was created and the command still exited with 0. A CI job relying on the json output would never notice. Load the statistics plugin when either `--stats` or `--json` is given. The verbosity is still only raised to INFO for an explicit `--stats`, so `--json` alone writes the file without printing the statistics table. Add tests/test_cli.py with tests for `-j` alone, `-s -j` and for the absence of a json file when `-j` is not given. The `-j` alone test fails without this change. AI disclosure: this patch (code, tests and this message) was written by Claude, an AI model made by Anthropic, at the request of the PR author. It is marked in the code with a comment in cli.py and tests/test_cli.py. Co-authored-by: Claude <noreply@anthropic.com>
… the cli Running `mecha` on a file that cannot be decoded as utf-8 ended with `Plugin "mecha.cli.validate" raised an exception` and a raw `UnicodeDecodeError` traceback. For a validator this should be a normal error that is counted in the summary and gives a non-zero exit code, and it must not prevent the remaining files from being validated. Wrap each compilation in `validate` with a small context manager that turns `UnicodeDecodeError` into an error `Diagnostic`. - A single file or a file found in a directory is reported with its own path and the remaining files are still validated. - For a data pack or a zip, the decoding error is raised while the pack is loaded and does not say which file caused it, so the error is reported with the path of the pack and the validation of that pack stops at that point. `Mecha.compile` is intentionally left unchanged to not alter the behavior of the library API. Add two tests to tests/test_cli.py. Both fail without this change. AI disclosure: this patch (code, tests and this message) was written by Claude, an AI model made by Anthropic, at the request of the PR author. It is marked in the code with a comment in cli.py and tests/test_cli.py. Co-authored-by: Claude <noreply@anthropic.com>
`mecha -m abc foo.mcfunction` failed with `Plugin "beet.toolchain.project.ProjectBuilder.bootstrap" raised an exception` and a `ValueError` traceback coming from `split_version`, while a well-formed but unknown version such as `-m 9.99` already produced a readable message. `-l banana` also gets a normal click usage error. Validate the option with a click callback that reuses beet's `split_version`, so the accepted formats stay identical to what beet itself accepts, and report malformed values with `click.BadParameter`. Well-formed versions are passed through unchanged, including unknown ones, which are still handled by beet as before. Add two tests to tests/test_cli.py. The invalid version test fails without this change. AI disclosure: this patch (code, tests and this message) was written by Claude, an AI model made by Anthropic, at the request of the PR author. It is marked in the code with a comment in cli.py and tests/test_cli.py. Co-authored-by: Claude <noreply@anthropic.com>
`Mecha.compile` only handled `DiagnosticError` while reading a source file, so a function that is not valid utf-8 raised a bare `UnicodeDecodeError` and aborted the compilation of the whole pack. The previous commit worked around this in the cli, but could only report the path of the pack and stopped validating that pack at the first bad file. Handle `UnicodeDecodeError` next to `DiagnosticError` and report it as an error diagnostic of that compilation unit. The unit already carries its filename and resource location, so the error points at the exact file (`pack/data/demo/function/broken.mcfunction`) and the other functions of the pack are still compiled and validated. Behavior change for library users: `Mecha.compile` no longer lets a `UnicodeDecodeError` from reading a function escape. It is part of the `DiagnosticError` raised at the end of the compilation, or of the `report` collection when one is given. Not covered: when a data pack is loaded from a zip, beet decodes every file while opening the archive (`File.from_zip`), before `Mecha.compile` is called. That error is still reported with the path of the zip only and would need a change in beet itself. Add tests/test_decode_error.py and a data pack test to tests/test_cli.py. Both fail without this change. This commit applies on top of the previous cli commits because it extends tests/test_cli.py. AI disclosure: this patch (code, tests and this message) was written by Claude, an AI model made by Anthropic, at the request of the PR author. It is marked in the code with a comment in api.py and tests/test_decode_error.py. Co-authored-by: Claude <noreply@anthropic.com>
This branch has not been deployed
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
2 participants
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.
Summary
Five small fixes to the
mechacli and its diagnostics. Each one is its own commit and has tests.execute as @a run),underline_codedrew the caret under the first column of the next line, or no caret at all on the last line of the file. The caret now points at the end of the offending line. The reportedline:columnwas already correct; only the rendered snippet was misleading.-j/--jsonwithout-s/--stats:--jsonnow works on its own instead of requiring--stats.-m/--minecraftvalues: these are now rejected with a usage error instead of failing later.Mecha.compileonly handledDiagnosticErrorwhile reading a source file, so a function that is not valid utf-8 raised a bareUnicodeDecodeErrorand aborted the whole pack. It is now handled next toDiagnosticErrorand reported as an error diagnostic of that compilation unit. The error points at the exact file (e.g.pack/data/demo/function/broken.mcfunction) and the other functions are still compiled and validated. This replaces the cli-level workaround from commit 3, which could only report the pack path and stopped at the first bad file.Behavior change for library users
Mecha.compileno longer lets aUnicodeDecodeErrorfrom reading a function escape. It is now part of theDiagnosticErrorraised at the end of the compilation, or of thereportcollection when one is given.Not covered
When a data pack is loaded from a zip, beet decodes every file while opening the archive (
File.from_zip), beforeMecha.compileruns. That error is still reported with the path of the zip only and would need a change in beet itself.Tests
packages/mecha/tests/test_cli.py: cases for--json,--minecraft, and non-utf-8 files/packspackages/mecha/tests/test_decode_error.py: newpackages/mecha/tests/test_utils.py: caret renderingThe new tests fail without the corresponding change.
pytest packages/mecha/testspasses locallyAI disclosure
The code, tests and commit messages of these patches were written by Claude (Anthropic) and reviewed by me before submitting.