uloco / theme-bluloco-light

A fancy and sophisticated light designer color scheme.
https://marketplace.visualstudio.com/items?itemName=uloco.theme-bluloco-light
GNU Lesser General Public License v3.0
95 stars 11 forks source link

fix: add missing enummember token #43

Closed akiirui closed 3 years ago

akiirui commented 3 years ago

For rust-analyzer, result using token variable.other.enummember. But the theme not include this token, then it fallback to variable #383a42.

Disable rust-analyzer: Screenshot from 2021-07-22 11-25-18

Enable rust-analyzer: Screenshot from 2021-07-22 11-25-45

uloco commented 3 years ago

Hi @akiirui, thanks for your Pull Request! The problem here is, that this change will change all the enum member / key colors to red, which does not make sense. For example this would change the keys in a TypeScript enum to the left (here blue)

enum MyEnum {
  first = "first-value",
  second = "second-value"
}
uloco commented 3 years ago

What is Ok here for a type really?

akiirui commented 3 years ago

What is Ok here for a type really?

Thanks for reply, The Ok is a enum member

Ref: https://doc.rust-lang.org/std/result/

akiirui commented 3 years ago

And the TypeScript enum is also matching scope variable.other.enummember, but vscode not found this scope. And it fallback to scope variable #383a42 too.

If you think red #d52753 is not suitable for enum member, you can change it to other color.

Screenshot from 2021-07-22 17-06-11

uloco commented 3 years ago

Hmm, now seeing the Rust code I think it makes sense. But don't you think the color for a constant (purple) would make more sense here?

uloco commented 3 years ago

Variables are black by design. But since it is not variable, I guess purple is the way to go. Seeing the code in Rust also makes more sense. I don't know Rust really and callable enums are a new thing for me. If I could distinguish rust here from other languages I'd like to do it green so it is clear that it's callable, but unfortunately the language tokens are not as sophisticated yet.

uloco commented 3 years ago

Ok, sorry for spam. I think you are right. Red is the right color here since it is also a type. :)

uloco commented 3 years ago

@akiirui Published in 3.4.1 ;)

akiirui commented 3 years ago

Variables are black by design. But since it is not variable, I guess purple is the way to go. Seeing the code in Rust also makes more sense. I don't know Rust really and callable enums are a new thing for me. If I could distinguish rust here from other languages I'd like to do it green so it is clear that it's callable, but unfortunately the language tokens are not as sophisticated yet.

If you want to use different colors for each language, we can:

{
  "semanticTokenColors": {
    "enumMember:rust": {
      "foreground": "#COLOR"
    }
  }
}
akiirui commented 3 years ago

And the color constant (purple) maybe is better than red for enumMember, you are right.

Here are vscode default themes color scope for enumMember Dark: Screenshot from 2021-07-22 18-28-11 Light: Screenshot from 2021-07-22 18-30-58

In vscode default color themes, variable.other.enummember and variable.other.constant are using same color.

uloco commented 3 years ago

Variables are black by design. But since it is not variable, I guess purple is the way to go. Seeing the code in Rust also makes more sense. I don't know Rust really and callable enums are a new thing for me. If I could distinguish rust here from other languages I'd like to do it green so it is clear that it's callable, but unfortunately the language tokens are not as sophisticated yet.

If you want to use different colors for each language, we can:

{
  "semanticTokenColors": {
    "enumMember:rust": {
      "foreground": "#COLOR"
    }
  }
}

Oh wow, I didn't know that this was possible. But I guess it is fine to have it the same color everywhere, now that you mentioned it is just an enum. I already released the red variant, but yea, let's make it constant than, when vscode also uses this. Makes kind of sense. Thanks again for pointing this out :)