alphapapa / solarized-everything-css

A collection of Solarized user-stylesheets for...everything?
GNU General Public License v3.0
278 stars 43 forks source link

Add ability to blacklist sites from all-sites #38

Closed jgkamat closed 6 years ago

jgkamat commented 6 years ago

This is a very simple exclusion mechanism for stylesheets. If a file starts with _, such as _bigblow.styl it will:

  1. Be excluded from all-sites.css.
  2. Have it's _ stripped when css files are generated, so file names stay the same.

I think this is a fairly simple implementation, only a couple lines needed to be changed. Let me know if you can think of a nicer way to do it though.

Closes #37

After this, I want to try to merge #35, it's been sitting for a while, and this will get the all-sites css sheet working again, which was my only gripe with it.

alphapapa commented 6 years ago

Hey Jay,

Sorry for the late response, been busy IRL. Couple of things:

  1. I see a lot of changes to CSS files, which shouldn't be relevant to this PR. Maybe you need to rebase it?

  2. It's neat that you were able to use a regexp in the Stylus file to change the matched files, but I'm not sure this will quite have the desired effect:

@require 'sites/[^_]*.styl'

If that's an actual regexp (I didn't know Stylus supports them), then I think that will exclude files that have an _ anywhere in the name, rather than just at the beginning of the filename.

Other than that, looks like a nice, simple way to fix it! Thanks.

jgkamat commented 6 years ago

Sorry for the late response, been busy IRL. Couple of things:

No worries, I've been much more absent from some things that I would have liked to stay on top of as well :P

I see a lot of changes to CSS files, which shouldn't be relevant to this PR. Maybe you need to rebase it?

This is actually the bigblow stylesheet definitions going away from the all-sites files. I could split that into a seperate commit if you want (I probably should do that for easier reviewing).

It's neat that you were able to use a regexp in the Stylus file to change the matched files, but I'm not sure this will quite have the desired effect:

I tested renaming github.styl -> github_test.styl, and I verified that the github definitions weren't removed from the all-sites file. This isn't a real regex, but it's globs, I think using fnmatch. To make it a bit more explicit, I'm switching the _ syntax to use the globbing style documented in the wikipedia page (which seems to work the same).

alphapapa commented 6 years ago

This is actually the bigblow stylesheet definitions going away from the all-sites files. I could split that into a seperate commit if you want (I probably should do that for easier reviewing).

Haha, oops, I should have noticed that. Yeah, maybe it would be good to split them up.

I tested renaming github.styl -> githubtest.styl, and I verified that the github definitions weren't removed from the all-sites file. This isn't a real regex, but it's globs, I think using fnmatch. To make it a bit more explicit, I'm switching the syntax to use the globbing style documented in the wikipedia page (which seems to work the same).

Cool, globs are good too. Nice that Stylus has them.

Thanks for fixing this! Go ahead and merge whenever you feel it's ready.