Skip to content

Show tapped casks.#4357

Merged
reitermarkus merged 7 commits intoHomebrew:masterfrom
reitermarkus:tapped-cask-count
Jul 2, 2018
Merged

Show tapped casks.#4357
reitermarkus merged 7 commits intoHomebrew:masterfrom
reitermarkus:tapped-cask-count

Conversation

@reitermarkus
Copy link
Copy Markdown
Member

@reitermarkus reitermarkus commented Jun 19, 2018 •
edited
Loading

  • 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?

Also show cask count when tapped in addition to 0 formulae.

==> Tapping homebrew/cask
Tapped 0 formulae (4,083 files, 4MB)

@ghost ghost assigned reitermarkus Jun 19, 2018
@ghost ghost added the in progress Maintainers are working on this label Jun 19, 2018
MikeMcQuaid
MikeMcQuaid reviewed Jun 19, 2018
View reviewed changes
Comment thread Library/Homebrew/tap.rb Outdated
Copy link
Copy Markdown
Member

@MikeMcQuaid MikeMcQuaid Jun 19, 2018

Choose a reason for hiding this comment

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

How about not outputting formulae or casks if they are zero? Just an idea (and I can be talked out of it)

Copy link
Copy Markdown
Contributor

@apjanke apjanke Jun 20, 2018

Choose a reason for hiding this comment

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

I like this idea. How about adding a command count, too? Like:

Tapped 3 casks.
Tapped 14 casks and 2 commands.
Untapped 2349 formulae.
Untapped 2673 formulae and 5 commands.

Copy link
Copy Markdown
Member

@MikeMcQuaid MikeMcQuaid Jun 20, 2018

Choose a reason for hiding this comment

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

Yeh, that would be good 👍

@reitermarkus reitermarkus force-pushed the tapped-cask-count branch 2 times, most recently from aedab91 to dd04532 Compare June 20, 2018 18:37
@reitermarkus
Copy link
Copy Markdown
Member Author

reitermarkus commented Jun 20, 2018

Also added commands.

MikeMcQuaid
MikeMcQuaid reviewed Jun 21, 2018
View reviewed changes
Comment thread Library/Homebrew/utils/formatter.rb Outdated
Copy link
Copy Markdown
Member

@MikeMcQuaid MikeMcQuaid Jun 21, 2018

Choose a reason for hiding this comment

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

array_comma_and or something to me would read a bit more obviously; I was a bit confused as to what enumeration meant.

Comment thread Library/Homebrew/tap.rb Outdated
Copy link
Copy Markdown
Member

@MikeMcQuaid MikeMcQuaid Jun 21, 2018

Choose a reason for hiding this comment

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

The &.prepend(" ") is neat; I wonder if it's worth making this an optional parameter for Formatter.enumeration

MikeMcQuaid
MikeMcQuaid reviewed Jul 1, 2018
View reviewed changes
Comment thread Library/Homebrew/utils/formatter.rb Outdated
Copy link
Copy Markdown
Member

@MikeMcQuaid MikeMcQuaid Jul 1, 2018

Choose a reason for hiding this comment

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

Is there an upstream bug you can link to here?

Copy link
Copy Markdown
Member Author

@reitermarkus reitermarkus Jul 1, 2018

Choose a reason for hiding this comment

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

Done.

MikeMcQuaid
MikeMcQuaid reviewed Jul 1, 2018
View reviewed changes
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.

A few non-blocking comments

Comment thread Library/Homebrew/cmd/tap-info.rb Outdated
info += ", private" if tap.private?
if (formula_count = tap.formula_files.size).positive?
info += ", #{Formatter.pluralize(formula_count, "formula")}"
if (contents = tap.contents).empty?
Copy link
Copy Markdown
Member

@MikeMcQuaid MikeMcQuaid Jul 1, 2018

Choose a reason for hiding this comment

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

info += if?

Copy link
Copy Markdown
Member Author

@reitermarkus reitermarkus Jul 1, 2018

Choose a reason for hiding this comment

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

👍

Comment thread Library/Homebrew/tap.rb

formula_count = formula_files.size
puts "Tapped #{Formatter.pluralize(formula_count, "formula")} (#{path.abv})" unless quiet
formatted_contents = Formatter.comma_and(*contents)&.prepend(" ")
Copy link
Copy Markdown
Member

@MikeMcQuaid MikeMcQuaid Jul 1, 2018

Choose a reason for hiding this comment

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

Thoughts about the prepend being part of comma_and?

Copy link
Copy Markdown
Member Author

@reitermarkus reitermarkus Jul 1, 2018

Choose a reason for hiding this comment

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

I don't think it's worth it.

Copy link
Copy Markdown
Member

@MikeMcQuaid MikeMcQuaid Jul 2, 2018

Choose a reason for hiding this comment

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

My thinking was it's currently an accessory function where the only use is with the prepend?

Copy link
Copy Markdown
Member Author

@reitermarkus reitermarkus Jul 2, 2018

Choose a reason for hiding this comment

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

Still, I think it‘s not worth it:

Formatter.comma_and(*contents)&.prepend(" ")

vs.

Formatter.comma_and(*contents, prepend: " ")

and a more complex comma_and.

@reitermarkus reitermarkus merged commit cbc3184 into Homebrew:master Jul 2, 2018
@ghost ghost removed the in progress Maintainers are working on this label Jul 2, 2018
@reitermarkus reitermarkus deleted the tapped-cask-count branch July 2, 2018 18:46
@lock lock bot added the outdated PR was locked due to age label Aug 1, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Aug 1, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@MikeMcQuaid MikeMcQuaid MikeMcQuaid approved these changes

+1 more reviewer

@apjanke apjanke apjanke left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@reitermarkus reitermarkus

Labels

outdated PR was locked due to age

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@reitermarkus @MikeMcQuaid @apjanke