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

vscode onedarkpro vs this one #23

Closed
sinkyl opened this issue Feb 7, 2022 · 13 comments
Closed

vscode onedarkpro vs this one #23

sinkyl opened this issue Feb 7, 2022 · 13 comments

Comments

@sinkyl
Copy link

sinkyl commented Feb 7, 2022

Hi,

This is a suggestion.
I end up changing many highlights and colors to represent the onedarkpro for vscode that has over 4M downloads, a standard onedarkpro theme. If this repo is for everyone to enjoy the standard onedarkpro theme with neovim you should represent the exact onedarkpro.

For example, the default backgroud color should be bg = "#282c34", the next line should be purple and not italic TSInclude = { fg = theme.colors.blue, style = theme.options.italic }, -- For includes: "#include" in C, "use" or "extern crate" in Rust, or "require" in Lua. and so on.

People should start from the standard theme and then change settings/highlights/colors if needed. But, if this repo is for yourself just ignore what I just said.

One last suggestion, the plugins by default should be set to false, so we can keep the onedarkpro.settup clean as much as possible.

Thanks

@olimorris
Copy link
Owner

Thanks for the feedback.

Could you let me know the highlights and colors that you've changed and I'll look into incorporating into the theme.

Regarding the italics, I'll caveat by saying in the readme I specify "Use the themes opinionated italic styles?" but there is a case for setting this to false by default.

@sinkyl
Copy link
Author

sinkyl commented Feb 8, 2022

local colors = require("onedarkpro").get_colors("onedark")
...

colors = { 
   bg = "#282c34"
},


hlgroups = { 
   -- onedarkpro -> treesitter.lua plugin
   TSInclude = { fg = colors.purple, style = "NONE" },
   TSTypeBuiltin = { fg = colors.yellow },
   TSConstBuiltin = { fg = colors.purple },
   TSConstructor = { fg = colors.yellow },
   TSRepeat = { style = "NONE" },
   TSFuncMacro = { fg = colors.blue },
   TSParameterReference = { fg = colors.red },

   -- onedarkpro -> theme.lua
   Include = { fg = colors.purple },
   Function = { fg = colors.blue },
   Operator = { fg = colors.fg }, 
   Macro = { fg = colors.blue },
   Special = { fg = colors.fg },
   Structure = { fg = colors.purple },
   Identifier = { fg = colors.red },
   Typedef = { fg = colors.purple },
   Constant = { fg = colors.orange },
 }

Results:

  • Rust
    nvim
    nvr
    vscode
    vsr

  • C
    nvim
    nvc
    vscode
    vsc

  • C#
    nvim (The first in red is annoying. It should be yellow but TSVariables set it to red)
    nvcsharp
    vscode
    vscsharp

  • Go
    nvim (the functions in the interface are in red)
    nvgo
    vscode
    vsgo

  • Typescript is ok

  • Javascript is not

The onedarkpro for vscode is adding unique style for each language. IMO, this repo should do the same.
For now, I'm trying to add custom highlighters with treesitter/playground.
Nice work BTW.

olimorris added a commit that referenced this issue Feb 8, 2022
sinkyl made a number of solid suggestions in issue #23 (#23) regarding the theme's lack of compliance to the original onedarkpro.

This commit address _some_ of their concerns regarding highlight groups and theme defaults. As of now, all options are turned off by default. In retrospect, I consider them to be too opinionated to be the default for theme. Also, a number of changes to hlgroups in the `treesitter.lua` and `theme.lua` file have been made as per #23. Finally, the background color is now set at #282c34 for the dark theme.
@olimorris
Copy link
Owner

olimorris commented Feb 8, 2022

Firstly, thank you for your detailed feedback and config. It is greatly appreciated.

I agree with the majority of your changes and have addressed them in the latest commit. Some I have not included (detail and reasoning below).

The onedarkpro for vscode is adding unique style for each language. IMO, this repo should do the same.

Yes it does. I have actively thought about doing this and may look into it in the near future. Let me know if you make any progress. Always open to pull requests.

FYI, the hlgroups which I have kept the same are:

TSRepeat
Operator
Special

Reason being that they caused some difference in look between Neovim and VS Code in Ruby, Python and Javascript files which I don't have with their current values. Happy to be corrected and adjust them (could always be a setting I missed in my custom VS Code config).

But please let me know your thoughts on the changes. I've updated the readme screenshots too.

@mmirus
Copy link
Contributor

mmirus commented Feb 8, 2022

Thanks both! I was able to remove a few of my customizations after updating.

I did prefer TSConstBuiltin = { fg = theme.colors.orange }, over the purple, though. See the end of the second line for why:

image

vs

image

@mmirus
Copy link
Contributor

mmirus commented Feb 8, 2022

@olimorris
Copy link
Owner

@mmirus
Copy link
Contributor

mmirus commented Feb 8, 2022

Maacro and Faunction in theme.lua:

Maacro = { fg = theme.colors.blue }, -- same as Define

olimorris added a commit that referenced this issue Feb 8, 2022
Big props to mmirus for noticing in #23
@olimorris
Copy link
Owner

@mmirus, you sir, are a legend as always! Thanks for spotting

@mmirus
Copy link
Contributor

mmirus commented Feb 8, 2022

You got it!

@sinkyl
Copy link
Author

sinkyl commented Feb 9, 2022

Ok for the highlights but the plugins are somehow always set to true event I change it to false.

This is not working:

   all = false,
   packer = true,
   treesitter = true,

or this

   packer = false,
   treesitter = false,

@olimorris
Copy link
Owner

Should now be fixed. I've had to apply some trickery to how we merge the default config with the user's custom config. End result is your test cases above now work.

olimorris added a commit that referenced this issue Feb 9, 2022
@olimorris
Copy link
Owner

@sinkyl - Would you also be willing to try out the filetype_hlgroups branch?

I've got a WIP feature which allows you to add custom highlight groups for specific filetypes. Works great on my end (to overcome the TSField issue with yaml files) but keen to see your use cases and especially if you notice any performance issues.

olimorris added a commit that referenced this issue Feb 9, 2022
@sinkyl
Copy link
Author

sinkyl commented Feb 10, 2022

All good now.
I'll give a try later.
Thx 💯

olimorris added a commit that referenced this issue Feb 15, 2022
* feat ✨ toggle light/dark versions with bg #5

* chore: update README.md

* fix: speed up load time

* chore: update README.md

* feat: ✨ add alacritty and kitty themes #11

* refactor: rename transparent option to transparency

* refactor: rename highlight_cursorline option to cursorline

* feat: add filetype highlight group customization

* fix: 🔨 #23 plugins always set to true

* chore: speed up buffer navigation

This doesn't make a noticable difference on any of my buffers but is a logical inclusion

* chore: update README.md

* feat: on theme reload apply filetype hlgroups

* fix: urls to onedarkpro in README.md

* chore: fix black color in onedark

* fix: filetype highlights do not work with floating windows

* chore: add disclaimer about telescope highlighting
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