Skip to content
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

Allow function for outFolder in dest #15

Closed
wants to merge 1 commit into from

Conversation

TomAdam
Copy link

@TomAdam TomAdam commented Apr 1, 2014

We use a PHP framework that stores assets within bundles. We want to use gulp to compile these assets but there are a large quantity of the bundles. It is obviously possible to set up a pipelines for each using a loop, but we would end up with 100's.

I propose allowing passing a function as outFolder to dest, so that the input file info can be used to manipulate the output path. I attempted to create a plugin to do this but it just ended up as a copy of dest, so decided a PR would be much cleaner. I think this feature will be useful to others, and doesn't add too much code.

It's only a proposal at this point so I haven't taken the time to write a test or document it, but if you are interested let me know and I will produce both of these.

@yocontra
Copy link
Member

yocontra commented Apr 1, 2014

Looks fine except that basePath is being persisted between files, when it should probably be scoped into the saveFile function when it happens to be a function

@TomAdam
Copy link
Author

TomAdam commented Apr 2, 2014

outFolder needs to be executed once per file so the file metadata can be used to manipulate basePath, so it can't be executed earlier. basePath could be redefined within saveFile to prevent the output of the function polluting the parent scope, but as it isn't used anywhere else I don't see how it could cause an issue. What is the issue you see?

Separately, I have an simple use case to demonstrate the feature:

gulp.src('*/assets/*')
    .pipe(gulp.dest(function(file) {
        return path.resolve(path.dirname(file.path), '../public'); 
    }));

Great to know you are interested. I'll look into getting some tests and documentation together later this week.

@yocontra
Copy link
Member

yocontra commented Apr 2, 2014

Yeah this all looks fine, just needs tests

@chmontgomery
Copy link

+1 for this

@jackp
Copy link

jackp commented Apr 11, 2014

👍

@teamrun
Copy link

teamrun commented May 23, 2014

+1 for this~
and i hope to have more documents about vinyl-fs and gulp's src and dest API

@yocontra
Copy link
Member

Does anyone want to contribute tests/documentation for this so I can merge?

@TomAdam
Copy link
Author

TomAdam commented May 23, 2014

I'd taken my eye off this one, but will put together the tests this weekend.

@yocontra
Copy link
Member

Update?

@yocontra yocontra closed this in 583d30e Jun 10, 2014
phated pushed a commit that referenced this pull request Nov 27, 2017
phated pushed a commit that referenced this pull request Nov 27, 2017
phated pushed a commit that referenced this pull request Nov 27, 2017
phated pushed a commit that referenced this pull request Nov 27, 2017
phated pushed a commit that referenced this pull request Nov 27, 2017
phated pushed a commit that referenced this pull request Nov 27, 2017
phated pushed a commit that referenced this pull request Nov 27, 2017
phated pushed a commit that referenced this pull request Nov 28, 2017
phated pushed a commit that referenced this pull request Nov 28, 2017
phated pushed a commit that referenced this pull request Nov 28, 2017
phated pushed a commit that referenced this pull request Nov 28, 2017
phated pushed a commit that referenced this pull request Dec 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants