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

fix(cli): pass through --help flag for tasks that receive args #3046

Merged
merged 3 commits into from Oct 30, 2023

Conversation

icj217
Copy link
Contributor

@icj217 icj217 commented Oct 27, 2023

A common use case for tasks is to act as proxies to other CLIs/tools (e.g. pip, poetry, etc). The current behavior of displaying projen's own help document for the --help flag for tasks prevents the proper passthrough of --help flags that are intended for the downstream tool (when receiveArgs is true).

The help document shown for tasks is fairly generic and can't be customized for tasks anyway.

I considered adding an option to the task itself (e.g. disableHelp: boolean) to control this behavior, but this current implementation makes more sense to me since receiveArgs should really mean all args. I realize that --inspect will still be captured by projen, but that one actually does make sense since a) it's not a standard flag on most tools and b) the return value is useful for understanding the task.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@icj217 icj217 marked this pull request as ready for review October 27, 2023 21:09
@codecov-commenter
Copy link

codecov-commenter commented Oct 27, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (0bc2ec5) 96.33% compared to head (d5d1d52) 96.33%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3046   +/-   ##
=======================================
  Coverage   96.33%   96.33%           
=======================================
  Files         185      185           
  Lines       34576    34576           
  Branches     3212     3212           
=======================================
+ Hits        33308    33310    +2     
+ Misses       1268     1266    -2     

see 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mrgrain
Copy link
Contributor

mrgrain commented Oct 27, 2023

Thanks! And I agree with your assessment.

Code looks good. You just need to make the build checks pass.

test/cli.test.ts Outdated Show resolved Hide resolved
@mrgrain
Copy link
Contributor

mrgrain commented Oct 30, 2023

realize that --inspect will still be captured by projen, but that one actually does make sense since a) it's not a standard flag on most tools and b) the return value is useful for understanding the task.

Long term future: I kind of think the task runner should register new and inspect as default tasks and fail if a project tries to register another task of this name.

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.

None yet

3 participants