torchbox / django-pattern-library

UI pattern libraries for Django templates
https://torchbox.github.io/django-pattern-library/
BSD 3-Clause "New" or "Revised" License
368 stars 46 forks source link

Feature: Allow `yml` extension #169

Closed tbrlpld closed 2 years ago

tbrlpld commented 2 years ago

Description

So far, the context YAML files had to have the .yaml extension. YAML files often also use the .yml extension. This PR adds a second attempt at finding the YAML file with the .yml extension after the .yaml extension has failed to produce a result.

The second attempt is simply enabled by adding a second nested try-except-block. I am not sure this is the most elegant solution, but it works and seems easy to understand.

Fixes #161

Checklist

tbrlpld commented 2 years ago

@thibaudcolas I have pushed changes according to your review.

Regarding the caching: I also think that would be overkill. Especially in larger projects I can imagine that both extensions could occur when no linter or CI checks preventing it from happening. In that case it would be particularly confusing why the files are not recognized while they would work in another project which is consistent.

If we wanted to allow projects to be consistent, we could add a setting, but I am afraid that could also cause confusion because you would need to know how it project is configured.

I am not sure, but would using glob to check for file existence save the additional IO?

thibaudcolas commented 2 years ago

@tbrlpld thank you, I’ll try to review this once I get the chance. Re glob – I doubt it’s worth it. We’d have to do 1 scandir with glob, then the 1 file read, so it’d be two IO calls for all cases rather than the "1 or 2" situation we’d be in here.

tbrlpld commented 2 years ago

Thanks for reviewing @bcdickinson