feat(hooks): support interactive hook scripts#1925
feat(hooks): support interactive hook scripts#1925PhilipNelson5 wants to merge 2 commits intocommitizen-tools:masterfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1925 +/- ##
=======================================
Coverage 97.99% 97.99%
=======================================
Files 60 60
Lines 2691 2691
=======================================
Hits 2637 2637
Misses 54 54 ☔ View full report in Codecov by Sentry. |
98d622f to
8746afc
Compare
There was a problem hiding this comment.
Then how should we test the cmd.run? Is the test for that removed?
There was a problem hiding this comment.
I'm not aware of the project history. I don't know if it was ever tested.
There was a problem hiding this comment.
I see. I just misunderstood what the test test_run does here.
|
Please fix the coverage pipeline, thanks |
|
@bearomorphism I added tests for |
da45b62 to
d483e68
Compare
|
Thank you for fixing the pipeline failures! @PhilipNelson5 @woile please review this feature |
| """ | ||
| if env is not None: | ||
| env = {**os.environ, **env} | ||
| return subprocess.run(cmd, shell=True, env=env).returncode |
There was a problem hiding this comment.
According to #1918 using shell=True is a security issue if True, should we remove it?
There was a problem hiding this comment.
Just removing shell=True causes 100 failed tests and 296 errors. It's probably outside the scope of this PR to fix that.
Description
When a pre/post bump hook needs user input it fails because stdin, stdout, and stderr are redirected. This feature adds a new cmd utility to run a command without capturing/redirecting input or output to the subprocess. This allows interactive hooks to request input and display output to the user interactively.
Checklist
Was generative AI tooling used to co-author this PR?
Code Changes
uv run poe alllocally to ensure this change passes linter check and testsUpdate the documentation for the changesDocumentation Changes
Runuv run poe doclocally to ensure the documentation pages renders correctlyCheck and fix any broken links (internal or external)Expected Behavior
When a pre/post bump hook tries to get user input, it successfully does so.
Steps to Test This Pull Request
cz bump.Additional Context