-
Notifications
You must be signed in to change notification settings - Fork 51
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
Conversation
c96af4b
to
031cc9a
Compare
031cc9a
to
475e0a0
Compare
There was a problem hiding this 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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
.
b423ae1
to
b0c97c9
Compare
There was a problem hiding this 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!
Add
--force-filetype
option to force an instance file to be parsed with the requested file format parser.Fixes #341