Skip to content

dev-cmd/bump-cask-pr: use FromContentLoader when relevant. #17043

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

Merged
merged 1 commit into from
Apr 8, 2024

Conversation

MikeMcQuaid
Copy link
Member

When loading from tmp_contents in bump-cask-pr we're always loading from the contents and not from a e.g. filename etc. As a result, skip the detection of the correct loader (as the regex can be a bit flaky) and instead use FromContentLoader directly.

When loading from `tmp_contents` in `bump-cask-pr` we're always loading
from the contents and not from a e.g. filename etc. As a result, skip
the detection of the correct loader (as the regex can be a bit flaky)
and instead use `FromContentLoader` directly.
@MikeMcQuaid MikeMcQuaid added the bug label Apr 8, 2024
@Bo98
Copy link
Member

Bo98 commented Apr 8, 2024

as the regex can be a bit flaky

Do you know which regex is the flaky one?

@MikeMcQuaid
Copy link
Member Author

@Bo98 This:

# Cache compiled regex
@regex ||= begin
token = /(?:"[^"]*"|'[^']*')/
curly = /\(\s*#{token.source}\s*\)\s*\{.*\}/
do_end = /\s+#{token.source}\s+do(?:\s*;\s*|\s+).*end/
/\A\s*cask(?:#{curly.source}|#{do_end.source})\s*\Z/m
end
return unless content.match?(@regex)

Thanks for review!

@MikeMcQuaid MikeMcQuaid merged commit 0e9b2e3 into master Apr 8, 2024
@MikeMcQuaid MikeMcQuaid deleted the bump_cask_pr_content_loader branch April 8, 2024 13:19
@github-actions github-actions bot added the outdated PR was locked due to age label May 9, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants