Skip to content

loader: require .json extension and allow .yaml extension#520

Merged
bosd merged 2 commits intoinvoice-x:masterfrom
rmilecki:loader-extensions
Nov 2, 2023
Merged

loader: require .json extension and allow .yaml extension#520
bosd merged 2 commits intoinvoice-x:masterfrom
rmilecki:loader-extensions

Conversation

@rmilecki
Copy link
Copy Markdown
Collaborator

@rmilecki rmilecki commented Aug 6, 2023

loader: allow YAML templates with .yaml extension

Oficially suggested extension for YAML files is .yaml. Accept it.
loader: require .json extension for JSON files

Trying to parse every file other than *.yml as JSON file isn't the best
idea. It wastes time on trying to parse non-template files. It produces
warnings like "json Loader Failed to load (...)template" for files that
were never meant to be templates. It's a bad pracice in general.

Check for file extension before using json.loads().

@bosd
Copy link
Copy Markdown
Collaborator

bosd commented Aug 7, 2023

Thanks for this PR.
I agree the loader could use some work. It's on my wish list :)
As the recent refactoring broke the possibility to stream load templates.

I think adding more extension filters will block the possibility to streamload templates even further.

Rafał Miłecki added 2 commits November 2, 2023 19:10
Trying to parse every file other than *.yml as JSON file isn't the best
idea. It wastes time on trying to parse non-template files. It produces
warnings like "json Loader Failed to load (...)template" for files that
were never meant to be templates. It's a bad pracice in general.

Check for file extension before using json.loads().

Fixes: 0d2d16c ("Native JSON Template Support")
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
Oficially suggested extension for YAML files is .yaml. Accept it.

Link: https://yaml.org/faq.html
Fixes: invoice-x#514
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
@bosd bosd force-pushed the loader-extensions branch from 7beb0be to 18015ce Compare November 2, 2023 18:10
@bosd bosd merged commit f4e984f into invoice-x:master Nov 2, 2023
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.

2 participants