Skip to content

cli/parser: improve name_to_option for some cask args#10139

Closed
Rylan12 wants to merge 1 commit intoHomebrew:masterfrom
Rylan12:fix-cask-arg-names
Closed

cli/parser: improve name_to_option for some cask args#10139
Rylan12 wants to merge 1 commit intoHomebrew:masterfrom
Rylan12:fix-cask-arg-names

Conversation

@Rylan12
Copy link
Copy Markdown
Member

@Rylan12 Rylan12 commented Dec 24, 2020

  • 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 typecheck with your changes locally?
  • Have you successfully run brew tests with your changes locally?
  • Have you successfully run brew man locally and committed any changes?

Follow up to #10133

Previously (note that --input_methoddir is incorrectly displayed as --input-methoddir):

$ brew install --input_methoddir=abc --formula safe-rm
[...]
Error: Invalid usage: Options --formula and --input-methoddir are mutually exclusive.

Now (note the correct --input_methoddir flag is shown):

$ brew install --input_methoddir=abc --formula safe-rm
[...]
Error: Invalid usage: Options --formula and --input_methoddir are mutually exclusive.

CC @issyl0 and @reitermarkus

@BrewTestBot
Copy link
Copy Markdown
Contributor

Review period will end on 2020-12-25 at 19:18:45 UTC.

@BrewTestBot BrewTestBot added the waiting for feedback Merging is blocked until sufficient time has passed for review label Dec 24, 2020
Copy link
Copy Markdown
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Rylan12! I think it'd be nice to update these flags to be consistent with the others eventually, though.

@Rylan12
Copy link
Copy Markdown
Member Author

Rylan12 commented Dec 25, 2020

Agreed. I'll look into it.

@Rylan12
Copy link
Copy Markdown
Member Author

Rylan12 commented Dec 25, 2020

This won't be needed if #10147 is merged, so putting on hold until a decision is made there.

@BrewTestBot BrewTestBot removed the waiting for feedback Merging is blocked until sufficient time has passed for review label Dec 25, 2020
@BrewTestBot
Copy link
Copy Markdown
Contributor

Review period ended.

@Rylan12
Copy link
Copy Markdown
Member Author

Rylan12 commented Dec 29, 2020

#10147 has been merged so this is no longer needed.

@Rylan12 Rylan12 closed this Dec 29, 2020
@Rylan12 Rylan12 deleted the fix-cask-arg-names branch December 29, 2020 03:08
@BrewTestBot BrewTestBot added the outdated PR was locked due to age label Jan 28, 2021
@Homebrew Homebrew locked as resolved and limited conversation to collaborators Jan 28, 2021
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.

3 participants