Skip to content

brew reinstall --interactive#6997

Merged
MikeMcQuaid merged 4 commits intoHomebrew:masterfrom
sj26:reinstall-interactive
Feb 6, 2020
Merged

brew reinstall --interactive#6997
MikeMcQuaid merged 4 commits intoHomebrew:masterfrom
sj26:reinstall-interactive

Conversation

@sj26
Copy link
Copy Markdown
Contributor

@sj26 sj26 commented Feb 4, 2020

More often than I realised, I want to brew reinstall --build-from-source --interactive $FORMULA to add some custom configuration. It seems like a useful addition?

@MikeMcQuaid
Copy link
Copy Markdown
Member

@Homebrew/maintainers would any of you use this?

@claui
Copy link
Copy Markdown
Contributor

claui commented Feb 4, 2020

As a workaround, I’ve always used the following kludge:

brew uninstall $FORMULA; brew install --build-from-source --interactive $FORMULA

and I’d be more than delighted to have this feature.

@SMillerDev
Copy link
Copy Markdown
Member

I don't care specifically about interactive but I'm very much in favor of install and reinstall behaving the same.

@jonchang
Copy link
Copy Markdown
Contributor

jonchang commented Feb 4, 2020

Yes, I think that brew install and brew reinstall should have the same interface.

@igas
Copy link
Copy Markdown

igas commented Feb 4, 2020

When I bump formula and it fails locally, for debug purposes I use similar to @claui approach.

@EricFromCanada
Copy link
Copy Markdown
Member

It would also be useful to have --HEAD available, as it's a quick way of seeing if a bug in a released version has been fixed by a newer commit.

@MikeMcQuaid
Copy link
Copy Markdown
Member

I don't think it's worth making the interface identical because not all options make sense for reinstall. I'm game to add options as requested but let's not add them all en-masse (install has a lot).

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.

Thanks for the PR! You will also need to brew man and commit the changes.

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

Choose a reason for hiding this comment

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

Suggested change
reinstall_formula(f) do |fi|
reinstall_formula(f, interactive: args.interactive?)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Nice suggestion!

I wasn't sure what to do here — deep inside reinstall_formula uses ARGV.interactive? so perhaps just adding the flag to the listed flags in cmd/reinstall is enough, and remove the yield? I notice reinstall_formula is only otherwise used in cmd/upgrade which also does not list --interactive.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, just listing the switch in the switches is enough to make it work. I'll back out the rest.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sounds good!

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

Choose a reason for hiding this comment

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

Suggested change
yield fi if block_given?

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

Choose a reason for hiding this comment

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

Suggested change
fi.interactive = args.interactive?

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

Choose a reason for hiding this comment

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

Suggested change
end

@MikeMcQuaid
Copy link
Copy Markdown
Member

It would also be useful to have --HEAD available, as it's a quick way of seeing if a bug in a released version has been fixed by a newer commit.

I think brew upgrade --HEAD would be make more logical sense here. If that doesn't already work: would accept PR for that!

sj26 added 4 commits February 6, 2020 09:56
More often than I realised, I want to `brew reinstall
--build-from-source --interactive $FORMULA` to add some custom
configuration. It seems like a useful addition?
@sj26 sj26 force-pushed the reinstall-interactive branch from b998e8f to 8cd0177 Compare February 5, 2020 22:58
@sj26
Copy link
Copy Markdown
Contributor Author

sj26 commented Feb 5, 2020

I think brew upgrade --HEAD would be make more logical sense here. If that doesn't already work: would accept PR for that!

Does brew upgrade --fetch-HEAD $FORMULA already do the job?

Edit: Oh, no, I see — upgrading from non-HEAD to HEAD.

@MikeMcQuaid MikeMcQuaid merged commit f144f63 into Homebrew:master Feb 6, 2020
@MikeMcQuaid
Copy link
Copy Markdown
Member

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

@lock lock bot added the outdated PR was locked due to age label Mar 10, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Mar 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

outdated PR was locked due to age

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants