Support languages when installing from the API
#14456
Conversation
Rylan12
added
critical
|
Review period ended. |
|
Install |
| if json_cask[:artifacts].any? { |artifact| FLIGHT_STANZAS.include?(artifact.keys.first) } | ||
| # Use the cask-source API if there are any `*flight` blocks or the cask has multiple languages | ||
| if json_cask[:artifacts].any? { |artifact| FLIGHT_STANZAS.include?(artifact.keys.first) } || | ||
| json_cask[:languages].any? |
There was a problem hiding this comment.
For post-this PR: this makes me wonder if we actually want to include all artefacts and languages in the JSON. It's could to bloat it quite a bit unnecessarily?
There was a problem hiding this comment.
Yeah, it might be okay to do. Some casks like firefox have 51 languages, but most casks don't have any at all. Think it's worth doing it that way?
There was a problem hiding this comment.
@Rylan12 Perhaps just compare the size of the overall (gzipped) JSON output? It's it's noticeable then optimise, if not: leave it as-is.
There was a problem hiding this comment.
Why gzipped? We don't currently compress the JSON files before downloading, right?
I will compare later when I have some more time.
There was a problem hiding this comment.
@Rylan12 I think/thought we're downloading them gziped. If not and we can do that transparently (a non-trivial number of web servers will do this): we should.
There was a problem hiding this comment.
I will compare later when I have some more time.
Cool, no rush! Happy to ship this as-is for now?
There was a problem hiding this comment.
Yes, sorry for the delay. Let's merge and re-evaluate
There was a problem hiding this comment.
I checked: we use --compressed which means we will download files gzipped by the server.
This comment was marked as off-topic.
This comment was marked as off-topic.
Fixes Homebrew/homebrew-cask#140264
This PR allows languages to be used when installing casks from the API. When a cask has multiple language options, just use the source file to load the cask. This is exactly what we do for casks with
*_flightblocks, and I think it makes sense to do here as well. There are currently 41 casks withlanguageblocks.In theory, we could check whether the current language is different from the default and only use this method when they differ, but I found that detection to be more complicated than I felt was justified, but I can look into that more if we think that will be valuable.
Note: this requires #14453 to be merged and put through a
formulae.brew.shrun before this can be merged (hence thedo not mergelabel).