Scheme data from inside the .yml should be canonical

[joshgoebel]

Ref: https://github.com/base16-project/base16-builder-node/issues/9

Lets take synth-midnight-dark.yaml:

scheme: "Synth Midnight Terminal Dark"

This means the generated slug will become: synth-midnight-terminal-dark. This has always been the behavior of the node builder.

It seems the spec (0.9.1) disagrees:

Where the slug is taken from the scheme filename made lowercase with spaces replaced with dashes and extension is taken from /template/config.yaml.

It seems the spec has always said this, so that's not optimal (for me), but still I feel this behavior is wrong (or at least concerning)... I'll try to lay out why. I'd like us to consider perhaps changing this behavior and rule that out before I take the time to patch node builder.

The issues I see:

• It causes confusion about the scheme name.
  • Is the name in scheme or is it the filename?
  • If the actual name is in scheme why should the filename not reflect the canonical name?
  • Are light and dark important parts of the name or not?
  • What does it mean when these conflict? Is this not problematic?
• It stores critical data about how to build the scheme outside the scheme... the filename is not part of the scheme's data.
  • Renaming a scheme's file should not change it's canonical name.
  • I should be able to store the scheme's data in a database and still generate correct themes (without any file system).
  • The fact that I can't do this means key data is missing from the scheme.yaml files.

I can understand why this decision was made (to use the filename) for convenience, but I think we should perhaps revisit the issue in light of the downsides.

Thoughts?

[joshgoebel]

It seems some themes filenames are "short names"... their full names are different or more nuanced. Just blindly using filename seems to result in a type of "data loss" when it comes to naming)

