Skip to content
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

Detect input format based on file name extension #1582

Merged
merged 4 commits into from Mar 8, 2023

Conversation

ryenus
Copy link
Contributor

@ryenus ryenus commented Mar 2, 2023

When the inputFormat is not specified, instead of just assume the input format to be "yaml", we can also look at the input filename, more specifically the file extension, to deduce the input format.

This Fixes #1440.

@mikefarah
Copy link
Owner

Looks really good - could you add some acceptance tests (see acceptance_tests folder) to test that the detection works for the various file types?

@ryenus ryenus force-pushed the detect-input-fmt branch 2 times, most recently from 44f6fed to b412fd2 Compare March 3, 2023 01:35
@ryenus
Copy link
Contributor Author

ryenus commented Mar 3, 2023

@mikefarah Thank you! I added a test, actually just copied but removing the explicit -p args.
Hopefully it works as expected. Just a humble attempt as a rare-time gopher, Please feel free to amend :-)

@ryenus ryenus force-pushed the detect-input-fmt branch 2 times, most recently from 45fbf37 to 0ad46f0 Compare March 3, 2023 02:53
@ryenus
Copy link
Contributor Author

ryenus commented Mar 3, 2023

@mikefarah, any idea why testLeadingSeparatorDoesNotBreakCommentsOnOtherFiles is broken? Could you please take a look?

shunit2:ERROR testLeadingSeparatorDoesNotBreakCommentsOnOtherFiles() returned non-zero return code.
ASSERT:expected:<# a1
a: 1
# a2

# b1
b: 2
# b2> but was:<# a1
a: 1
# a2

b: 2
# b2>

@mikefarah
Copy link
Owner

Looks like the #b1 comment disappeared 🤔 not sure why that would happen

@hubchub
Copy link

hubchub commented Apr 4, 2023

I don't think this belongs in a v4.x release.
It is a nice feature and all but it totally breaks backwards compatibility.

@ryenus
Copy link
Contributor Author

ryenus commented Nov 16, 2023

@hubchub, no, detecting input format based on the filename is not really breaking, as it works only when the input format is not specified. Meanwhile the explicitly set input format is always respected, just like how it behaved in the past. If you meant the change related to the default output format, that's a separate topic.

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.

Detect input format based on file name extension
3 participants