Collect icon names from svgPath directories and JSON maps - #20
Merged
Merged
Conversation
Without a `path`, AbstractIcon::path() falls back to `svgPath` for name collection. IconCollection::buildMap() calls names() on construction, so the fallback runs on every helper init, not only with checkExistence. - FontAwesome4/5/6 (and so 7), Bootstrap and Material collectors fed an svgPath directory straight to file_get_contents(), raising "Read of N bytes failed with errno=21 Is a directory". They now scan the directory for .svg file names, like Lucide/Feather already did. - A JSON SVG map as svgPath was rejected with "is not a directory", which buildMap() swallowed, silently disabling auto-prefixing (and failing hard with checkExistence). path() now accepts an existing .json map; the map keys are the icon names. FA4/Material read it as JSON, FA6 skips style detection for plain string map entries. - The collector in-memory cache is a single static array shared by all subclasses and was keyed by path only, so two collectors reading the same path got each other's result. The key now includes the class. - IconCollection defaults `separator` to ':' so it works standalone without IconHelper.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (19)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Setting
svgPathto a directory of individual SVGs (for examplevendor/fortawesome/font-awesome/svgs/solid/withFontAwesome7Icon) and leavingpathunset triggered this notice on every helper init:Without
path,AbstractIcon::path()falls back tosvgPathfor name collection, andIconCollection::buildMap()callsnames()in its constructor. Several collectors did not handle that fallback.Fixes
svgPath: the FontAwesome 4/5/6/7, Bootstrap and Material collectors passed the directory tofile_get_contents(). They now collect the.svgfile names, like Lucide and Feather already did.svgPath:path()rejected it with "is not a directory".buildMap()swallowed that error, so auto-prefixing silently stopped working. WithcheckExistenceon, the helper failed at construction. This hit the Feather JSON map example in the docs. The map keys are now used as icon names for every set.IconCollection:render()failed with aTypeErrorunlessseparatorwas passed explicitly. OnlyIconHelperset a default.IconCollectionnow defaults it to:as well.I probed all nine icon classes through
IconCollectionwithcheckExistenceon, in both modes (SVG directory and JSON map). Each case now returns names and raises no notices.The docs page
docs/helpers/icon-configuration.mdnow describes thesvgPathfallback for name collection.