homebrew-cask Get rid of `colon`

Per discussion bellow, the new method should be csv. Example:

`version`	`version.csv[0]`	`version.csv[1]`	`version.csv[2]`
`123,456,789`	`123`	`456`	`789`

As per discussion in https://github.com/Homebrew/homebrew-cask-versions/pull/10064. : is a special character, and (unless I’m misremembering) it has given us trouble in the past. It’s used in highly specific cases, and those can be handled by multiple ,.

This issue both tracks what needs to be done and serves as discussion. To do this we’d need to:

[x] Remove references to colon in the version documentation. - Homebrew/brew#12445
[ ] Remove support for before_colon and after_colon from the core.
[ ] Remove support for places where : is used to split versions (https://github.com/Homebrew/brew/pull/10376/files#r562759754).
[x] Edit the cask where : is used in. Which are:

In homebrew-cask

In homebrew-cask-versions

In homebrew-cask-drivers

Ping @Homebrew/cask

Dec 11 '20 01:12 vitorgalvao

Good points.

The only thing that bugs me is the version.after_comma.before_comma pattern that could emerge. I feel that’s quite a pain to write and read.

Would it be more contributor-friendly to have something like version.comma_parts[1]?

Dec 11 '20 06:12 claui

what exactly are the problems that the colon causes?

The only thing that bugs me is the version.after_comma.before_comma pattern that could emerge. I feel that’s quite a pain to write and read.

yes exactly. if we get rid of the colon i'd like to replace it with another character and not with multiple colons as that would less elegant and more work in many cases.

also, if there is a mass-replace of the colon please let me know the exact time this will be done beforehand - thanks!

Dec 11 '20 10:12 core-code

The only thing that bugs me is the version.after_comma.before_comma pattern that could emerge. I feel that’s quite a pain to write and read.

Agreed, though not that much worse than the current state of affairs (.before_colon.after_comma). That still takes some mental effort.

Would it be more contributor-friendly to have something like version.comma_parts[1]?

I’m fine with that. It seems like the clearest of all solutions and we might even get away with just parts. version.parts[0] and version.parts[1] might even be mentally simpler to parse than before_comma and after_comma (which need to be mapped to the concept of “before” and “after”, which annoyingly are in the inverse alphabetical of what we want).

what exactly are the problems that the colon causes?

Before opening the issue I was under the impression we had gotten rid of it already, but I just realised I’m thinking of using /. That’s the thing that caused problems, but because HFS uses : for paths, I must’ve been thinking of it.

also, if there is a mass-replace of the colon please let me know the exact time this will be done beforehand

Worry not, I didn’t forget and I’m acutely aware these changes need to be handled with care. Though there aren’t that many casks that would need changing.

Dec 11 '20 18:12 vitorgalvao

version.comma_parts[1]

if you do something new like that, could you give me a week of notice before changing any casks to use it?

Dec 11 '20 19:12 core-code

version.comma_parts[1]

if you do something new like that, could you give me a week of notice before changing any casks to use it?

Yes, absolutely.

Dec 11 '20 19:12 claui

Another reason why parts or comma_parts is superior to just getting rid of colon: after_comma.after_comma is weird. Even with one after_comma, it’s not obvious if that refers to the first or last.

Dec 13 '20 19:12 vitorgalvao

how about using either - or _ as a replacement for : and add

after_hyphen
before_hyphen

after_underscore
before_underscore

Dec 13 '20 19:12 core-code

also, do we have a specific case where the colon has ever produced a problem?

Dec 13 '20 19:12 core-code

how about using either - or _ as a replacement for

Those are commonly used in versions, so it blurs the line between what’s a divider or not.

also, do we have a specific case where the colon has ever produced a problem?

Right now, I’m not sure. Though I’m getting partial to parts as a nicer method anyway. That said, I wouldn’t lose sleep over closing this, as long as we’re consistent.

Dec 13 '20 19:12 vitorgalvao

I think we should just have comma as a separator. Maybe version.csv[1] instead of version.parts[1]?

Example:

version "1.2.3,4567:9abcdef0"

url "#{version.after_comma.before_colon}"
url "#{version.before_colon.before_colon}"

vs.

version "1.2.3,4567:9abcdef0"

url "#{version.csv[1]}"

also, do we have a specific case where the colon has ever produced a problem?

The (mostly theoretical) technical issue:

You cannot have a path to a cask in a Caskroom with a version containing : in a PATH variable. At least I think so … in Finder you cannot even create a folder containing :.

The readability issue:

I guess mostly the problem is that having all of these combinations of #after_ and #before_ is hard to follow.

Dec 14 '20 11:12 reitermarkus

At least I think so … in Finder you cannot even create a folder containing :

i think this i just a Finder issue for compatibility with classic macOS - colon works normally in the filesystem / CLI

i agree that the proposed system will improve readability (and writability) in some cases though

Dec 14 '20 12:12 core-code

So we have a record, pointing out a new place in the core that’s introducing :.

Jan 22 '21 16:01 vitorgalvao

@Homebrew/cask Can we get a quick vote on if we should do this (👍) or close (👎)?

And if so, should we use comma_parts, parts, or csv? I’m fine with csv: it’s short and already maps well (comma-separated values, comma-separated version).

Feb 13 '21 02:02 vitorgalvao

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

Mar 26 '21 00:03 BrewTestBot

I’m 👍 with csv.

Mar 26 '21 06:03 claui

@bevanjkay with the new audit for this this issue can be closed right?

Apr 27 '22 19:04 SMillerDev

The audit isn't 100% complete yet, I just haven't been able to get around to fixing it. At the moment it is only checking the url stanza, but it needs to check the entire cask_contents, or stanza by stanza. As far as I know there was only one cask using an alternative version pattern outside of the url.

Apr 27 '22 23:04 bevanjkay

I am not sure this is related, I get an error when trying to upgrade some outdated packages.

$ brew upgrade ==> Casks with 'auto_updates true' or 'version :latest' will not be upgraded; pass --greedy to upgrade them. ==> Upgrading 4 outdated packages: Error: Cask 'adoptopenjdk8' is unreadable: undefined method `before_colon' for "232:b09":Cask::DSL::Version Did you mean? before_comma

Dec 24 '22 04:12 sasconsul

Not unless you haven't updated for 3 years

Dec 24 '22 18:12 SMillerDev

homebrew-cask homebrew-cask copied to clipboard

Get rid of `colon`

homebrew-cask
homebrew-cask copied to clipboard