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

colors are off, for typescript #312

Open
snr2718 opened this issue Jan 5, 2024 · 3 comments
Open

colors are off, for typescript #312

snr2718 opened this issue Jan 5, 2024 · 3 comments

Comments

@snr2718
Copy link

snr2718 commented Jan 5, 2024

Hi, I started using your plugin very recently. I am in love with it and really appreciate all the efforts. I wanted to submit the following issue:

The colors are off, for typescript.

Attaching screenshots:

  1. File opened in github.com ( google chrome; mac os; dark theme).
Screenshot 2024-01-05 at 2 55 34 PM
  1. File openend in neovim ( neovim version: 0.9.4; terminal: iterm2; theme: github_dark_default)
Screenshot 2024-01-05 at 2 55 20 PM

As you can see, the colors for class name and types of class properties are different from github.com.

@tmillr
Copy link
Member

tmillr commented Jan 15, 2024

This is true. I remember this difference when revamping the highlights, and this outcome was intentional. This is essentially how JavaScript is highlighted on github.com (i.e. with class names and the constructor keyword receiving highlights, as opposed to TypeScript where these go unhighlighted). The additional highlights are simply being borrowed/reused from js, although a little extra work could be done in order to differentiate the 2 langs and reflect precisely the quirks of github.com's interlanguage, highlighting differences (but personally I would consider that to be: a downgrade, less consistent, less optimal highlighting, etc., in this particular case).

TLDR: alot of langs needed to be revamped at once and it was simply less work to borrow/resuse this from the JavaScript highlighting, plus I thought it was better having these highlights included for TypeScript

Feel free to share your own thoughts on this decision if you disagree.

@niamleeson
Copy link

niamleeson commented May 4, 2024

Are those customizable? I've noticed that const is treated like a regular keyword.

This theme
Screenshot 2024-05-04 at 3 08 29 AM

VSCode (I changed the color for const and this to be more pleasant)
Screenshot 2024-05-04 at 3 08 58 AM

@tmillr
Copy link
Member

tmillr commented Jun 11, 2024

Are those customizable? I've noticed that const is treated like a regular keyword.

This theme Screenshot 2024-05-04 at 3 08 29 AM

VSCode (I changed the color for const and this to be more pleasant) Screenshot 2024-05-04 at 3 08 58 AM


Maybe this will have to be revisited and have its default hl changed, but the hl for const is correct as it is a keyword and GitHub highlights it the same as any other keyword:

Ts

const t = this.method();

Js

const t = this.method();

Also, the default ecma treesitter query currently captures all keywords under the same capture, so I don't think changing the hl for just const is currently supported. However it is technically still possible to change const via a custom treesitter query (if you are using treesitter). For example, you could try something like this if you'd like:

;; extends
["const"] @keyword.modifier

...and put that in a file at queries/ecma/highlights.scm under any dir in your runtimepath (e.g. ~/.config/nvim/queries/ecma/highlights.scm on unix). After that, you can change/configure the color of this hl group via config:

require('github-theme').setup({ groups = { all = { ['@keyword.modifier'] = { ... } } } })

Note

Replace ['@keyword.modifier'] with ['@keyword.modifier.typescript'] if you only want to change the color of modifier keywords for ts instead of all langs. This concept also applies below.

To clear or set the hl for this, you should be able to do:

['@variable.builtin.typescript'] = {},            -- empty table (clears hl), or
['@variable.builtin.typescript'] = { fg = 'fg' }, -- for when an empty tbl doesn't work (actually sets fg to global fg)

and add that to groups.all as above.

Or, you can override builtin variables at a higher level:

require('github-theme').setup {
  spec = { all = { syntax = { builtin0 = { ... } } } } }
}

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

No branches or pull requests

3 participants