Exclude inaccessible sprites from generated spritesheets #79
Conversation
…options to `node-glob` (and documents it), much in the same way the `spritesmithOptions` allows developers to pass options to that underlying library. * Changes the default behavior of the plugin to use whatever the _last sprite is of a given name_ as that sprite. This is after all the default behavior of all the spritesheet templates. So it makes sense that a spritesheet shouldn't contain any sprites that are inaccessible. That is unless the developer chooses to make their own spritesheet template that relies primarily on the sprites' source_file property (which most templates don't even use). So there's an argument to be made that while default, I should also add an option that allows those redundantly named sprites to not be skipped.
|
Let me know if you want me to undo the whitespace changes @mixtur; I only meant to add the documentation, not cleanup whitespace. |
|
|
|
|
(I’ll also be sure to add “optional” to the globOptions bullet in the readme for consistency.) |
Then we should probably switch to minimatch. If it is too troublesome I'll do this myself
Nope. It was clearly a bug. I agree that it is a breaking change, so we'll raise major version there. I don't want to complicate config even more. It is already full of awkward hacks. If people will start complaining anyway I'll add this option later.
I agree. |
Or maybe not. I am afraid to break something in an unexpected way. glob is also using minimatch under the hood. |
|
About using minimatch rather than glob, I agree. But upon further investigation (‘cause I was curious what pitfalls switching from glob to minimatch might entail), despite what gaze states in their README about using minimatch, it actually uses globule which uses glob. Furthermore, even though they don’t document the fact, it does pass its options through to globule, which in turn passes them through to glob. So in fact, we can pass globOptions to gaze as well for consistency, as gaze and glob are already complimentary. (Though for the sake of consistency, I will try to reuse the gaze object created for the watching for the getting of the image sources as well with its watched method.) I’ll tackle this first thing tomorrow morning (it’s nearly 3am) and let you know. (We’re already relying on all this functionality in my fork in very extensive production scenario.) References:
|
|
If glob uses minimatch, then mystery solved 😂. And since I found that gaze does pass its options to globule and in turn to glob, then I should be able to add globOptions to a sole call to gaze, and then reuse that instance calling its watched() method to get the source images. I’ll ping you tomorrow morning when it’s done. |
|
For a little background about our use case, we’re generating separate spritesheets that essentially inherit from a default set of sprites. We’re using webpack-chain to build independent webpack configs that do distinct builds from the same codebase where each build’s resolve modules array is largely the same, but has different specific paths. So for the spritesmith portion of each build, we use a globified version of that resolve modules array to generate a spritesheet that inherits from a shared set of sprites. It worked perfectly without modification except for needing to disable glob’s default behavior of sorting the set of matched paths with its nosort option, so that later matched, more specific sprites would trump earlier matched, less specific sprites. Rather than having all sprite name conflicts resolved by whatever the alphabetic ordering of their paths happened to be. |
…ance of `gaze` across the plugin. * Renames the `globOptions` to just `options` and fleshes out its documentation. * Removes the dependency on `glob` but doesn't commit the package-lock 'cause it's already woefully out of date. (even its package version is still 0.5.3 rather than 0.5.4)
|
Alright, sorry for the delay @mixtur; let me know if there's anything else I can address to get this PR to a mergeable state! |
|
Is there anything else you'd like me to do to this PR in order to merge it @mixtur? |
|
Nope I am just slow. Sorry. I'll check everything and will likely merge at evening (6-7 hours from now) |
Adds
globOptionsto thesrcoption to allow developers to pass options tonode-glob(and documents it), much in the same way thespritesmithOptionsallows developers to pass options to that underlying library.Changes the default behavior of the plugin to use whatever the last sprite is of a given name as that sprite. This is after all the default behavior of all the spritesheet templates. So it makes sense that a spritesheet shouldn't contain any sprites that are inaccessible. That is unless the developer chooses to make their own spritesheet template that relies primarily on the sprites' source_file property (which most templates don't even use). So there's an argument to be made that while default, I should also add an option that allows those redundantly named sprites to not be skipped.