tinted-theming / home

Style systems and smart build tooling for crafting high fidelity color schemes and easily using them in all your favorite apps.
MIT License
252 stars 12 forks source link

Start moving towards separate scheme systems #55

Closed belak closed 8 months ago

belak commented 2 years ago

This is an experimental spec change which may or may not be merged later. It changes a number of things:

This would conflict with #47, but it shouldn't be too hard to reconcile the ideas (at least conceptually).

Fixes #42, https://github.com/base16-project/base16-schemes/issues/3

joshgoebel commented 2 years ago

Moves colors in scheme files to be under a top-level colors key

I'm not comfortable with this one. I think:

If we want to change the structure of a Base16 scheme file, it shouldn't be done in builder, and it can't be done under the auspices of Base16. Chris has made it clear that Base16 is a closed standard - or else that only he is driving the ship. So we need to leave it alone. If we support Base16 - it's "as is", ie "v0.2"...

If this WAS solely an evolution of Base16 (to have a colors key) I'd be more open to it I think (as a "niceness")... but we're not stewarding the Base16 spec... this kind of change should wait until one of our own specs, like Base17 or Ansi16.

joshgoebel commented 2 years ago

Requires colors to start with #

I like this, but it's another Base16 breaking change.

joshgoebel commented 2 years ago

is would conflict with #47, but it shouldn't be too hard to reconcile the ideas (at least conceptually).

Which ideas are you referring to? I'm not sure I see anything here that's really in conflict with the base17 proposal... oh other than perhaps:

Requires colors to start with #

My original plan was that Base17 be forwards compatible... ie, any existing theme can be moved into the Base17 space without requiring any changes (the perfect seamless upgrade)... but perhaps that need not be a hard requirement. It just seemed like a very nice thing to reassure people though because we benefit the more people embrace Base17.

I do agree with this idea overall (of requiring # again), but I wonder if may we could add it to the Base17 spec later (in a later revision) or perhaps make the base17 spec excluded from that rule? Perhaps in the short term it could say that # is strongly recommended and may be required in a future version.

belak commented 2 years ago

Which ideas are you referring to? I'm not sure I see anything here that's really in conflict with the base17 proposal...

I was talking about the PRs being conflicting, not the ideas.

I do agree with this idea overall (of requiring # again), but I wonder if may we could add it to the Base17 spec later (in a later revision) or perhaps make the base17 spec excluded from that rule? Perhaps in the short term it could say that # is strongly recommended and may be required in a future version.

We still need to decide if we're going with separate specs, or sticking to a single "palate style scheme family" spec for now, but that's separate from this.

joshgoebel commented 2 years ago

separate specs, or sticking to a single "palate style scheme family" spec ... but that's separate from this.

Is it? As long as we still have base16 a single larger "uni-spec" (that includes base16) can't require # because that's incompatible with base16 (which we can't change).

That's why I don't think this requirement (leading #) belongs in the builder spec at all - because right now it's impossible to add. It's sort of ok if builder changes details of inputs/outputs, where things are on the file system... if we separate the specs because then you can still say:

By keeping them separate we avoid all this. If we just accept base16 and base17 style systems are discrete units than one can easily require # while the other does not.

belak commented 2 years ago

Please explain "for a smaller PR"... removing this makes for a larger, not smaller PR.

It's one less file, and less changed lines. Removing it makes the PR smaller in both of those metrics.

What is the harm of an example of the format? I recall prior agreement that this would be useful.

I think the change is useful, but I don't think it belongs in this PR - I'm focusing on the builder spec rather than styling and it's already hard enough to get agreement on that.

EDIT: it looks like that change already made it in anyway in 29ddedf2fba0a5147db412e9f2977888c2e37a5d. My changes remove it from the PR, but not from main.

belak commented 8 months ago

Closing in favor of #80