-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
theme: Make dark_plus.toml more accurate to VSCode #8719
Conversation
If I'm reading the changes correctly, it looks like some of the colors are actually inaccurate to the original theme (e.g. |
Hi @kirawi, As for VSCode's source I checked this two: dark+ and vsdark
I think the statusline's color can be discussed, some people could prefer that. |
Neither contain the full definition of the theme. What I had to do was open up the command palette and run As for the statusline, I am firmly against it. If anyone wants to change it, then they are free to do so locally. However, it wouldn't be Dark+ if the theme deviated from the original theme. |
runtime/themes/dark_plus.toml
Outdated
PopupBack = '#2D2D30' | ||
Selection = '#264F78' | ||
Cursor = '#A6A6A6' | ||
WhiteSpace = '#4D4D4D' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be Whitespace
set to 3e3e3d
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I prefer to keep the names in terms of colors. That was originally my intention with this theme, but I never found the time to finish that. Trust me, its less of a headache trying to find a semantic name for things.
Inlay hint colors were changed to the colors of Dark+ experimental in #7611, but they are now defined as "editorInlayHint.background": "#4d4d4d1a",
"editorInlayHint.foreground": "#969696", Might be good to update that. |
I'll get back to you once I've checked everything. |
Ok, when I get home I will check it myself also. For the change a think we stick to the original |
I've came back to the original structure and only made some modifications:
|
Sorry, I didn't mean to indicate that all of your changes were unwanted. I liked how you previously formatted it, but it's fine if you don't want to do that work again. I agree that VSCode Dark Modern should be a separate theme and you are free to open up a new PR with the original changes for that. You would have more creative liberty that route since I only maintain Dark+ :P |
No problem I did realize a lot of the colors I was used to were influenced by the vim colorschemes I previously used. So there was a lot of personal taste I decided to rollback to your version. After all you are the maintainer.
Do you mean separated the names with comments and aligned everything? |
If it is close enough to the Vim version, then we could probably offer a Vim variant of Dark+. We already offer a Zed variant of onelight.
Yeah, it looked nice. |
Did the formatting.
I was previously using this colorscheme. But I don't know if it different enough to make it a new variant. |
I think it would be acceptable, but I'll leave that to the discretion of the maintainers. |
Please note line 10 and 11, usize has now gone blue, is this a mistake? Please don't make any changes to the inlay hint colours or statusbar, thanks. This PRHow it wasmod hero;
mod world;
mod zombie;
use std::io;
use world::GameState;
#[derive(Debug)]
// a position on a grid to be displayed on the terminal
pub struct Point2d {
pub x: usize,
pub y: usize,
}
fn main() {
let screensize = Point2d { x: 32, y: 32 };
let mut game_state = GameState::new(&screensize);
game_state.add_hero('h');
game_state.add_zombies(64, 'z');
loop {
game_state.render_screen();
println!("Press hjkl to move the hero");
let mut key = String::new();
io::stdin()
.read_line(&mut key)
.expect("Failed to read line");
game_state.update(key.trim());
}
} |
In VSCode are green but it is inconsistent because it uses a regex highlight system. Since we are using TreeSitter, it does what we tell it to do, I've changed how it displays |
So, I think reverting that change in highlighting would be the more correct choice. For some reason (maybe historical), primitive types in C++ are not treated like any other type in both VSCode and CLion. In VSCode, they're actually under |
From what I remember I deliberately omitted Highlighting doesn't need to be identical across editors, and given we're using a different engine (tree-sitter vs regex-based textmate grammar) it's unlikely we can get them 1:1 anyway. |
For recap. Do I revert the built-in types to green? |
I've reverted the type.builtin to be blue again. |
Is it possible to add the highlights for debugging that were introduced in #5957?
I am not sure exactly what they are, I looked at https://github.com/microsoft/vscode/blob/main/extensions/theme-defaults/themes/dark_plus.json. |
Hey, sorry for the delay. I'll be looking at this within a few days. |
@kirawi would you consider adding these as a separate pull request before the next release? I am not sure what colors are best.
also, as soon as #9647 lands, which will be before the next release, we will need a but even better might be a colored underline something like: @saccarosium This PR needs to be updated to the latest master as there have been changes to the theme since. |
Sorry about leaving this unattended for so long. I've checked it over and it looks good, but there are some merge conflicts to address before it can be merged. I'd understand if you're no longer interested in continuing the PR given the time it took to review though. |
@David-Else I haven't used dark plus in a year or so. I'll be removing myself from maintainer of the theme after this PR. I think that your suggestions thus far have been consistent with VSCode, so if you're interested in becoming a new maintainer then I'd be fine with that. |
At this point, I think we can close this. I don't really use helix anymore. |
The current implementation of the Dark+ has some inconsistencies with the original. I've made some modification, witch the most importants are:
C
CPP