search

hay-kot / scaffold

A cookie cutter alternative with in-project scaffolding for generating components, controllers, or other common code patterns.
https://hay-kot.github.io/scaffold/
MIT License
51 stars 5 forks source link

feat: hooks #154

Closed izeau closed 3 months ago

izeau commented 3 months ago

This PR adds an implementation for a post_scaffold hook that is run once RenderRWFS has been called. I am using it in order to generate lock files for Terraform modules, as well as running formatters on generated files. It also provides a workaround for #136: empty files can be generated using e.g. touch.

More hooks can be implemented of course :)

Please let me know what you think when you have time

hay-kot commented 3 months ago

Thanks for the PR this is super awesome and something I've been wanting to add for sure. I have a couple thoughts.

  1. Looking at cookiecutter, they also support hooks but instead use a special directory hooks to put them in and i guess just look them up by name and execute them in the given folder. What do you think of that type of API? I don't know that I have a specific preference, but I know it can be nice to write these in a named file so you get LSP and autocomplete.
  2. I like the --no-hooks flag, I was also thinking that there could be an option in the users scaffold rc for hooks, something like an enum that's like yes or no or ask and we default to ask so they are prompted to run hooks? Then --no-hooks would just override that enum?

PR looks excellent, love that you added tests!

izeau commented 3 months ago
  1. Looking at cookiecutter, they also support hooks but instead use a special directory hooks to put them in and i guess just look them up by name and execute them in the given folder. What do you think of that type of API? I don't know that I have a specific preference, but I know it can be nice to write these in a named file so you get LSP and autocomplete.

That's silly of me, checking how cookiecutter does it did not cross my mind. I do agree that their implementation has several benefits (not having to manage a temporary file is another one) -- however I believe not forcing a language or extension is for the best, as it would allow users to stay language-agnostic, or to use binaries instead. What is your stance on this? Also should I rename the post_scaffold hook into post_gen_project? I am not the best at naming things.

  1. I like the --no-hooks flag, I was also thinking that there could be an option in the users scaffold rc for hooks, something like an enum that's like yes or no or ask and we default to ask so they are prompted to run hooks? Then --no-hooks would just override that enum?

I had this idea in mind too, but was not sure on how to integrate it. This is great!

Thanks for the pointers, I will try to update the PR within the next 24 hours.

hay-kot commented 3 months ago

however I believe not forcing a language or extension is for the best, as it would allow users to stay language-agnostic, or to use binaries instead. What is your stance on this? Also should I rename the post_scaffold hook into post_gen_project?

I think post_scaffold is good! I agree that not forcing the language or extension is a good call. We can always change this in the future to match all post_* patterns, but I think limiting to exactish matches is a good start.

izeau commented 3 months ago

I have updated the branch to fix the issues you raised. There is now support for a run_hooks setting in the scaffoldrc, which can be set to either a boolean value, "yes", "no", or "prompt". I have replaced the --no-hooks flag with a more versatile --run-hooks flag, which can override the configuration set in the scaffoldrc.

When --run-hooks=prompt, the user is given the option to either run the hook, skip it, or review it (print its rendered contents). This is only available if --no-prompt is false, otherwise hooks are skipped automatically.

I am somewhat disappointed with plain white code print. I have tried to use chroma (via glamour & directly) to highlight the code but the analysis is quite limited at the time and only a handful of languages are automatically detected. I also think that giving the user the opportunity to open the rendered file in $VISUAL or $EDITOR would be nice but this adds a bit of work: detecting the editor depending on the OS, having a fallback (which one?), presumably re-loading the rendered file from disk if the user has edited it, etc. Maybe this can be done in the future?

izeau commented 3 months ago

Hi there! Sorry for the delay, I did not notice the failing tests. I have fixed the lint failures and added SIGINT handling to ensure temp files are properly cleaned up.

hay-kot commented 3 months ago

This looks great! I'm going to take a deeper look this weekend, possibly make a few tweaks and get this merged, sorry it's taking so long!

izeau commented 3 months ago

No worries! Thank you for your work!