Skip to content

Add template overrides from files in new config#653

Closed
jacklee1792 wants to merge 2 commits intooapi-codegen:masterfrom
jacklee1792:feature/file-template-override
Closed

Add template overrides from files in new config#653
jacklee1792 wants to merge 2 commits intooapi-codegen:masterfrom
jacklee1792:feature/file-template-override

Conversation

@jacklee1792
Copy link
Copy Markdown

@jacklee1792 jacklee1792 commented Jul 1, 2022

Closes #607.

This PR introduces template file overrides from filenames. Overrides listed in output-options.user-template-files will just override whatever is given by output-options.user-templates, so this doesn't break any existing configuration files.

Example usage:

package: mypackage
generate:
  models: true
  client: true
output:
  test.gen.go
output-options:
  user-template-files:
    client.tmpl: ./templates/my-client.tmpl
    client-with-responses.tmpl: ./templates/my-client-with-responses.tmpl

@Celant
Copy link
Copy Markdown

Celant commented Aug 1, 2022

Replacing the templates in the startup seems to break the template functions for me. I've tested overriding the templates in pkg/codegen/codegen.go instead, and that seems to work properly. I've tested this in my fork, and it works correctly. I'd suggest updating your PR to move the config overrides into that file, or if you'd prefer, I can make a PR for my fork.

@jacklee1792
Copy link
Copy Markdown
Author

jacklee1792 commented Aug 1, 2022

@Celant Interesting, do you know why moving the overrides to codegen.go changes the behaviour? As far as I can tell, if you have old-config-style set to false it should be the same.

In any case I think your solution is better, feel free to open a PR for your fork. I'll close mine once yours is up.

@w32blaster
Copy link
Copy Markdown

w32blaster commented Aug 5, 2022

@Celant hey mate, can you make a release for your branch, please? I heavily relied on template overriding in my project. Thanks!

@chanced chanced mentioned this pull request Aug 5, 2022
Closed
@jamietanna jamietanna self-requested a review September 6, 2022 08:24
@jeroendk
Copy link
Copy Markdown
Contributor

jeroendk commented Oct 27, 2022

any plans on moving this PR forward?

@stevenh
Copy link
Copy Markdown
Contributor

stevenh commented Nov 12, 2022 •
edited
Loading

Liking this functionality, allows for individual templates to be picked from different directories or file names.

That said its still not as easy to use as -templates <dir> on the command line, which I suspect is actually the most command and useful case.

I just submitted #852 as having to have go .tmpl files in a .yaml file really isn't usable. It restores compatibility with the old command switches which still have their place.

With all this in mind would you consider adding a user-template-dir to this PR?

stevenh
stevenh suggested changes Nov 12, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@stevenh stevenh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some style comments below, but in addition to this I would move this into the codegen package so it works for uses of that and not just oapi-codegen command line.

Comment thread cmd/oapi-codegen/oapi-codegen.go Outdated
Comment thread cmd/oapi-codegen/oapi-codegen.go Outdated
Comment thread cmd/oapi-codegen/oapi-codegen.go Outdated
Comment thread cmd/oapi-codegen/oapi-codegen.go Outdated
Comment thread cmd/oapi-codegen/oapi-codegen.go
@jacklee1792 jacklee1792 force-pushed the feature/file-template-override branch from d70b518 to b031c5b Compare November 24, 2022 00:54
@jacklee1792
Copy link
Copy Markdown
Author

jacklee1792 commented Nov 24, 2022

In the latest commits I've combined ideas from master...Celant:oapi-codegen:feat/user-template-files together with #707.

Template overrides can now be specified by any of the following, in order of lowest to highest precedence:

  • user-templates-dir
  • user-template-files
  • user-templates

@jacklee1792
Copy link
Copy Markdown
Author

jacklee1792 commented Dec 5, 2022

Hey @jamietanna @deepmap-marcinr, are there any plans to move forward with this PR or to provide custom template functionality some other way? As it is right now custom templates are basically unusable without either providing them inline in the config file or writing a custom wrapper around codegen.

@debuggerpk
Copy link
Copy Markdown

debuggerpk commented Dec 5, 2022

This PR is incredibly useful. Thanks @jacklee1792

@kNoAPP
Copy link
Copy Markdown

kNoAPP commented Mar 30, 2023

Hey @jamietanna @deepmap-marcinr, are there any plans to move forward with this PR or to provide custom template functionality some other way? As it is right now custom templates are basically unusable without either providing them inline in the config file or writing a custom wrapper around codegen.

I would love to see some love or activity on this PR. It is a pretty useful feature.

@jamietanna
Copy link
Copy Markdown
Member

jamietanna commented Sep 2, 2023

Apologies on the delay with this one folks! Will try and get a look at it for the next release - would it be possible to get this branch up-to-date with the current state of the repo? 🙏

@jamietanna jamietanna added this to the v1.15.0 milestone Sep 2, 2023
@jacklee1792
Copy link
Copy Markdown
Author

jacklee1792 commented Sep 7, 2023

Apologies on the delay with this one folks! Will try and get a look at it for the next release - would it be possible to get this branch up-to-date with the current state of the repo? 🙏

@jamietanna In rebasing I found #968 already allows loading from files, this PR doesn't do much more than adding another option to load from a directory instead of specifying the file mapping one-by-one. So feel free to close!

@jamietanna jamietanna modified the milestones: v1.16.0, v2.1.0 Oct 23, 2023
@jamietanna jamietanna mentioned this pull request Jan 25, 2024
Closed
@jamietanna
Copy link
Copy Markdown
Member

jamietanna commented Jan 25, 2024

Interesting, thanks for letting me know! Appreciate the work (and apologies for the delay) with this one.

@jamietanna jamietanna closed this Jan 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jamietanna jamietanna Awaiting requested review from jamietanna

1 more reviewer

@stevenh stevenh stevenh requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v2.1.0

Development

Successfully merging this pull request may close these issues.

Template overrides in new config style not possible?

8 participants

@jacklee1792 @Celant @w32blaster @jeroendk @stevenh @debuggerpk @kNoAPP @jamietanna