buo / homebrew-cask-upgrade

A command line tool for upgrading every outdated app installed by Homebrew Cask
MIT License
2.39k stars 89 forks source link

Adding resiliency to `nil` version #223

Closed ondrejfuhrer closed 1 year ago

ondrejfuhrer commented 1 year ago

This adds a bit of resiliency to missing version.

Fixes #221

muescha commented 1 year ago

you are sure that if there is no version that the directory structure for installed casks are also in versions?

      versions = installed_versions(token)
      current_version = DSL::Version.new(versions.first)
muescha commented 1 year ago

https://github.com/buo/homebrew-cask-upgrade/blob/bed5779cfed3db5e65acea1144a6d68e5ff84596/lib/extend/cask.rb#L13

i would suggest to print here the token if the option is --verbose - then it is more easy to find the first failing cask token

ondrejfuhrer commented 1 year ago

you are sure that if there is no version that the directory structure for installed casks are also in versions?

      versions = installed_versions(token)
      current_version = DSL::Version.new(versions.first)

To be honest I only fixed what was reported in the issue and other similar places (chaining calls where the result of previous could be nil). Not sure what exactly are you suggesting here?

muescha commented 1 year ago

Not sure what exactly are you suggesting here?

If there is a cask where "version" = nil then brew can not construct a path like /{token}/{version}/

Then also the path construct maybe broken or the path not include a version number. But the implementation of installed_versions is based on this path schema. That's why I guess this line may also break 🤷‍♂️ or the DSL::Version when there is nil or something strange value

ondrejfuhrer commented 1 year ago

Not sure what exactly are you suggesting here?

If there is a cask where "version" = nil then brew can not construct a path like /{token}/{version}/

Then also the path construct maybe broken or the path not include a version number. But the implementation of installed_versions is based on this path schema. That's why I guess this line may also break 🤷‍♂️ or the DSL::Version when there is nil or something strange value

Ah, ok, makes sense. However in that case the glob function returns empty array no matter what you throw at it (except invalid characters for path I believe).

In this case, since it went through to next lines, it doesn't break in this case.

But thanks for calling it out 🙂