Skip to content

Conversation

@Rylan12
Copy link
Member

@Rylan12 Rylan12 commented Dec 29, 2025

Currently, there is a method in Formulary to convert strings to symbols if they begin with :. This functionality isn't specific to formulae, so extracting this out lets it be used in casks and other places where it might be useful.

I opted for patching String to define this method since it's not overriding an existing method and seems potentially useful for any string, but I'm sure there's another place this can go if preferred to avoid the monkey patch.

Copilot AI review requested due to automatic review settings December 29, 2025 03:39
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extracts string-to-symbol conversion functionality from the Formulary module into a reusable String extension method called to_string_or_symbol. This makes the functionality available for casks and other parts of the codebase where strings prefixed with : need to be converted to symbols.

Key changes:

  • Added String#to_string_or_symbol method that converts strings starting with : to symbols, otherwise returns the string unchanged
  • Migrated test coverage from formulary_spec.rb to new string_spec.rb
  • Updated all usages in api/formula.rb to use the new String extension method

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
Library/Homebrew/extend/string.rb Adds the new to_string_or_symbol method to the String class with proper documentation and type signatures
Library/Homebrew/test/extend/string_spec.rb New test file providing coverage for the to_string_or_symbol method
Library/Homebrew/formulary.rb Removes the now-extracted convert_to_string_or_symbol class method
Library/Homebrew/test/formulary_spec.rb Removes tests for the deleted convert_to_string_or_symbol method
Library/Homebrew/api/formula.rb Updates two call sites to use the new instance method instead of the removed class method

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to extract but to Utils please 🙇🏻

@Rylan12 Rylan12 force-pushed the to-string-or-symbol-extension branch from e9a5b81 to bb6951f Compare December 29, 2025 15:44
Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@MikeMcQuaid MikeMcQuaid added this pull request to the merge queue Dec 29, 2025
Merged via the queue into main with commit 9eac4e2 Dec 29, 2025
37 checks passed
@MikeMcQuaid MikeMcQuaid deleted the to-string-or-symbol-extension branch December 29, 2025 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants