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

add `brew formula` command to show location of a formula #1972

Merged
merged 5 commits into from Feb 13, 2017

Conversation

Projects
None yet
3 participants
@timotheecour
Copy link
Contributor

timotheecour commented Feb 8, 2017

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

Full rationale for this PR is given Homebrew/homebrew-core#8935 and @ilovezfs said (Homebrew/homebrew-core#8935 (comment))

We'd consider a PR for such a command but no one is actively working on it.

@@ -15,7 +15,7 @@
Developers:
brew create [URL [--no-fetch]]
brew edit [FORMULA...]
brew (where|edit) [FORMULA...]

This comment has been minimized.

@ilovezfs

ilovezfs Feb 8, 2017

Contributor

I think I'd prefer these remain separate in the docs.

This comment has been minimized.

@timotheecour

timotheecour Feb 8, 2017

Contributor

sure, i did that because:

  • brew (info|home|options) does it above
  • i remember seeing somewhere to keep vertical size of --help small
  • as i described in Homebrew/homebrew-core#8935 brew where seems to supersede brew edit , brew cat

shall i go ahead w ur change?

This comment has been minimized.

@ilovezfs

ilovezfs Feb 8, 2017

Contributor

Let's see what Mike thinks before I send you off doing more work :)

This comment has been minimized.

@timotheecour

timotheecour Feb 8, 2017

Contributor

actually here it is in plain site in help.rb:

# NOTE Keep the lenth of vanilla --help less than 25 lines!

so how about:

  • option A: brew (formula|edit) [FORMULA...]
  • option B: brew formula [FORMULA...]
    and remove brew edit (which, again, is superseded by brew formula, whereas the other direction is not the case)

This comment has been minimized.

@ilovezfs

ilovezfs Feb 8, 2017

Contributor

Neither brew cat nor brew edit would be superseded by this command.

@ilovezfs

This comment has been minimized.

Copy link
Contributor

ilovezfs commented Feb 8, 2017

The equivalent command for commands is brew command so I wonder if this should be brew formula. If it is brew where, then I wonder if it should handle tap arguments as well. Currently you can do

iMac-TMP:~ joe$ brew --repo homebrew/games
/usr/local/Homebrew/Library/Taps/homebrew/homebrew-games

But this seems a little weird to me:

iMac-TMP:~ joe$ brew where homebrew/games
Error: No available formula with the name "homebrew/games" 
iMac-TMP:~ joe$ 
@timotheecour

This comment has been minimized.

Copy link
Contributor

timotheecour commented Feb 8, 2017

i like brew formula, will change to that

@timotheecour timotheecour changed the title add brew where command to show location of a formula add `brew formula` command to show location of a formula Feb 8, 2017

@timotheecour

This comment has been minimized.

Copy link
Contributor

timotheecour commented Feb 8, 2017

ok @ilovezfs PTAL, all comments addressed; i left off brew formula from --help so as not to block this PR; we can always edit --help in subsequent PR if needed

@@ -793,6 +794,12 @@ _brew_vendor_install() {
'--target'
}


# brew where formulae:
_brew_where() {

This comment has been minimized.

@MikeMcQuaid

MikeMcQuaid Feb 10, 2017

Member

Is it worth renaming this? Also, is it worth adding to the bash completion?

def formula
raise FormulaUnspecifiedError if ARGV.named.empty?
ARGV.resolved_formulae.each do |f|
puts "#{f.path}\n"

This comment has been minimized.

@MikeMcQuaid

MikeMcQuaid Feb 10, 2017

Member

puts should append a newline so the \n is probably unnecessary.


def formula
raise FormulaUnspecifiedError if ARGV.named.empty?
ARGV.resolved_formulae.each do |f|

This comment has been minimized.

@MikeMcQuaid

MikeMcQuaid Feb 10, 2017

Member

ARGV.resolved_formulae.each { |f| puts f.path }?

@@ -0,0 +1,15 @@
#: * `formula` <formulae>:

This comment has been minimized.

@MikeMcQuaid

MikeMcQuaid Feb 10, 2017

Member

I'd just go for <formula for consistency with other commands.

@@ -0,0 +1,15 @@
#: * `formula` <formulae>:
#: echo location of the specified <formulae> to stdout

This comment has been minimized.

@MikeMcQuaid

MikeMcQuaid Feb 10, 2017

Member

Display the path where <formula> is

@@ -88,6 +88,7 @@ __brew_common_commands() {
'doctor:audits your installation for common issues'
'edit:edit a formula'
'fetch:download formula resources to the cache'
'formula:location of formulae'

This comment has been minimized.

@MikeMcQuaid

MikeMcQuaid Feb 10, 2017

Member

formula: the path for a formula

@timotheecour timotheecour force-pushed the timotheecour:pr_brew_where_command branch from 4d8af61 to 0d715f9 Feb 12, 2017

@timotheecour

This comment has been minimized.

Copy link
Contributor

timotheecour commented Feb 12, 2017

@MikeMcQuaid PTAL (all comments addressed and rebased)

@MikeMcQuaid

This comment has been minimized.

Copy link
Member

MikeMcQuaid commented Feb 12, 2017

Thanks @timotheecour! Last thing: could you add a small integration test to test the output of this command?

@timotheecour

This comment has been minimized.

Copy link
Contributor

timotheecour commented Feb 12, 2017

@MikeMcQuaid PTAL : brew tests passes.

  • btw: it would lower barrier of entry for external contributors to add docs on specific things (eg steps involved in adding a command: where to add tests, how to update man-pages, commands to run eg brew man, brew tests, etc). Unfortunately #1973 (Update CONTRIBUTING.md) was marked as wontfix. This PR would've been easier for me if I didn't have to discover those by myself. Unfortunately also wiki was disabled. how about CONTRIBUTING-advanced.md or whatever other name if you don't want to bloat CONTRIBUTING.md ?

  • What command do i run to only run the test I'm adding?
    tried a few variants of brew tests --only=formula_cmd_test but didn't work (Pass files or folders to run error); an example in manpage would help

@MikeMcQuaid

This comment has been minimized.

Copy link
Member

MikeMcQuaid commented Feb 13, 2017

btw: it would lower barrier of entry for external contributors to add docs on specific things (eg steps involved in adding a command: where to add tests, how to update man-pages, commands to run eg brew man, brew tests, etc). Unfortunately #1973 (Update CONTRIBUTING.md) was marked as wontfix. This PR would've been easier for me if I didn't have to discover those by myself. Unfortunately also wiki was disabled. how about CONTRIBUTING-advanced.md or whatever other name if you don't want to bloat CONTRIBUTING.md ?

We'll consider documenting these things if we see multiple users struggling with them. Homebrew has thousands of contributors and millions of users; it doesn't scale to try and document every single thing an individual doesn't know.

What command do i run to only run the test I'm adding?
tried a few variants of brew tests --only=formula_cmd_test but didn't work (Pass files or folders to run error); an example in manpage would help

It's --only=formula_cmd I believe. Agreed adding to manpage would be useful 👍

--

Thanks for your first contribution to Homebrew/brew! Without people like you submitting PRs we couldn't run this project. You rock!

@MikeMcQuaid MikeMcQuaid merged commit 7281cfa into Homebrew:master Feb 13, 2017

3 checks passed

codecov/patch 100% of diff hit (target 54.18%)
Details
codecov/project Absolute coverage decreased by -0.01% but relative coverage increased by +45.81% compared to cf18a99
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@timotheecour timotheecour deleted the timotheecour:pr_brew_where_command branch Feb 13, 2017

@Homebrew Homebrew locked and limited conversation to collaborators May 3, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.