Skip to content

cli/parser: chomp '=' from comma_array flag name#7184

Merged
MikeMcQuaid merged 2 commits intoHomebrew:masterfrom
dawidd6:cli-parser-comma-array-name-chomp
Mar 23, 2020
Merged

cli/parser: chomp '=' from comma_array flag name#7184
MikeMcQuaid merged 2 commits intoHomebrew:masterfrom
dawidd6:cli-parser-comma-array-name-chomp

Conversation

@dawidd6
Copy link
Copy Markdown
Contributor

@dawidd6 dawidd6 commented Mar 16, 2020

brew bump-formula-pr help message:

Before:

        --mirror=                    Use the specified URL as a mirror URL. If
                                     URL is a comma-separated list of URLs,
                                     multiple mirrors will be added.

After:

        --mirror                     Use the specified URL as a mirror URL. If
                                     URL is a comma-separated list of URLs,
                                     multiple mirrors will be added.
  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew tests with your changes locally?

@MikeMcQuaid
Copy link
Copy Markdown
Member

What's the advantage of dropping the =?

@dawidd6
Copy link
Copy Markdown
Contributor Author

dawidd6 commented Mar 16, 2020

Visual consistency with other flags in the help message. That's it I guess.

@MikeMcQuaid
Copy link
Copy Markdown
Member

Interested in @GauthamGoli and @EricFromCanada's thoughts.

@EricFromCanada
Copy link
Copy Markdown
Member

Unlike a switch which does not take an argument, a flag requires an argument only if defined with a trailing =, which it removes later. The question for comma_array is, since it always requires an argument, should it always be defined with a trailing =, or should it be optional and just removed during parsing? I'd say the latter, which this PR implements, and I'd also say the = should be dropped from bump-formula-pr (requiring a run of brew man).

@GauthamGoli
Copy link
Copy Markdown
Contributor

Agree with @EricFromCanada.
= is used to differentiate between Optional and Required flag args, comma_array does not need that differentiation.

@MikeMcQuaid MikeMcQuaid merged commit 5e63d0c into Homebrew:master Mar 23, 2020
@MikeMcQuaid
Copy link
Copy Markdown
Member

Thanks again @dawidd6!

@lock lock bot added the outdated PR was locked due to age label Apr 25, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Apr 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

outdated PR was locked due to age

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants