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

Fix brew completion for `brew install` #3309

Merged
merged 5 commits into from Aug 19, 2016

Conversation

Projects
None yet
3 participants
@tjjh89017
Contributor

tjjh89017 commented Aug 18, 2016

Description

Since brew mirgate Formula into Taps diectory
Change the way to find out all Homebrew-Core Formula
Compatible with old version Homebrew

Fix brew completion for `brew install`
Since brew mirgate Formula into Taps diectory
Change the way to find out all Homebrew-Core Formula
Compatible with old version Homebrew
Show outdated Hide outdated share/completions/brew.fish
# __fish_complete_suffix .rb
ls $formuladir/*.rb | sed 's/.rb$//' | sed "s|^$formuladir||"
find $formuladir -name "*.rb" -type f 2> /dev/null | sed 's/.rb$//' | sed 's|^/\([^/]*/\)*||'

This comment has been minimized.

@floam

floam Aug 18, 2016

Member

Let's try to use the string builtin here instead of sed

@floam

floam Aug 18, 2016

Member

Let's try to use the string builtin here instead of sed

@floam

This comment has been minimized.

Show comment
Hide comment
@floam

floam Aug 18, 2016

Member

Might it assume less and be more future-proof (and able to generalize to other Taps) if you get the Formula directory via: brew --repository homebrew/homebrew-core?

Member

floam commented Aug 18, 2016

Might it assume less and be more future-proof (and able to generalize to other Taps) if you get the Formula directory via: brew --repository homebrew/homebrew-core?

@floam

This comment has been minimized.

Show comment
Hide comment
@floam

floam Aug 19, 2016

Member

This is pretty broken in master right now, I'm merging the fix.

Member

floam commented Aug 19, 2016

This is pretty broken in master right now, I'm merging the fix.

@floam

This comment has been minimized.

Show comment
Hide comment
@floam

floam Aug 19, 2016

Member

Wait, this is wrong:

set -l formuladir (brew --repository)/Library/Formula/ (brew --repository homebrew/core)

Member

floam commented Aug 19, 2016

Wait, this is wrong:

set -l formuladir (brew --repository)/Library/Formula/ (brew --repository homebrew/core)

@tjjh89017

This comment has been minimized.

Show comment
Hide comment
@tjjh89017

tjjh89017 Aug 19, 2016

Contributor

brew --repository homebrew/core is working fine.
But now I just use brew search to list all local formula, however it has some problem when tap-pin is used.

Contributor

tjjh89017 commented Aug 19, 2016

brew --repository homebrew/core is working fine.
But now I just use brew search to list all local formula, however it has some problem when tap-pin is used.

@floam

This comment has been minimized.

Show comment
Hide comment
@floam

floam Aug 19, 2016

Member

Something like this will get a working path for the main keg.

> echo (brew --repository homebrew/homebrew-core)/Formula
/usr/local/Library/Taps/homebrew/homebrew-core/Formula
Member

floam commented Aug 19, 2016

Something like this will get a working path for the main keg.

> echo (brew --repository homebrew/homebrew-core)/Formula
/usr/local/Library/Taps/homebrew/homebrew-core/Formula
@floam

This comment has been minimized.

Show comment
Hide comment
@floam

floam Aug 19, 2016

Member

Compared to what I was getting from the PR update:

> echo (brew --repository)/Library/Formula/ (brew --repository homebrew/core)
/usr/local/Library/Formula/ /usr/local/Library/Taps/homebrew/homebrew-core
> ls /usr/local/Library/Formula/
ls: /usr/local/Library/Formula/: No such file or directory
Member

floam commented Aug 19, 2016

Compared to what I was getting from the PR update:

> echo (brew --repository)/Library/Formula/ (brew --repository homebrew/core)
/usr/local/Library/Formula/ /usr/local/Library/Taps/homebrew/homebrew-core
> ls /usr/local/Library/Formula/
ls: /usr/local/Library/Formula/: No such file or directory
@floam

This comment has been minimized.

Show comment
Hide comment
@floam

floam Aug 19, 2016

Member

Oh, I see, probably that's fine and for compatibility with other installs, not necessarily a problem.

Member

floam commented Aug 19, 2016

Oh, I see, probably that's fine and for compatibility with other installs, not necessarily a problem.

@tjjh89017

This comment has been minimized.

Show comment
Hide comment
@tjjh89017

tjjh89017 Aug 19, 2016

Contributor

/usr/local/Library/Formula/ for compatible
find will search recursively because some custom tap won't have Formula directory.
Dont specify Formula for try to modify easily to search all formula from Taps.
But it seems useless, while using brew search instead

Contributor

tjjh89017 commented Aug 19, 2016

/usr/local/Library/Formula/ for compatible
find will search recursively because some custom tap won't have Formula directory.
Dont specify Formula for try to modify easily to search all formula from Taps.
But it seems useless, while using brew search instead

@floam

This comment has been minimized.

Show comment
Hide comment
@floam

floam Aug 19, 2016

Member

I will try this soon. How is the speed of brew search?

Member

floam commented Aug 19, 2016

I will try this soon. How is the speed of brew search?

@tjjh89017

This comment has been minimized.

Show comment
Hide comment
@tjjh89017

tjjh89017 Aug 19, 2016

Contributor

brew search seem a little slower than find & string replace using Macbook Air 2014.
But I think brew search will be the better choice for this purpose because it's portable.

2016-08-19 15 03 38

Contributor

tjjh89017 commented Aug 19, 2016

brew search seem a little slower than find & string replace using Macbook Air 2014.
But I think brew search will be the better choice for this purpose because it's portable.

2016-08-19 15 03 38

@floam

This comment has been minimized.

Show comment
Hide comment
@floam

floam Aug 19, 2016

Member

That's much better than I'm getting. Maybe something is wrong with my install.

floam@MacBook-Pro ~> brew search x > /dev/null
floam@MacBook-Pro ~> echo $CMD_DURATION 
14438

edit: it's because I had a query. Seems fine to me.

Member

floam commented Aug 19, 2016

That's much better than I'm getting. Maybe something is wrong with my install.

floam@MacBook-Pro ~> brew search x > /dev/null
floam@MacBook-Pro ~> echo $CMD_DURATION 
14438

edit: it's because I had a query. Seems fine to me.

@floam

This comment has been minimized.

Show comment
Hide comment
@floam

floam Aug 19, 2016

Member

I like it, and you can just use brew search directly now instead of wrapping it in that function.

Member

floam commented Aug 19, 2016

I like it, and you can just use brew search directly now instead of wrapping it in that function.

@tjjh89017

This comment has been minimized.

Show comment
Hide comment
@tjjh89017

tjjh89017 Aug 19, 2016

Contributor

With fresh install Hackintosh (Yosemite) last week.

$ find . -name "*.rb" -type f 2> /dev/null | string replace -r '^/(?:[^/]*/)*([^/]*)\.rb' '$1' 1> /dev/null
$ echo $CMD_DURATION
233
$ brew search 1> /dev/null
$ echo $CMD_DURATION
333
Contributor

tjjh89017 commented Aug 19, 2016

With fresh install Hackintosh (Yosemite) last week.

$ find . -name "*.rb" -type f 2> /dev/null | string replace -r '^/(?:[^/]*/)*([^/]*)\.rb' '$1' 1> /dev/null
$ echo $CMD_DURATION
233
$ brew search 1> /dev/null
$ echo $CMD_DURATION
333

@floam floam modified the milestones: critical, next-2.x Aug 19, 2016

@floam floam self-assigned this Aug 19, 2016

@floam

This comment has been minimized.

Show comment
Hide comment
@floam

floam Aug 19, 2016

Member

Let me know when you're done and happy and I'll merge this to master.

Thanks!

Member

floam commented Aug 19, 2016

Let me know when you're done and happy and I'll merge this to master.

Thanks!

@tjjh89017

This comment has been minimized.

Show comment
Hide comment
@tjjh89017

tjjh89017 Aug 19, 2016

Contributor

For brew install I think it is done.
You could merge it when anytime you want.
I will work on for other issues (e.g. tap-pin) and create another PR for fix
Thanks a lot. :)

Contributor

tjjh89017 commented Aug 19, 2016

For brew install I think it is done.
You could merge it when anytime you want.
I will work on for other issues (e.g. tap-pin) and create another PR for fix
Thanks a lot. :)

@floam floam merged commit 5dd9590 into fish-shell:master Aug 19, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@krader1961 krader1961 modified the milestones: fish 2.4.0, next-2.x Sep 3, 2016

nomaed added a commit to nomaed/fish-shell that referenced this pull request Jul 17, 2017

Fix brew completion for `brew install` (#3309)
* Fix brew completion for `brew install`
* Using `brew search` rather than `brew --repository`
- Homebrew migrated the directory holding their Formulas into Taps, breaking fish's completions.
- New method to find all Homebrew-core Formulas
- Compatible with old versions of Homebrew and more future proof
* Replace fixed path to search formula with `brew --repository`
* Replace `sed` with builtin `string replace`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment