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

Can't get task description and flags because returning wrapped function #65

Closed
sttk opened this issue Jun 29, 2016 · 7 comments
Closed

Comments

@sttk
Copy link
Contributor

sttk commented Jun 29, 2016

By modification that gulp.task(name) returns wrapped function instead of original function, gulp-cli can't get task description and flags now.

Since gulp.task(name).unwrap() returns original function, we can get description and flags by gulp.task(name).unwrap().description and gulp.task(name).unwrap().flags.

Is this intentional way?

@phated
Copy link
Member

phated commented Jun 29, 2016

@sttk that was an unintentional regression with this change. This change needed to be implemented to fix gulpjs/gulp#1492

Do you think we should mirror those properties onto the wrapper function? It feels like that leaks gulp details. I think the best solution would be to use unwrap() in gulp-cli. Thoughts?

@sttk
Copy link
Contributor Author

sttk commented Jun 30, 2016

@phated I've modified gulp-cli to adapt to both v1.0.0 and previous versions of undertaker in this PR.

I think the best solution would be to use unwrap() in gulp-cli.

I think it is OK, but does the name unwrap become official?
Users could set task description and flags in following way:

gulp.task('clean', function() { ... });
gulp.task('clean').description = '...';

gulp.task('default', gulp.series(...));
gulp.task('default').description = '...';

And now users can set them as follows:

gulp.task('clean', function() { ... });
gulp.task('clean').unwrap().description = '...';

gulp.task('default', gulp.series(...));
gulp.task('default').unwrap().description = '...';

Or are these bad manners?

@sttk
Copy link
Contributor Author

sttk commented Jun 30, 2016

@phated By the way, if gulp.task(name) need not to return its original function, I think taskWrapper need not to be a function and is better to be an object. Because an object can have name property and would be suitable to have properties for a task such as description and flags

@phated
Copy link
Member

phated commented Jun 30, 2016

@sttk I've been thinking about this all morning. The wrapper function should still be a function but the CLI should check for taskWrapper.description before attempting taskWrapper.unwrap().description (and similar for flags). That should support all the old ways of supplying flags/descriptions. Remember, flags and descriptions are something that gulp (specifically gulp-cli) care about and shouldn't leak into undertaker.

@sttk
Copy link
Contributor Author

sttk commented Jun 30, 2016

@phated description and flags are examples and what I want to say above are about any properties and about extensibility of undertaker or applications using it.

But this is a mere idea. I accept your saying that the wrapper should be a function.

@sttk
Copy link
Contributor Author

sttk commented Aug 22, 2016

@phated This issue was already solved by gulp-cli PR #87. Can I close this?

@phated
Copy link
Member

phated commented Aug 22, 2016

@sttk thanks for reminding me. I forgot there was an issue about this opened over here.

@phated phated closed this as completed Aug 22, 2016
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

No branches or pull requests

2 participants