Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: syntax tokens #57

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft

feat: syntax tokens #57

wants to merge 8 commits into from

Conversation

bennypowers
Copy link
Member

@bennypowers bennypowers commented Jan 23, 2023

based on #55

closes #16

@netlify
Copy link

netlify bot commented Jan 23, 2023

Deploy Preview for red-hat-design-tokens failed.

Name Link
🔨 Latest commit 30216c3
🔍 Latest deploy log https://app.netlify.com/sites/red-hat-design-tokens/deploys/643bed44b803a200085a0011

@bennypowers bennypowers force-pushed the tokens/syntax branch 2 times, most recently from 539049f to 80c300a Compare January 26, 2023 08:52
@bennypowers
Copy link
Member Author

bennypowers commented Jan 26, 2023

OK @coreyvickery this is ready for you to tinker with.

To get set up, check out this branch

git checkout tokens/syntax

Then clean out your working directory (save any uncommitted changes, they may be deleted)

git clean -dfx

Next install dependencies

npm ci

Then start the 11ty dev server

npm start

Once that's ready, you can change the values in tokens/color/syntax.yaml and they will be reflected in the Syntax Highlighting section of the demo page. NB: the current values are basically randomly chosen, so have at 'em.

You can uncomment any of the commented tokens and add values to see how they will reflect on the page. You can add markdown fenced code blocks to tokens/color/syntax.md to see how other languages will look.

@marionnegp marionnegp self-assigned this Feb 3, 2023
@coreyvickery
Copy link
Collaborator

@marionnegp Tagging you to take over.

@marionnegp
Copy link
Contributor

@bennypowers, I've been trying to match up the Prism JS tokens with the XD mockups, and I'm wondering how Prism is matching tokens to pieces of the code. For example, in the screenshot below myValue isn't mapped to a token, but it seems like it should be. The keyword class is also being used for multiple parts of the typescript example.
Screenshot 2023-02-08 at 4 23 37 PM

Is there a way to change this (maybe using Prism's Custom Class plugin)? Would that be a lot to manage in the future?

@bennypowers
Copy link
Member Author

@marionnegp if i understood correctly, you mean we should customize prism's parser to highlight tokens that it doesn't already, or to highlight them differently. If we went that route, we'd have to then also require all our users to implement a custom version of prism's parser whenever using our tokens, in order to get consistent results, and I think that's prohibitively complex for a user that just wants to theme a syntax snippet.

we could try highlighting with highlight.js instead, which is also quite popular.

@marionnegp
Copy link
Contributor

@bennypowers, yeah, that level of complexity was what I was afraid of. I think we should try highlight.js instead. @coreyvickery had used the highlight.js demos as a guide and tweaked the color, so it should be easier to update token colors.

@changeset-bot
Copy link

changeset-bot bot commented Feb 22, 2023

⚠️ No Changeset found

Latest commit: 30216c3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@marionnegp
Copy link
Contributor

@bennypowers, could you add highlight.js here instead of Prism.js? Does it cause any issues to use Prism in PFE and Highlight here?

@bennypowers
Copy link
Member Author

prism is the default in 11ty, but we can override that for the sake of the token pages, which I've done in the most recent commits.

This is getting at a deeper problem which is that we should prefer not to tie our token names to a given library. So let's say we got things worked out using highlight JS, and using their token names in our token names. What if we get a request for prism.js support? treesitter? vscode?

@bennypowers
Copy link
Member Author

bennypowers commented Mar 6, 2023

The current state of the PR encodes our color token names using highlightjs' syntax token names. The result is that the DP (as of 3c5a412) looks more-or-less like the XD, but this will be difficult to maintain and port to other syntax highlighting schemes like vscode, treesitter, or prism (which is the default highlighter for 11ty)

I think the way forward is:

  1. use treesitter token names to define our token names
  2. keep the logic of how to convert from treesitter token names to final output (e.g. prism.js theme, highlightjs theme, vscode theme json, etc) in the output generator files in lib/formats
  3. evaluate the results: if the designers find them to be too much of a compromise, establish a table of each exceptional case and how to express them in each format (prism, highlightjs, treesitter, vscode)

For reference, here are the highlight groups that nvim treesitter uses

@text.literal      Comment
@text.reference    Identifier
@text.title        Title
@text.uri          Underlined
@text.underline    Underlined
@text.todo         Todo
@comment           Comment
@punctuation       Delimiter
@constant          Constant
@constant.builtin  Special
@constant.macro    Define
@define            Define
@macro             Macro
@string            String
@string.escape     SpecialChar
@string.special    SpecialChar
@character         Character
@character.special SpecialChar
@number            Number
@boolean           Boolean
@float             Float
@function          Function
@function.builtin  Special
@function.macro    Macro
@parameter         Identifier
@method            Function
@field             Identifier
@property          Identifier
@constructor       Special
@conditional       Conditional
@repeat            Repeat
@label             Label
@operator          Operator
@keyword           Keyword
@exception         Exception
@variable          Identifier
@type              Type
@type.definition   Typedef
@storageclass      StorageClass
@structure         Structure
@namespace         Identifier
@include           Include
@preproc           PreProc
@debug             Debug
@tag               Tag

@bennypowers
Copy link
Member Author

leaving this here for future reference: https://dbanks.design/blog/vs-code-theme-with-style-dictionary/#Syntax-styles

@marionnegp
Copy link
Contributor

I used this syntax highlighting spreadsheet to match Tree-sitter tokens to what we have in this XD as best as I could. If I wasn't sure whether there was any code from the XD mockup that matched, I left a ? in the cell.

@bennypowers, I can try installing Tree-sitter and working to add these if that would be the next step.

@bennypowers
Copy link
Member Author

bennypowers commented Apr 16, 2023

@marionnegp I resolved the conflicts in this branch. b60bf62 looks nice! Build fails for good reasons: we need to realign syntax greys to the new grey tokens

@markcaron markcaron added the needs discovery Needs discovery label Sep 15, 2023
@coreyvickery
Copy link
Collaborator

Update on syntax highlighting colors posted here.

RedHat-UX/red-hat-design-system#1091

@markcaron markcaron modified the milestone: RHDS Tokens 2.0 Jan 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs discovery Needs discovery
Projects
Status: Dev WIP 🔵
Status: In Progress 🟢
Status: In Progress 🟢
Development

Successfully merging this pull request may close these issues.

New Tokens: Code Color Scheme
4 participants