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

Add format_command_args(). #6290

Merged
merged 4 commits into from Feb 24, 2019
Merged

Add format_command_args(). #6290

merged 4 commits into from Feb 24, 2019

Conversation

@cjerdonek
Copy link
Member

@cjerdonek cjerdonek commented Feb 22, 2019

This PR does mainly two things:

  1. It refactors out from call_subprocess() a simple helper function to format command arguments for display. I called this function format_command_args(). This lets us use this function in the recently created function get_legacy_build_wheel_path(), which I have done in this PR (resulting in a nicer display for that function). Refactoring the function out also (1) makes call_subprocess() simpler, and (2) makes it easier to test the functionality (tests were also added).

  2. It simplifies the implementation using shlex.quote() -- also making it copy-pasteable.

The PR is broken up into separate commits, each of which does something different. For example, simplifying the implementation is done in its own commit.

@cjerdonek cjerdonek added this to the Print Better Error Messages milestone Feb 22, 2019
@cjerdonek cjerdonek force-pushed the add-format-command branch from 8453d62 to 1c144bb Feb 22, 2019
@cjerdonek cjerdonek force-pushed the add-format-command branch from 1c144bb to 87f0539 Feb 22, 2019
@pradyunsg
Copy link
Member

@pradyunsg pradyunsg commented Feb 24, 2019

Super clean PR! I learnt stuff here @cjerdonek. :)

Loading

@cjerdonek
Copy link
Member Author

@cjerdonek cjerdonek commented Feb 24, 2019

Aw, thank you! 😊

Loading

@cjerdonek cjerdonek merged commit b882399 into pypa:master Feb 24, 2019
29 checks passed
Loading
@cjerdonek cjerdonek deleted the add-format-command branch Feb 24, 2019
@lock
Copy link

@lock lock bot commented May 28, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

Loading

@lock lock bot added the S: auto-locked label May 28, 2019
@lock lock bot locked as resolved and limited conversation to collaborators May 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants