Exclude inaccessible sprites from generated spritesheets

[OlenDavis]

• Adds globOptions to the src option to allow developers to pass 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.

[OlenDavis]

Let me know if you want me to undo the whitespace changes @mixtur; I only meant to add the documentation, not cleanup whitespace.

[mixtur]

• Will globOptions also work with watch mode? ( gaze )
• About changing default behaviour. You are loosing data here as I understand, right? I am ok with change in behaviour, but as user I would prefer to see some warning when such collision happens. Or it will be even more confusing then with current behaviour.

[OlenDavis]

• gaze uses minimatch under the hood rather than glob, so options for glob wouldn’t be compatible with it.
• I agree. Do you think this is something that should only be warned if the developer has enabled the logCreatedFiles option? Or do you think this is something that should be warned by default and the warning should be able to be disabled by a new disableWarnings option? (‘Cause in my case, the skipping of redundant sprites is expected behavior that I was surprised to find wasn’t happening; so I at least wouldn’t want such a warning to be un-suppressable.)

[OlenDavis]

• What do you think of what I mentioned in the second bullet of the original PR description? (Adding an option that allows developers to explicitly enable the previously default behavior of not skipping redundant sprites.) Because currently, this PR wouldn’t allow developers who had been relying on that previously default behavior to use this plugin anymore without a fork of their own. I would call such an option includeRedundantSprites.

[OlenDavis]

(I’ll also be sure to add “optional” to the globOptions bullet in the readme for consistency.)

[mixtur]

gaze uses minimatch under the hood rather than glob, so options for glob wouldn’t be compatible with it.

Then we should probably switch to minimatch. If it is too troublesome I'll do this myself

What do you think of what I mentioned in the second bullet of the original PR description? (Adding an option that allows developers to explicitly enable the previously default behavior of not skipping redundant sprites.) Because currently, this PR wouldn’t allow developers who had been relying on that previously default behavior to use this plugin anymore without a fork of their own. I would call such an option includeRedundantSprites.

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’ll also be sure to add “optional” to the globOptions bullet in the readme for consistency.)

I agree.

[mixtur]

Then we should probably switch to minimatch. If it is too troublesome I'll do this myself

Or maybe not. I am afraid to break something in an unexpected way. glob is also using minimatch under the hood. What is your exact use case by the way? May be it'll be easier to add just that?

[OlenDavis]

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:

• https://github.com/shama/gaze/blob/master/lib/gaze.js#L180 (Note that minimatch isn’t actually referenced in the repo’s package.json or any of its referenced packages.)
• https://github.com/cowboy/node-globule#globulefind

[OlenDavis]

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.

[OlenDavis]

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.

[OlenDavis]

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!

[OlenDavis]

Is there anything else you'd like me to do to this PR in order to merge it @mixtur?

[mixtur]

Nope I am just slow. Sorry. I'll check everything and will likely merge at evening (6-7 hours from now)