Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

brew tab: new command for editing tab information #17449

Merged
merged 1 commit into from
Jun 13, 2024
Merged

Conversation

ZhongRuoyu
Copy link
Member

@ZhongRuoyu ZhongRuoyu commented Jun 8, 2024

  • 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 typecheck with your changes locally?
  • Have you successfully run brew tests with your changes locally?

Add brew tab, a new command to edit tab information, as previously discussed in #17125 (comment). Currently, this supports marking or unmarking formulae as installed on request.

Sample usage:

$ brew tab --installed-on-request curl
==> curl is now marked as installed on request.
$ brew autoremove --dry-run
[no output]
$ brew tab --no-installed-on-request curl
==> curl is now marked as not installed on request.
$ brew autoremove --dry-run
==> Would autoremove 2 unneeded formulae:
curl
rtmpdump

@ZhongRuoyu ZhongRuoyu added the features New features label Jun 8, 2024
@ZhongRuoyu ZhongRuoyu force-pushed the brew-mark branch 2 times, most recently from 75a5230 to ea0809b Compare June 9, 2024 13:33
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.

Thanks for this! Looks good from a brief look but hold off merging until after next stable release so I can do a proper review tomorrow. Thanks again ❤️

apainintheneck
apainintheneck previously approved these changes Jun 9, 2024
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.

Looks good so far, thanks @ZhongRuoyu!

@apainintheneck apainintheneck dismissed their stale review June 11, 2024 03:38

I'm not sure this is ready after seeing other comments.

@Bo98
Copy link
Member

Bo98 commented Jun 12, 2024

Just to note: brew install <already-installed-formula> already does the behaviour of brew mark --installed-on-request <formula> - it's just the reverse that's currently not possible.

Feels a lot to type though - might be worth shortening to auto/manual, --requested or something else?

@ZhongRuoyu
Copy link
Member Author

might be worth shortening to auto/manual, --requested or something else?

For consistency I chose to go with the flags already present in brew leaves:

$ brew leaves --help
Usage: brew leaves [--installed-on-request] [--installed-as-dependency]

List installed formulae that are not dependencies of another installed formula
or cask.

  -r, --installed-on-request       Only list leaves that were manually
                                   installed.
  -p, --installed-as-dependency    Only list leaves that were installed as
                                   dependencies.

But perhaps we can add short flags. Need to come up with a short flag for the negation (e.g., --no-installed-on-request), though.

@MikeMcQuaid
Copy link
Member

But perhaps we can add short flags. Need to come up with a short flag for the negation (e.g., --no-installed-on-request), though.

I feel given it's less "safe" (i.e. can trigger autoremove): we probably don't need a short flag for this.

Add `brew tab`, a new command to edit tab information, as previously
discussed in #17125 (comment).
Currently, this supports marking or unmarking formulae as installed on
request.

Sample usage:

    $ brew tab --installed-on-request curl
    ==> curl is now marked as installed on request.
    $ brew autoremove --dry-run
    [no output]
    $ brew tab --no-installed-on-request curl
    ==> curl is now marked as not installed on request.
    $ brew autoremove --dry-run
    ==> Would autoremove 2 unneeded formulae:
    curl
    rtmpdump

Co-authored-by: Mike McQuaid <mike@mikemcquaid.com>
@ZhongRuoyu ZhongRuoyu changed the title cmd/mark: new command brew tab: new command for editing tab information Jun 13, 2024
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.

Looks great, thanks again @ZhongRuoyu!

@MikeMcQuaid MikeMcQuaid merged commit 4f35bc1 into master Jun 13, 2024
26 checks passed
@MikeMcQuaid MikeMcQuaid deleted the brew-mark branch June 13, 2024 08:11
@github-actions github-actions bot added the outdated PR was locked due to age label Jul 14, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 14, 2024
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
features New features outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants