search

openapi-contrib / openapi3-generator

Use your API OpenAPI 3 definition to generate code, documentation, and literally anything you need.
Apache License 2.0
89 stars 27 forks source link

feat(templates): support optional .hbs extensions in templates. #14

Closed peterlenagh closed 5 years ago

peterlenagh commented 5 years ago

Thanks for this project!

I thought it'd be easier to work with the templates if they had the correct extensions (for syntax highlighting, making prettier not wrongly format them on save etc. etc.).

This PR:

peterlenagh commented 5 years ago

Thanks for getting back to me @fmvilas.

No worries if you don't want to merge this, I can see its maybe more of a personal preference thing than an improvement over what's already in place. That's why I tried to implement it in such a way that it was optional.

I've updated the PR with a fix to only replace hbs from the end of filenames, and updated the readme to reflect the changes in this PR.

To each of your points specifically:

What happens if you really want to have a Handlebars file in your template? Say, for instance, a template wants to provide views with Handlebars and it has files named like user-profile.hbs. As it is right now, they will also get affected while they shouldn't.

As is, this would remove the extension yes. You could name them .hbs.hbs if you needed to persist the extension (I've added this to the README) but I see that in this case it's not really as optional as I'd wanted.

Also I don't know how common a usecase this is? it's very meta. I was tempted to put a yo dawg in the readme 😆.

An alternative approach here might be to just not rm .hbs if it's the only extension. This would mean you can't generate files with no extension though - I don't know if that would ever be a problem. We could make sure it still worked for dotfiles. If you'd prefer that approach, I can update the PR. Let me know.

This is replacing any existence of .hbs in a file name. Should it use a regex like /.hbs$/ to make sure it only removes occurrences at the end of the file name?

Yup, thanks. Fixed.

If the extension is .hbs, your editor will highlight Handlebars syntax but will not highlight the "final" syntax of the file, e.g., "js", "markdown", "html", etc. Are you translating a problem of the editor to the code?

So i guess this is a preference thing, or maybe there're handlebars file extensions conventions I'm ignorant of.

In other templating languages I've used, I'd expect the file to have the extension of the templating language, prefixed by the extension for the outputted file. This is as much for developer ergonomics as IDEs. In my opinion - I'd prefer to look at a filename and know what kind of file it was.

IDK that IDE's can correctly highlight js in a '.js' file if its interspersed with handlebars syntax anyway? Vscode was certainly freaking out a bit for me (more around linting that syntax highlighting IIRC). I'd think IDE's have a better chance of perfectly highlighting a .js.hbs than a .js w/ hbs inside, even if today that means hbs higlighting and no js highlighting.

Appreciate this is all maybe a bit subjective, and happy for you to do whatever you think's best with this PR.

Thanks again for the project, and your time.

fmvilas commented 5 years ago

Awesome! Thanks for the explanation!

Honestly, I think this is a problem editors have to solve. Think, for instance, how every editor supports HTML+CSS+JS even when the extension is .html. That said, given the reality is different and you are the second person who suggest this change already, I think this PR does have value and it's worth merging.

Thanks for taking the time to contribute and explain it so well!

fmvilas commented 5 years ago

It's published on npm, version 0.11.0. Thanks, @peterlenagh!