Update builder-spec to 0.12.0 #99
Conversation
JamyGolden
force-pushed
the
main
branch
3 times, most recently
from
cedd2d4 to
4b709fa
Compare
builder.md
Outdated
| diff-inserted: base0B | ||
| support: base0C | ||
| regular-expressions: base0C | ||
| regular-expressions: base0C |
There was a problem hiding this comment.
Like the use of the overrides for more semantic naming, just a note the regular-expressions are duplicated here though
| @@ -67,6 +67,54 @@ These files have the following structure: | |||
| base0D: "dddddd" | |||
| base0E: "eeeeee" | |||
| base0F: "ffffff" | |||
| overrides: | |||
There was a problem hiding this comment.
Another thought would be sorting this alphabetically so it's a little easier to check options (eg. there's a default-foreground and a default-background)
There was a problem hiding this comment.
Hmm, I'd be ok with this but currently this information is sorted by base0* variable. I've added a comment above each "section" and sorted those sections alphabetically, what do you think of this?
There was a problem hiding this comment.
Yeah that makes a lot of sense. Happy with that
|
Happy with the direction this is taking, I think this pr needs a few minor tweaks before it is ready to merge :) |
JamyGolden
force-pushed
the
feature/builder-spec-0.12
branch
from
a7827a3 to
55abb45
Compare
JamyGolden
force-pushed
the
feature/builder-spec-0.12
branch
from
f50e2e6 to
949f356
Compare
JamyGolden
force-pushed
the
feature/builder-spec-0.12
branch
from
949f356 to
8b2e3e5
Compare
|
@belak what are your thoughts? Aslo, which versioning system is used for the styling and builder spec? I was under the impression that Semver major version 0 generally follows the rule of the minor number encompassing major and minor changes and the patch number still used for patches. |
Yeah, semver v0.x.y isn't super well defined. The spec technically says "Major version zero (0.y.z) is for initial development. Anything MAY change at any time. The public API SHOULD NOT be considered stable." However, most software seems to consider 0.1.0 and 0.1.1 to be compatible. I've always aimed for "if we bump the patch version, everything still using a builder which supports the older patch version should continue to work. Bumping the minor version is considered a breaking change." Essentially, we use patch version for non-breaking features as well as bug-fixes. Maybe at some point after 0.12 is stable and ready-to-go, we should look into releasing a version 1.0. |
|
Ok makes sense, so then I'll change this version to |
| @@ -67,6 +67,85 @@ These files have the following structure: | |||
| base0D: "dddddd" | |||
| base0E: "eeeeee" | |||
| base0F: "ffffff" | |||
| overrides: | |||
There was a problem hiding this comment.
Out of curiosity, how did we arrive at the current names? This is a bit of a larger question, but what would we think of organizing this by category?
Stuff like code.comments, code.boolean, etc.
I realize there's the potential for a lot of bikeshedding here, so as long as we understand where the current names come from, and have good ideas on how to name new names I don't think we need to worry about it too much.
There was a problem hiding this comment.
I went through the styling spec and used the names used in the "Text Editor" column.
If we decide on creating things like code.comments, etc then I'll probably use TmTheme files as heavy inspiration.
There was a problem hiding this comment.
How well does yaml cope with code.comments as a key? Or are we suggesting something like
...
code:
comments: "ffffff"If the latter then my preference would be to keep a flatter yml structure to simplify builders
There was a problem hiding this comment.
Yeah suggesting the latter, mustache doesn't allow property names with dots in them though, do this would be a problem. The flat structure makes sense when we don't have many variables, but when we have many we start prepending the variable names with what could be used as structure instead. As long as we use something like - dashes to signify a property of some other object, then to me it's the same. If we signify properties with dashes and then also allow dashes in the property name then it becomes confusing. For example scheme-supported-systems isn't clear which are properties. Could be scheme-supported with property system at a glance for example. So I'd opt for scheme-supportedSystem which maintains the child/parent relationship.
There was a problem hiding this comment.
Yeah I think that's reasonable tbh, I'm quite a fan of camel case. There is absolutely a case for more deeply nested structures but I think it would enable a lot more scope creep in terms of the number of variables used. Themes like textmates and vscodes appear to just grow exponentially in terms of complexity. I think a lot of the appeal of tinted theming is the simplicity in that regard
There was a problem hiding this comment.
Having said all that, I've created a draft version of property names that aren't derived from the styling.md document. @belak as you suggested it can get cumbersome pretty quickly, but I agree that we need to make sure we cover our bases with the new spec and can expand on it in future easily.
Here is another proposed layout. Of course users would likely only define ALL of properties here but the options give them the ability to more powerfully customise their scheme:
comment: base0x
invisibles: base0x
string:
quoted: base0x
regexp: base0x
constant:
language:
boolean: base0x
null: base0x
undefined: base0x
nan: base0x
infinity: base0x
self: base0x
super: base0x
debug: base0x
numeric: base0x
character:
entity: base0x
escape: base0x
format: base0x
storage:
type:
class: base0x
struct: base0x
enum: base0x
interface: base0x
entity:
name:
class: base0x
function: base0x
label: base0x
namespace: base0x
tag: base0x
typedef: base0x
variable: base0x
other:
attributeName: base0x
keyword:
control: base0x
declaration: base0x
operator: base0x
other: base0x
variable:
alias: base0x
function: base0x
language: base0x
parameter: base0x
other:
constant: base0x
member: base0x
property: base0x
readwrite: base0x
markup: base0x
bold: base0x
code: base0x
italic: base0x
quote: base0x
strike: base0x
diff:
added: base0x
changed: base0x
deleted: base0x
header: base0x
editor:
background: base0x
backgroundDark: base0x
backgroundLight: base0x
deprecated: base0x
foreground: base0x
foregroundDark: base0x
foregroundLight: base0x
invalid: base0x
lineBackground: base0x
searchText: base0x
selectionBackground: base0x
meta:
annotation: base0xThere was a problem hiding this comment.
I typed the response without seeing yours first @FredHappyface.
At base16/base24's core they are simple, most people won't use these override options, but I think it's good to provided the options to allow template maintainers the option to fix bugs with a scheme or all schemes. So while a typical scheme wouldn't use the overrides, I can see specific themes doing something like this:
system: "base16"
name: "Nord"
author: "arcticicestudio"
variant: "dark"
palette:
base00: "2E3440"
base01: "3B4252"
base02: "434C5E"
base03: "4C566A"
base04: "D8DEE9"
base05: "E5E9F0"
base06: "ECEFF4"
base07: "8FBCBB"
base08: "BF616A"
base09: "D08770"
base0A: "EBCB8B"
base0B: "A3BE8C"
base0C: "88C0D0"
base0D: "81A1C1"
base0E: "B48EAD"
base0F: "5E81AC"
overrides:
constant:
language:
boolean: "base0C"
entity:
name:
variable: "base0D"I feel like providing additional options doesn't make it complex. The difference between this and sublime/vscode/tmthemes is that you don't need to style any of those variables for base16/24 where in the mentioned editors it's a requirement so base16/24 simplicity is default with the override options.
There was a problem hiding this comment.
Yeah I agree with that. Thanks :)
There was a problem hiding this comment.
Based on the conversations in Matrix, I've simplified the above list by only keeping items specified in the styling.md document for now and I've also added the default base0x values.
comment: base03
string:
quoted: base0B
regexp: base0C
constant:
language:
boolean: base09
character:
entity: base08
entity:
name:
class: base0A
function: base0D
tag: base08
variable: base08
other:
attributeName: base09
keyword:
control: base0E
declaration: base0E
markup:
bold: base0A
code: base0B
italic: base0E
quote: base0C
diff:
added: base0B
changed: base0E
deleted: base08
ui:
background: base00
backgroundDark: base04
backgroundLight: base01
deprecated: base0F
foreground: base05
foregroundDark: base04
foregroundLight: base06
lineBackground: base03
searchText: base0A
selectionBackground: base02| @@ -159,6 +238,7 @@ A builder MUST provide the following variables to template files: | |||
| - `scheme-system` - obtained from the `system` key of the scheme input | |||
| - `scheme-variant` - obtained from the `variant` key of the scheme input | |||
| - `scheme-is-{{variant}}-variant` - dynamic value built from the `variant` key of the scheme file. e.g. `variant: "light"` provides `scheme-is-light-variant` with a value of `true`. | |||
| - `scheme-override-*` - obtained from the key within `overrides` object within the scheme yaml | |||
There was a problem hiding this comment.
Could we have an example here? Would diff-changes: base0E resolve scheme-override-diff-changes to base0E or something else?
There was a problem hiding this comment.
Yeah that's what I was thinking.
Co-authored-by: Kaleb Elwert <kaleb@coded.io>
Co-authored-by: Kaleb Elwert <kaleb@coded.io>
I think we need to clarify a few of the points on the whole bright colours prior to a 1.0 just that it means it may be harder to make such changes in the future and in a nice backwards compatible way |
General
@belak @FredHappyface and I had a discussion about the next Tinted Theming specification step. We discussed various possibilities but ultimately decided it was a good idea to expand the base16 builder spec to allow for more template color flexibility for base16 (and base24 since it's a superset of base16).
The general consensus was that we expand the spec, while being base16 backwards compatible, to include an "overrides" object, containing a set of pre-defined values where a scheme creator can make some adjustments to the scheme which would allow them to ultimately change how a theme is generally built using a tinted-builder. This change ultimately addresses #10, #11, #24 and #48. The pre-defined values will be heavily influenced by the existing styling specification.
This draft PR kicks off the continued discussion around this spec to move forward with.
PR specific
I've taken the values from the styling specification and added them to the overrides section basically word-for-word and added the current behaviour as default values.
In the current spec
default-background: base00,lighter-background: base01,selection-background: base02andbrightest-foreground: base07are single values with a singlebase0*mapping. So it might make sense to keep it out? Or maybe we should include it so when we potentially expand on an override in those sections, minimal change is required?There are things like
markup-italicswhich is oddly specific. If we decide it doesn't make sense to add, then we should probably update the styling spec based on our decisions here too.