• icy => icy_dark (correct slug should be icy_dark - the scheme's name)
• irblack => "IR Black" (with a space) (correct slug should be ir-black)
• solarflare => "Solar Flare" (with a space) (correct slug should be solar-flare)

One could make the argument to just "fix all these mismatches", but that doesn't solve the problem long-term... or prevent filenames and name names from becoming out of sync again in the future - because the core problem here is "two sources of truth". There should be a single source of truth concerning the name.

I propose:

• filename is irrelevant, unimportant, and NOT part of the scheme meta-data
• the canonical name is stored in scheme and slugs are generated from that

It's impossible for this system to every get "out of sync" since there is nothing to keep in sync - if we make it clear that the data is only inside the scheme rather than also outside it.

(you could still say the filenames could get out of sync but i'd suggest it doesn't matter since it would no longer be part of the scheme data and hence have no effect on the build)

[joshgoebel]

Ok, I take back what I said about the spec always disagreeing... it seems it was ambiguous until 0.8.1 when it was made clear that the slug comes from the filename not the file data. Not that what it's said historically is hugely important here, just interesting point of fact.

[joshgoebel]

We've also talked some about build tooling, smaller pieces, interop, etc... and there was even talk in the spec about "minimum viable builder"... something like this should be theoretically possible:

cat icy.yaml | base16-build --with-template highlightjs

IE, taking all the scheme data and piping it directly into a builder... there is no filename here... the builder is reading the YAML from STDIN... generating the output should still be possible since the name is encoded in the YAML data itself and can be slugified from there.

[joshgoebel]

https://github.com/base16-project/base16-builder-node/issues/9#issuecomment-1178620546

I posted this issue before seeing this... so it looks like some thought went into the decision... I appreciate your taking time to find the back history. Never used Github via webarchive before, every wierd.

I think I still raise a few points that are worth addressing - esp if you feel the spec itself needs no major changes on this point:

• conflicting different/names of schemes
• multiple sources of truth
• inability to use scheme files with simple unix compose patterns (piping, etc)...

Do you at least agree that these are negatives of the current approach?

At the very least the divergent naming should be cleaned up - is it safe to assume the filenames are always wrong and that the canonical data (that should be used to correct broken filenames) is in the scheme field?

[belak]

It causes confusion about the scheme name. Is the name in scheme or is it the filename? If the actual name is in scheme why should the filename not reflect the canonical name? Are light and dark important parts of the name or not? What does it mean when these conflict? Is this not problematic?

The scheme-name is human readable and could be completely different from the slug (but generally isn't). One example I can think of would be a scheme which has light and dark variants, but wants the names to be the default (so you could have "Default Light" and "Default Dark" but the scheme slugs could be default and default-dark). I'm not saying that's a best practice, but we allow it currently. I don't think it's a major issue when the slug and name conflict, especially because we can fix that in our schemes repo, but it may be an issue with other sources.

Note that this is one reason I prefer using our scheme repo over external sources - we can ensure integrity of the data and not have to deal with a whole bunch of fallbacks.

It stores critical data about how to build the scheme outside the scheme... the filename is not part of the scheme's data. Renaming a scheme's file should not change it's canonical name.

It doesn't change the name, it changes the slug, which is used to generate other filenames and methods of programatic access. Slug should be separate from name.

I should be able to store the scheme's data in a database and still generate correct themes (without any file system). The fact that I can't do this means key data is missing from the scheme.yaml files.

I understand where you're coming from - I don't disagree, however that wasn't the focus of the existing builder spec.

• conflicting different/names of schemes

This was part of the reason I wanted to move to a single scheme repository - having a single folder and using the filename means that it's impossible to have multiple schemes which generate the same output file name. It's still definitely possible to have mismatched scheme-names and scheme-slugs, though I think https://github.com/base16-project/base16-schemes/pull/4 attempted to fix this with all the obvious issues in our schemes repo.

• multiple sources of truth

This was by design, to simplify the builder, but I'm still open to a scheme-slug field. Though that ends up being backwards incompatible with schemes, and if a builder wanted to remain backwards compatible with older-style-schemes, it would have to support filenames as a slug fallback (or we would need to ensure we have a standardized way of slugifying the scheme titles).

• inability to use scheme files with simple unix compose patterns (piping, etc)...

Yes, I agree this is a tradeoff of the existing design - the current builder spec was not written with this in mind. It may be worth revisiting this, but I'm not sure yet since we're still deciding what direction we want builders to go in.

[belak]

Here is a copy of what I posted in the base16-builder-node issue:

This was a change made from a previous spec (0.8.1) because we didn't want to deal with defining unicode folding or making a specific slugify algorithm a part of the spec. Using the filename was not a mistake.

The old repo is gone, but I dug up the old issues:

• https://web.archive.org/web/20210124190414/https://github.com/chriskempson/base16/issues/60
• https://web.archive.org/web/20210124190407/https://github.com/chriskempson/base16/issues/78
• https://web.archive.org/web/20210124190045/https://github.com/chriskempson/base16/pull/107

[belak]

Here are the possibilities I see:

1. Generate scheme-slug from the scheme-name
   • This has a number of trade-offs. Because the filename is no longer an input, it's possible for multiple schemes to conflict with each other's output filename.
   • We would also require some form of "unicode folding" to be defined (the Rosé theme is ironically a thorn in our side with this)
   • Some form of "slugification" algorithm would need to be defined
2. Add scheme as a property to the existing schemes.
   • This is not backwards compatible with existing builders. We would need to accept this, also name all our files with the same filename as the slug, or be explicit about falling back to the filename, which ends up with the same issues as the current setup.
   • This would still need us to define behavior when conflicting slugs come up.
3. Leave it as-is
   • This doesn't fix the problem, but stays backwards compatible with previous builders
   • This doesn't require conflict behavior to be added to the spec

[joshgoebel]

it's possible for multiple schemes to conflict with each other's output filename.

This is easily solved. Node builder had to deal with this before and it's handled in just 2 lines of code.

    if (fs.existsSync(targetPath)) {
      console.warn(`Overwriting ${targetPath}\nYou have two schemes using the same slug name.`);

We would also require some form of "unicode folding"

I'm sure this is perhaps a hard problem but I just imagine certainly their are libraries that have solved this for us? I'm not sure we need 100 pages in the spec to describe a specific algorithm vs a short summary and link to the library chosen. (yes this is complicated by multiple builders)

Add scheme as a property to the existing schemes

Wait, they all already have scheme, am I missing something? I'm assuming you mean scheme-slug... I like having slug as part of the metadata more than I like using the filename (outside the data). It's still sort duplicate data, but at least it's all in one place.

This is not backwards compatible with existing builders.

Do we care? I assumed as soon as Base17 is agreed upon and we start adding new features to schemes that this ship has sailed - and we'd possibly rename the repo base17_schemes to indicate that... obviously existing builders could support base17, but they would be under no obligation to do so.

This would still need us to define behavior when conflicting slugs come up.

Throw an error or warning. Actually right now node builder throws a warning and overwrites, so for CI usage an error would likely be better.

---

but stays backwards compatible with previous builders

Unless I'm missing something this is soon going out the door anyways, so it doesn't matter.

This doesn't require conflict behavior to be added to the spec

Unless I'm missing something it's trivial to add this. Something like:

In case of more than one scheme having duplicate slugs an error should be raised and the builder should stop.

[belak]

Wait, they all already have scheme, am I missing something? I'm assuming you mean scheme-slug... I like having slug as part of the metadata more than I like using the filename (outside the data). It's still sort duplicate data, but at least it's all in one place.

Yes, I meant slug - good catch.

We would also require some form of "unicode folding"

... I just imagine certainly their are libraries that have solved this for us? I'm not sure we need 100 pages in the spec to describe a specific algorithm vs a short summary and link to the library chosen. (yes this is complicated by multiple builders)

Yes, there are libraries - it's just something additional builders would need to handle if we went that route.

Also, for clarification, I think the term is Unicode Normalization, not folding. Folding usually refers to changing the case of Unicode characters.

This is not backwards compatible with existing builders.

Do we care? ...

I'm not sure - we haven't really decided what we want to support as an org in both the short and long term.

[joshgoebel]

it's just something additional builders would need to handle if we went that route.

I thought we'd kind of decide more builders == bad. That one canonical would be great, but that we're currently left with two - both with their pros and cons. If that's true then worrying about additional builder's shouldn't concern us... they'd have to find their own comparable slug library - which is likely easy in many languages, and harder in others.

But it's not anything that should lock someone out if they were motivated to create a builder.

[belak]

I've made an initial pass at slug in the scheme files in #55, though I'm not super happy with it (mostly because as a fallback it requires us to specify how to sanitize the name and requiring builders to understand and normalize unicode, even if done using a library, isn't very simple).

Is there anything else other than slug we'd need to add?

[belak]

This has been resolved with the 0.11.0 spec - there is a definition of how to generate a slug from the name as well as a slug field which can be used to override the output slug.