New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a copy parameter to GenerateFile #464

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@Altreus

Altreus commented Jun 15, 2015

This is more of a proposal than a PR at the moment, because obviously there's not even a test for it yet.

I thought it would be useful to have a copy option in GenerateFile. My use case is that I'm making a profile that would want to copy more files into the lib/ directory, meaning the paths to the files would want to be templated as well as the content of them.

Rather than listing a bunch of lines, it seemed prudent to do something like this.

@Altreus

This comment has been minimized.

Show comment
Hide comment
@Altreus

Altreus Jul 29, 2015

OK I updated the PR with actually-tested code. I've not looked at writing an actual test in t/ for it yet, mind.

Altreus commented Jul 29, 2015

OK I updated the PR with actually-tested code. I've not looked at writing an actual test in t/ for it yet, mind.

@rjbs

This comment has been minimized.

Show comment
Hide comment
@rjbs

rjbs Oct 13, 2015

Owner

Hi! Sorry for the delay. I'm working through my backlog again.

I'm not entirely sure I understand the problem you want to solve. Is there a reason that GatherDir::Template won't work?

At any rate, I do not like that this PR leaves it possible for both content and copy to be provided, with content being, it seems, silently ignored when copy is set.

Let me know what you want to achieve and maybe I'll be enlightened, or I can help find a way that I think is more suitable for the capability to be added. Thanks!

Owner

rjbs commented Oct 13, 2015

Hi! Sorry for the delay. I'm working through my backlog again.

I'm not entirely sure I understand the problem you want to solve. Is there a reason that GatherDir::Template won't work?

At any rate, I do not like that this PR leaves it possible for both content and copy to be provided, with content being, it seems, silently ignored when copy is set.

Let me know what you want to achieve and maybe I'll be enlightened, or I can help find a way that I think is more suitable for the capability to be added. Thanks!

@karenetheridge

This comment has been minimized.

Show comment
Hide comment
@karenetheridge

karenetheridge Oct 13, 2015

Contributor

My guess is he wants to (optionally) provide the file content in an existing file, rather than inlined in dist.ini? In that case, [GatherDir::Template] might be closer to what is needed (it doesn't have an option to template the filename yet though).

Contributor

karenetheridge commented Oct 13, 2015

My guess is he wants to (optionally) provide the file content in an existing file, rather than inlined in dist.ini? In that case, [GatherDir::Template] might be closer to what is needed (it doesn't have an option to template the filename yet though).

@Altreus

This comment has been minimized.

Show comment
Hide comment
@Altreus

Altreus Oct 15, 2015

Yes, I wanted to template the file and the filename - and [GenerateFile] seemed to be the only one that supported a templated filename.

Supplying the content inline in dist.ini just became really cumbersome, once the file became longer than a trivial skeleton, so I thought it would be easier to generate a file based on a different file.

I recognise that this probably means I should try to fix our framework to have less boilerplate in its modules this stage it's easier to make a minting profile for it and ignore the problem!

I didn't consider adding this feature to [GatherDir::Template], mostly because I didn't know about it.

Altreus commented Oct 15, 2015

Yes, I wanted to template the file and the filename - and [GenerateFile] seemed to be the only one that supported a templated filename.

Supplying the content inline in dist.ini just became really cumbersome, once the file became longer than a trivial skeleton, so I thought it would be easier to generate a file based on a different file.

I recognise that this probably means I should try to fix our framework to have less boilerplate in its modules this stage it's easier to make a minting profile for it and ignore the problem!

I didn't consider adding this feature to [GatherDir::Template], mostly because I didn't know about it.

@rjbs

This comment has been minimized.

Show comment
Hide comment
@rjbs

rjbs Oct 22, 2015

Owner

Okay! I would prefer that GatherDir::Template get the change. I'm not 100% sure how I'd suggest to implement it, but maybe something like:

[GatherDir::Template]
; will find file ./the-FOO-vBAR.txt
rewrite.FOO = $zilla->expression->whatever
rewrite.BAR = $]
Owner

rjbs commented Oct 22, 2015

Okay! I would prefer that GatherDir::Template get the change. I'm not 100% sure how I'd suggest to implement it, but maybe something like:

[GatherDir::Template]
; will find file ./the-FOO-vBAR.txt
rewrite.FOO = $zilla->expression->whatever
rewrite.BAR = $]
@Altreus

This comment has been minimized.

Show comment
Hide comment
@Altreus

Altreus Nov 12, 2015

Done in GatherDir::Template. I decided "rename" made more sense

#508

Altreus commented Nov 12, 2015

Done in GatherDir::Template. I decided "rename" made more sense

#508

@Altreus Altreus closed this Nov 12, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment