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
63 stars 8 forks source link

feat: subdirectory support for remote scaffolds #219

Closed spion closed 1 month ago

spion commented 2 months ago

Hi! Love your project, especially the ability to use local templates nested under a .scaffold subdirectory.

I've been playing with the ability to specify both a remote URL and a subdirectory a little bit. You can try this PR out by running (assuming you have gh defined in shorts)

scaffold new --subdir t2 gh:spion/scaffold-test-templates

Why do I think this feature is neat?

Scaffold is great at addressing new project initialization, as well providing templates within an existing project. However, from the perspective of a toolbuilder, I'm not able to use any of those two modes.

Examples might include concourse pipelines, github actions workflows, linting configs and so on.

I think of it as a generic cli version of e.g. github's wizard to create new GHA workflows in your repo from existing language templates.

DX considerations

Having this as part of the URL might be better, perhaps

scaffold new gh:spion/scaffold-test-templates#t2

But before we get to that (and tests/productionizing), what do you think about the concept and need in general?

hay-kot commented 2 months ago

Yeah, I really really like this. Couple initial thoughts.

  1. Does this also work with project type scaffolds in sub-directories? Like, could I put all my go project templates in one repository and use the path syntax to call each one?
  2. I also like the idea of the dx of using the # I don't see a reason why we couldn't do that, but I'm wondering if we should provide a flag as well or if we should pick one or the other. I like the idea of scaffold new snips:#eslint vs scaffold new --subdir eslint snips:

Definitely want to see this get merged, so would love to see some tests and documentation for this one. Happy to help with anything I can!

spion commented 2 months ago

Does this also work with project type scaffolds in sub-directories? Like, could I put all my go project templates in one repository and use the path syntax to call each one?

I believe it does! I see now that decision is made on whether the scaffold.yaml toplevel has a templates directory or one of the {{ .ProjectName }} variants.

As a side note, I noticed that if you use templates but don't have rewrites specified for all your files, this will end up generating a templates directory with the files in there. What do you think about modifying this behavior (not as part of this PR, of course)? My expectations would be something along these lines:

  1. If no rewrites specified but a templates directory is found, generate an error: detected templates directory but norewritesspecified for templates scaffold - nothing to do
  2. If rewrites exist, but files with no matching rewrites are present, don't generate anything from them

I also like the idea of the dx of using the # I don't see a reason why we couldn't do that, but I'm wondering if we should provide a flag as well or if we should pick one or the other. I like the idea of scaffold new snips:#eslint vs scaffold new --subdir eslint snips:

I think the order of the subdirectory as the flag makes this confusing, and people might put it after the URL, which would result with attempts to set answer values. (at least, that's what I did :grinning:)

I changed the implementation to only support #subdirectory and added some tests. I was hoping to use the go built in URL parser for this, but it seems that they ignore the fragment parts because its not a use case that's needed for writing servers - so I'm used manual string splitting to get it.

In the process, I broke scaffold update. I think it may be necessary to separate scaffold repositories from scaffolds, perhaps by looking up the first .git directories.

spion commented 2 months ago

I fixed the broken update function, however this isn't going to always work optimally as it will potentially run pull multiple times for multiple nested scaffolds. This might not be too bad - the git library should realise that HEAD is already up to date with remote HEAD and quickly continue. Let me know what you think.

I've also been thinking about different separators - e.g. // - although combined with shorts it might be either cool or confusing depending on your view :grinning:

hay-kot commented 2 months ago

As a side note, I noticed that if you use templates but don't have rewrites specified for all your files, this will end up generating a templates directory with the files in there. What do you think about modifying this behavior (not as part of this PR, of course)? My expectations would be something along these lines:

If no rewrites specified but a templates directory is found, generate an error: detected templates directory but no rewrites specified for templates scaffold - nothing to do If rewrites exist, but files with no matching rewrites are present, don't generate anything from them

I think this makes sense, there's also a lint command, so it should also be added to that as well.

I think the order of the subdirectory as the flag makes this confusing, and people might put it after the URL, which would result with attempts to set answer values. (at least, that's what I did 😀)

100% I'm good with the way that it is now. Using a fragment for the url make sense I think.

I fixed the broken update function, however this isn't going to always work optimally as it will potentially run pull multiple times for multiple nested scaffolds. This might not be too bad - the git library should realise that HEAD is already up to date with remote HEAD and quickly continue. Let me know what you think.

This is largely a by-product of my poor design for packages, I'll need to take another pass at it after this gets merged I think and tidy things up, so this problem should resolve itself before I push a release.

This looks pretty good to me, so let me know if there's anything else you think this needs, otherwise I can merge it as is!

spion commented 2 months ago

This looks pretty good to me, so let me know if there's anything else you think this needs, otherwise I can merge it as is!

If you're happy with '#' as the separator choice, go ahead!