Formatter improvements#33
Conversation
|
This is too big a PR, but looking to see what else can be effectively pulled from it to avoid divergence. Note: we already pulled in switch to ruamel.yaml I do not want to change the basic syntax or add the Do not want to change most of the errors (most of which will stop interview from compiling, some only break or have unexpected behavior at runtime) into warnings |
There was a problem hiding this comment.
Pull request overview
This PR expands the project’s CLI/formatter/checker functionality by introducing structured message codes, Jinja-aware processing, and a new top-level dayaml command, along with substantial new test coverage and fixtures.
Changes:
- Added a message registry (
MessageCode+ templates) and refactored validators/checker output to use stable[E/W/C###]codes. - Added Jinja2 header detection + preprocessing for the checker, and text-based formatting for
# use jinjafiles in the formatter. - Introduced shared YAML file collection utilities and a new
dayamlmulti-command CLI; updated docs and test suite/fixtures accordingly.
Reviewed changes
Copilot reviewed 20 out of 23 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/dayamlchecker/yaml_structure.py |
Refactors validation to emit coded findings, adds Jinja preprocessing, improves CLI output and summary handling. |
src/dayamlchecker/messages.py |
New central registry of message codes, severity, templates, and experimental/real-error semantics. |
src/dayamlchecker/_jinja.py |
Adds Jinja header detection, syntax detection, and safe rendering for static analysis. |
src/dayamlchecker/_files.py |
Adds shared _collect_yaml_files used by checker/formatter. |
src/dayamlchecker/code_formatter.py |
Adds Jinja-aware formatting path; refactors CLI to use shared collection and improved output. |
src/dayamlchecker/cli.py |
New dayaml command dispatcher for check and format. |
src/dayamlchecker/__main__.py |
Keeps module entrypoint as checker CLI with a standard SystemExit(main()) guard. |
src/dayamlchecker/py.typed |
Continues to mark the package as typed for type checkers. |
tests/test_yaml_structure_cli.py |
Adds CLI/process_file integration tests for checker behavior and output formatting. |
tests/test_yaml_structure.py |
Greatly expands unit coverage for message codes, Jinja handling, and many validator edge cases. |
tests/test_code_formatter_cli.py |
Adds formatter CLI integration tests (quiet/no-summary/check mode, Jinja behavior, error handling). |
tests/test_code_formatter.py |
Adds extensive formatter unit tests including Jinja utilities and file collection. |
tests/test_dayaml_cli.py |
Adds tests for the new dayaml command dispatcher behavior. |
tests/_cli_helpers.py |
Adds a shared RunResult helper for CLI tests. |
tests/__init__.py |
Marks tests as a package to support shared helpers. |
tests/fixtures/large_valid_interview.yml |
Adds a “known-good” large fixture used by tests. |
tests/fixtures/large_invalid_interview.yml |
Adds a large “covers registry” fixture for error-code coverage assertions. |
tests/fixtures/large_invalid_jinja_syntax.yml |
Adds fixture for Jinja syntax error coverage. |
tests/fixtures/large_invalid_jinja_template.yml |
Adds fixture for Jinja template/runtime error coverage. |
requirements.txt |
Updates dependencies (adds Jinja2; removes PyYAML/types; retains ruamel.yaml/black/mypy). |
pyproject.toml |
Adds script entrypoints, dependency groups, and coverage config; updates dependency declarations. |
README.md |
Documents new dayaml CLI usage and the message code catalog. |
.gitignore |
Expands coverage ignore pattern and adds .vscode/. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| print(f"skipped: {display_path or input_file}") | ||
| return "skipped" | ||
|
|
||
| with open(input_file, "r") as f: |
| dependencies = [ | ||
| "esprima>=4.0.1", | ||
| "jinja2>=3.1.6", | ||
| "mako>=1.3.10", | ||
| "pyyaml>=6.0.2", | ||
| "black>=24.0.0", | ||
| "ruamel.yaml>=0.18.0", | ||
| "black>=26.3.1", | ||
| "ruamel-yaml>=0.19.1", |
| Tuple of (formatted YAML string, whether any changes were made) | ||
|
|
||
| Raises: | ||
| Exception: If YAML parsing fails (Jinja files are returned unchanged before parsing) |
| summary="fields code must be YAML string", | ||
| template="fields: code must be a YAML string, is {value_type}", | ||
| ), | ||
| MessageCode.FIELDS_DICT_KEYS: MessageDefinition( | ||
| code=MessageCode.FIELDS_DICT_KEYS, | ||
| summary="fields dict missing code key", | ||
| template='fields dict must have "code" key, is {value}', |
| @@ -1561,26 +1823,33 @@ def find_errors(input_file: str) -> list[YAMLError]: | |||
| with open(input_file, "r") as f: | |||
731f479 to
704d540
Compare
|
@nonprofittechy I didn't realize that you were doing this until just now. I just merged/your main branch into this and then rebased it into a few clean, logical commits. Hopefully it helps. I am just curious why you don't like the Let me know if there's anything else I can do to bring these features into the main line. Eventually I want to use this in my Anyways, I've been quite happy using my branch for my own work, but now I'll be testing this new combined branch. |
Co-authored-by: Copilot <copilot@github.com>
…in line with other codes Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
No description provided.