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

Fixes for base16 theme #1925

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

skipkayhil
Copy link

Background

Commits are split into refactor/changes/additions, so reviewing them individually should be much easier than the final diff.

Base16 ruby code highlighting reference: https://base16.vercel.app/previews/base16-default-dark

There are still a few discrepancies between the After and the reference highlighting:

  • In the base16 reference, require is highlighted like a keyword and attr_accessor, print, and puts are highlighted like methods, but the Rouge ruby lexer defines all of these as Name::Builtin. I think most of the Rouge's ruby Name::Builtin should just be removed so that they can just be methods, but that isn't related to the Base16 theme.
  • self is correctly highlighted like a keyword in the reference, but the lexer currently says its a Name::Class.
  • Rouge has Person::name as a Name, but I think it should be possible to update the lexer to correctly identity them as Name::Function

Example Screenshots

Before:

image

After:

image

@skipkayhil
Copy link
Author

hey @tancnle, anything I can do to help get this merged?

This helps make it very obvious which tokens will end up with the same
base16 color.
Previously
- base0B: Strings, Inherited Class, Markup Code, Diff Inserted

Now:
- base09 - Integers, Boolean, Constants, XML Attributes, Markup Link Url
           ^^^^^^^^
Variables:
- base08 - Variables, XML Tags, Markup Link Text, Markup Lists, Diff
  Deleted

Functions:
- base0D - Functions, Methods, Attribute IDs, Headings
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant