-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
Show dependency status for casks (✔/✘) #17982
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
base: master
Are you sure you want to change the base?
Conversation
|
Please provide a better issue/pull request title and/or description! |
1a92ff5 to
f6318c6
Compare
|
The issue that I run into so far is that when cask dependencies are just strings, so it fails when trying to check if they are satisfies in For the formula the dependencies a a fully-fledged type. |
|
@HaraldNordgren I'm game to help more if you want to keep this open? |
|
Thanks @MikeMcQuaid! After the vacation period, I am now back on my full-time job again. Combined with having kids, this doesn't leave a ton of time to work on open source, unfortunately. I will re-open this if I get some inspiration! 🤗 |
|
@HaraldNordgren Thanks for the heads up! |
f6318c6 to
84510d4
Compare
|
Hi @MikeMcQuaid! One year later and I'm on winter vacation so I have some time again 🎄 I think I got it to work now, do you want to take a look? 🤗 |
Library/Homebrew/utils/output.rb
Outdated
| def pretty_uninstalled(string) | ||
| if !$stdout.tty? | ||
| string | ||
| "#{string} ✘" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without these changes, I couldn't find a way to assert my changes.
Unfortunately, this requires changes to a lot of other tests. Is there a better way?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that stdout.tty is mocked to false. Cannot accept these changes as-is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MikeMcQuaid Is there a canonical way to achieve that? I tried a few things before and couldn't get that to work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MikeMcQuaid I found a way now, there will be a lot of control characters in the test output, is it worth it?
MikeMcQuaid
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good so far, bar comment.
Library/Homebrew/utils/output.rb
Outdated
| def pretty_uninstalled(string) | ||
| if !$stdout.tty? | ||
| string | ||
| "#{string} ✘" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that stdout.tty is mocked to false. Cannot accept these changes as-is.
When showing cask dependency info, also show the installation status.
This mirrors how it already works for formulas.