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
brew list shows cask without option #12003
brew list shows cask without option #12003
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good so far, nice work!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good so far! A few suggestions
@hyuraku If I'm correct and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just my comments above and then it looks good to me!
I also wouldn't mind verification (maybe from @cnnrmnn) that the change to to_formulae_to_casks
is okay but I'm pretty sure it's fine so probably not necessary to merge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change to to_formulae_to_casks
looks fine to me! Might be worth thinking about reorganizing that family of methods in the future. Is a bit strange that methods meant to return formulae and casks will return kegs if given some methods
and formulae if given others.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, one more naming thing I just noticed. The variables don't really contain the names but rather contain the actual kegs and casks themselves. After this, ✅ from me
Library/Homebrew/cmd/list.rb
Outdated
@@ -132,7 +132,10 @@ def list | |||
system_command! "find", args: args.named.to_default_kegs.map(&:to_s) + %w[-not -type d -print], | |||
print_stdout: true | |||
else | |||
args.named.to_default_kegs.each { |keg| PrettyListing.new keg } | |||
formula_names, cask_names = args.named.to_formulae_to_casks(method: :default_kegs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
formula_names, cask_names = args.named.to_formulae_to_casks(method: :default_kegs) | |
kegs, casks = args.named.to_formulae_to_casks(method: :default_kegs) |
Library/Homebrew/cmd/list.rb
Outdated
args.named.to_default_kegs.each { |keg| PrettyListing.new keg } | ||
formula_names, cask_names = args.named.to_formulae_to_casks(method: :default_kegs) | ||
|
||
formula_names.each { |keg| PrettyListing.new keg } if formula_names.present? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
formula_names.each { |keg| PrettyListing.new keg } if formula_names.present? | |
kegs.each { |keg| PrettyListing.new keg } if kegs.present? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @hyuraku! Nice work!
brew style
with your changes locally?brew typecheck
with your changes locally?brew tests
with your changes locally?solve #11983