Skip to content

Add --force-filetype option to force a filetype for an instance file #557

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 11, 2025

Conversation

mschoettle
Copy link
Contributor

@mschoettle mschoettle commented Apr 11, 2025

Add --force-filetype option to force an instance file to be parsed with the requested file format parser.

Fixes #341

@mschoettle mschoettle force-pushed the force-parser branch 3 times, most recently from c96af4b to 031cc9a Compare April 11, 2025 10:42
@mschoettle mschoettle changed the title Add force-parser option to force using a parser for a file Add force-filetype option to force a filetype for a file Apr 11, 2025
@mschoettle mschoettle marked this pull request as ready for review April 11, 2025 10:43
@mschoettle mschoettle changed the title Add force-filetype option to force a filetype for a file Add --force-filetype option to force a filetype for a file Apr 11, 2025
Copy link
Member

@sirosen sirosen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thanks so much for putting together this changeset!

There are some very minor tweaks I'd like us to make before merging, but I think this is good. And I agree that --force-filetype has a nice symmetry with --default-filetype, so that's a nice name for this option.

) -> t.Callable[[t.IO[bytes]], t.Any]:
filetype = path_to_type(path, default_type=default_filetype)

if filetype in self._by_tag:
filetype = force_filetype or filetype
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might make more sense to use force_filetype as an alternative to calling path_to_type().
e.g.,

if force_filetype is None:
    filetype = path_to_type(path, default_filetype=default_filetype)
else:
    filetype = force_filetype

I'm pretty sure it doesn't matter in practice -- outside of unit tests, the missing support block is hard to reach -- but IMO it would read cleaner because we take care of the assignment before we start making any control-flow decisions based on the value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! This actually caught a potential issue when force_filetype is not in self._by_tag.

@mschoettle mschoettle requested a review from sirosen April 11, 2025 14:57
@mschoettle mschoettle changed the title Add --force-filetype option to force a filetype for a file Add --force-filetype option to force a filetype for an instance file Apr 11, 2025
Copy link
Member

@sirosen sirosen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks again for going after this!

@sirosen sirosen merged commit 57f0e1c into python-jsonschema:main Apr 11, 2025
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow optionally interpreting .json files as JSON5
2 participants