PageMatch#find_versions: Fix return conditions

[samford]

• 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?

---

Casks with a livecheck block using PageMatch but not using #regex are currently failing (see #11882) because I recently introduced code in PageMatch#find_versions that returns early when a regex isn't provided. I had tested this change on homebrew/core, where all the related strategy blocks use #regex and pass it into the strategy block (i.e., do |page, regex|), but I didn't test it on homebrew/cask.

This PR fixes the issue by updating the condition to require either a regex or strategy block. I'll create a follow-up PR to add a related test and maybe modify PageMatch#find_versions's parameters (making regex nilable again) but I wanted to push out this fix in the interim time.

[BrewTestBot]

Review period skipped due to critical label.

[samford]

Going to go ahead and merge this, as it’s a simple change that fixes a bug.

[gibfahn]

Thanks for the fix @samford! This also caused issues with some livechecks in custom taps for us too, which were using a workaround for auth:

livecheck do
  url 'https://example.com'
  strategy :page_match do
    # This is a workaround for not being able to provide the header to curl in the :page_match
    # fetch itself.
    content = curl_with_workarounds('--header',
                                    "Authorization: Bearer #{CustomDownloadStrategy.get_auth_token(@url)}",
                                    @url).stdout
    JSON.parse(content)['releases']
        .select { |release| release['latest'] == true }
        .map { |release| release['version'] }
  end
end

[gibfahn]

Also worth noting that there's a block in the docs here that doesn't use the regex option: https://docs.brew.sh/Brew-Livecheck#pagematch-strategy-block

livecheck do
  url :homepage
  strategy :page_match do |page|
    page.scan(/href=.*?example[._-]v?(\d{4}-\d{2}-\d{2})\.t/i)
        .map { |match| match&.first&.gsub(/\D/, "") }
  end
end

so if the regex argument to the block is preferred, might be worth updating that too.

[samford]

This also caused issues with some livechecks in custom taps for us too, which were using a workaround for auth

This particular setup will be better supported by forthcoming features in the near future (options support and the Json strategy) and should allow you to create a straightforward livecheck block (instead of having to work around these technical limitations).

I recently took a detour to do a bit of cleanup and refactoring of strategies (to support continuing development) and I'm hoping to get a PR for the options DSL up within the next few days (famous last words). After laying groundwork in preceding PRs, the changes are pretty simple but there'll likely be some discussion around the benefits/detriments of the approach I've chosen (vs. alternatives). [After it's merged, we also have to wait until the commit is in a tagged brew release before we use it in livecheck blocks.]

Regarding the Json strategy, I've had it done for months but I'm pushing other work through the pipeline first. I'll try to get it out after the options DSL (or maybe alongside it), as I know you could get some use out of it. Either way, I'll try to remember to ping you when it's available.

---

Also worth noting that there's a block in the docs here that doesn't use the regex option

There's some history here but the short version is that the documentation is incorrect (in my view) and there are some things that I need to get done before creating a PR to fix it.

The example strategy blocks in the original documentation (which I wrote) used #regex and passed the regex into the strategy block. homebrew/core has always exclusively used this approach and that remains true.

The current documentation's "inline" approach came about because a homebrew/cask maintainer didn't know that you could pass the regex into the strategy block. I explained that this was possible (and how to take advantage of it) but they ignored my feedback and continued to inline regexes in strategy blocks.

Later, they created a PR to modify the documentation to use inline regexes in strategy block examples and I explained at length why passing the regex into the block is the appropriate approach but we couldn't reach a consensus and the PR stalled. It was eventually merged when I was asleep (time zones...) and that's how this situation came about.

I've always planned to open a PR to revert the strategy block example changes but I wanted to improve the situation before doing so. I've spent the interim time building up further support for my position on a technical level and within the community. Recently, some cask maintainers have been more receptive when I've suggested this approach in homebrew/cask, which gives me some hope that those livecheck blocks can be brought up to standard (not just in relation to the strategy blocks).

A majority of related cask strategy blocks can be updated to use #regex right now (primarily the ones that use one regex in the strategy block) but others use more than one regex and can't be updated easily (or until technical limitations are overcome). I plan to create a PR in the near future to modify #regex to support more than one regex (e.g., using a hash argument), so there won't be any technical reason to inline regexes after that.

With the technical issues resolved, I plan to create some bulk PRs to update the ~400 existing cask strategy blocks. If those are merged without objection, creating a PR to update the documentation should go smoothly at that point, as it will simply reflect the reality of existing livecheck blocks. There's plenty of work to be done but hopefully we get there in the end, as there are a number of good reasons to pass the regex into a strategy block.

[gibfahn]

Exciting news about the options block and json parser, I was surprised that it wasn't already an option, and so many livecheck blocks in cask/core seemed to be regex matching on JSON. Happy to beta-test a PR branch if that would be helpful.

Thanks for the diligence in making this part of homebrew easier to use! Also very helpful to update the core/cask repos, as people (or at least me 😁 ) tend to grep them for examples when writing tap formulae.