Skip to content

cmd/list.rb: --formula and --cask as default option on TTY #8851

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 13, 2020
Merged

cmd/list.rb: --formula and --cask as default option on TTY #8851

merged 1 commit into from
Oct 13, 2020
+26 −12

Conversation

Akylzhan
Copy link
Contributor

@Akylzhan Akylzhan commented Oct 4, 2020 •
edited by MikeMcQuaid
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?
  • Have you successfully run brew man locally and committed any changes?

Added clarifications suggested in #8668 (comment)

Fixes #8910

Sorry, something went wrong.

@reitermarkus
Copy link
Member

I suggested this: #8668 (comment)

@reitermarkus
Copy link
Member

Basically:

  • Show everything when stdout is a TTY.
  • When stdout is not a TTY, show only formulae and print a warning that this behaviour is deprecated and --formulae should be passed explicitly.

@Akylzhan
Copy link
Contributor Author

Akylzhan commented Oct 4, 2020

Basically:

  • Show everything when stdout is a TTY.
  • When stdout is not a TTY, show only formulae and print a warning that this behaviour is deprecated and --formulae should be passed explicitly.

Sorry, did not notice your comment. But I have replied here #8668 (comment)
Also, will update manpages tomorrow

@Akylzhan Akylzhan changed the title cmd/list.rb: clarify --formula as default option cmd/list.rb: --formula and --cask as default option on TTY Oct 5, 2020
@reitermarkus
Copy link
Member

Probably the bash/zsh completions need to be changed to use --formula.

@Akylzhan
Copy link
Contributor Author

Akylzhan commented Oct 5, 2020 •
edited
Loading

Also, I have a question, how to modify manpages? Should I write the changes in docs/Manpage.md and then apply brew man?

@reitermarkus
Copy link
Member

Yes, just run brew man and commit the changes.

@Akylzhan
Copy link
Contributor Author

Akylzhan commented Oct 5, 2020 •
edited
Loading

Also, how to test only list_spec.rb? brew tests only=list_spec.rb does not work if I am in test/cmd folder

@reitermarkus
Copy link
Member

brew tests --only=cmd/list

@Akylzhan
Copy link
Contributor Author

Akylzhan commented Oct 6, 2020

That's strange, brew test-bot --only-formulae --test-default-formula does not throw error on macOS, but on ubuntu it does

reitermarkus
reitermarkus reviewed Oct 8, 2020
View reviewed changes
Library/Homebrew/cmd/list.rb Outdated
safe_system "ls", *ls_args, HOMEBREW_CELLAR
else
safe_system "ls", *ls_args, HOMEBREW_CELLAR
print list_casks(args: args) unless args.formula?
Copy link
Member

@reitermarkus reitermarkus Oct 8, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
print list_casks(args: args) unless args.formula?
list_casks(args: args) unless args.formula?

Copy link
Contributor Author

@Akylzhan Akylzhan Oct 8, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Hm, tests throw error because list returns list_casks and not list Cellar, right?

@Akylzhan Akylzhan mentioned this pull request Oct 9, 2020
Merged
@Akylzhan
cmd/list.rb: --formula, --cask as default on TTY outputs
c8809ce 
cmd/list.rb: --formula, --cask as default TTY:

cmd/list.rb: proper deprecated message on non TTY outputs

update manpage

update zsh completion

updated manpages/brew.1

update tests

return list_casks

list_spec.rb: not output to stderr
MikeMcQuaid
MikeMcQuaid approved these changes Oct 12, 2020
View reviewed changes
Copy link
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.

CI is 💚. I'm 👍🏻 once @reitermarkus is.

@MikeMcQuaid
Copy link
Member

Merging as this fixes #8910.

Thanks so much for your contribution! Without people like you submitting PRs we couldn't run this project. You rock, @Akylzhan!

@MikeMcQuaid MikeMcQuaid merged commit a0a9fa2 into Homebrew:master Oct 13, 2020
@Akylzhan Akylzhan deleted the patch branch October 13, 2020 10:59
miketheman added a commit to miketheman/rvm that referenced this pull request Oct 24, 2020
@miketheman
Update brew list command
5f84654 
Homebrew has begun emitting a deprecation warning when checking for
requirements on osx as of Homebrew/brew#8851

Example output when installing a Ruby:

```
rvm install 2.7.2
Searching for binary rubies, this might take some time.
No binary rubies available for: osx/10.15/x86_64/ruby-2.7.2.
Continuing with compilation. Please read 'rvm help mount' to get more information on binary rubies.
Checking requirements for osx.
Warning: Calling `brew list` to only list formulae is deprecated! Use `brew list --formula` instead.
... (repeated ~12 times on my system)
Warning: Calling `brew list` to only list formulae is deprecated! Use `brew list --formula` instead.
Certificates bundle '/usr/local/etc/openssl@1.1/cert.pem' is already up to date.
Requirements installation successful.
...
```

This does not impact the execution, and the command completes
successfully, however it's extra noise for the end user.

The `--formula` flag was added back in August in Homebrew/brew#8229
and brew will auto-update in most scenarios - but users who have set the
environment variable to not auto-update brew may fail this command with:

    Error: ambiguous option: --formula

I do not have any insight when brew may handle the deprecation warning
and remove the existing behavior.

Signed-off-by: Mike Fiedler <miketheman@gmail.com>
@miketheman miketheman mentioned this pull request Oct 24, 2020
Merged
@soilSpoon soilSpoon mentioned this pull request Nov 27, 2020
Merged
This was referenced Dec 1, 2020
Merged
Merged
scpeters
scpeters reviewed Dec 1, 2020
View reviewed changes
Library/Homebrew/cmd/list.rb
@@ -86,7 +86,13 @@ def list
ls_args << "-r" if args.r?
ls_args << "-t" if args.t?

safe_system "ls", *ls_args, HOMEBREW_CELLAR
if !$stdout.tty? && !args.formula?
odeprecated "`brew list` to only list formulae", "`brew list --formula`"
Copy link
Contributor

Choose a reason for hiding this comment

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

this odeprecated call was converted to odisabled in #9209; I've opened #9357 to revert that change so it will remain odeprecated for 2.6.0

@scpeters scpeters mentioned this pull request Dec 1, 2020
Merged
@BrewTestBot BrewTestBot added the outdated PR was locked due to age label Jan 1, 2021
@Homebrew Homebrew locked as resolved and limited conversation to collaborators Jan 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@reitermarkus reitermarkus

@scpeters scpeters

@MikeMcQuaid MikeMcQuaid

Assignees
No one assigned
Labels
outdated PR was locked due to age
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

zsh completion error for brew list
6 participants
@Akylzhan @reitermarkus @MikeMcQuaid @felixbuenemann @scpeters @BrewTestBot