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.

Sign up for GitHub

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

Jump to bottom

Better syntax highlighting #1294

Closed
matklad opened this issue May 21, 2019 · 11 comments · Fixed by #2061
Closed

Better syntax highlighting #1294

matklad opened this issue May 21, 2019 · 11 comments · Fixed by #2061
Labels
E-easy E-hard E-medium fun A technically challenging issue with high impact

Comments

@matklad
Copy link
Member

matklad commented May 21, 2019

TreeSitter plugin shows that it is actually possible to implement decent highlighting with the current API.

Things we should steal:

  • visibleRanges support
  • VS.ThemeColor

Note that we need to make our internal highlightling infra "enterprise-ready" as well :-)
@matklad matklad added fun A technically challenging issue with high impact E-easy E-hard E-medium labels May 21, 2019
@matklad
Copy link
Member Author

matklad commented May 21, 2019

(Note: it is E-everything, because this is a large issue with isolated easy parts and harder parts)

@lnicola lnicola mentioned this issue May 21, 2019
Merged
@kjeremy
Copy link
Contributor

kjeremy commented May 21, 2019 •
edited
Loading

There was talk of a semantic highlighting LSP extension here: microsoft/vscode-languageserver-node#368

bors bot added a commit that referenced this issue May 21, 2019
@bors @lnicola
Merge #1299
51e82fe 
1299: Use ThemeColor and add support for light themes r=matklad a=lnicola

Part of #1294.

- switch to `ThemeColor`
- add light and high contrast theme definitions
- highlight control flow keywords and `unsafe`


Co-authored-by: Laurențiu Nicola <lnicola@dend.ro>
@killercup
Copy link
Member

killercup commented May 21, 2019 •
edited
Loading

Any interest in adding a mode with binding-specific coloring? If bindings and their use cases have unique IDs, picking colors them might be as simple as using the ID as seed in a function like this.

Edit: Started in #1319

@matklad
Copy link
Member Author

matklad commented May 21, 2019

Any interest in adding a mode with binding-specific coloring? I

That is, coloring each funcition parameter in unique color? We can do that, yeah.

@killercup
Copy link
Member

killercup commented May 21, 2019 via email

@univerz
Copy link

univerz commented May 22, 2019

picking colors them might be as simple as using the ID as seed in a function like this.

KDevelop have state of the art semantic highlighting and afaik was first IDE doing it (~2009). Through the years, they tried several approaches - I would like to suggest to look there for inspiration.

Semantic highliting looks funny at first, but I'm half-blind without it now.

@matklad
Copy link
Member Author

matklad commented May 23, 2019

and afaik was first IDE doing it

That's true! Though, IntelliJ has this feature as well. If only they named it less ambitious... Like, there's much more to semantic that coloring different references differently... The basic rule of semantic highlighting in a general sense is that the reference should be colored in the same color as the corresponding definition.

@univerz
Copy link

univerz commented May 23, 2019

...

True. I just wanted to point out that generating readable and good-looking colors, in sufficient quantity, is not a purely programming problem and there is more "science" behind it (i'm not an expert myself).

@lnicola lnicola mentioned this issue May 28, 2019
Merged
bors bot added a commit that referenced this issue May 29, 2019
@bors @lnicola
Merge #1338
b0d84cb 
1338: Highlight names correctly r=matklad a=lnicola

Part of #1294.

Co-authored-by: Laurențiu Nicola <lnicola@dend.ro>
@chriskrycho
Copy link

It'd also be appreciated if the colors were derived from the user's existing theme choice. I really don't want to have to supply custom colors for a specific editor plugin; I want my editor's theming to drive everything.

@lnicola
Copy link
Member

lnicola commented May 30, 2019

microsoft/vscode-languageserver-node#368 would help here, but I don't think there's much the extension can do.

The current colors are based on the default VS Code themes.

@seivan
Copy link
Contributor

seivan commented Oct 24, 2019

I took a stab at it here #2061

@seivan seivan mentioned this issue Oct 24, 2019
Merged
11 tasks
bors bot added a commit that referenced this issue Dec 4, 2019
@bors @seivan
Merge #2061
af33c22 
2061: Theme loading and "editor.tokenColorCustomizations" support. r=lnicola a=seivan

Fixes: [Issue#1294](#1294 (comment))

TODO: 
- [x] Load themes
- [x] Load existing `ralsp`-prefixed overrides from `"workbench.colorCustomizations"`.
- [x] Load overrides from `"editor.tokenColorCustomizations.textMateRules"`.
- [x] Use RA tags to load `vscode.DecorationRenderOptions` (colors) from theme & overrides.
- [x] Map RA tags to common TextMate scopes before loading colors.
- [x] Add default scope mappings in extension.
- [x] Cache mappings between settings updates. 
- [x] Add scope mapping configuration manifest in `package.json`
- [x] Load configurable scope mappings from settings.
- [x] Load JSON Scheme for text mate scope rules in settings.
- [x] Update [Readme](https://github.com/seivan/rust-analyzer/blob/feature/themes/docs/user/README.md#settings).

Borrowed the theme loading (`scopes.ts`) from `Tree Sitter` with some modifications to reading `"editor.tokenColorCustomizations"` for merging with loaded themes and had to remove the async portions to be able to load it from settings updates. 

~Just a PoC and an idea I toyed around with a lot of room for improvement.~
For starters, certain keywords aren't part of the standard TextMate grammar, so it still reads colors from the `ralsp` prefixed values in `"workbench.colorCustomizations"`. 

But I think there's more value making the extension work with existing themes by maping some of the decoration tags to existing key or keys. 

<img width="453" alt="Screenshot 2019-11-09 at 17 43 18" src="https://user-images.githubusercontent.com/55424/68531968-71b4e380-0318-11ea-924e-cdbb8d5eae06.png">
<img width="780" alt="Screenshot 2019-11-09 at 17 41 45" src="https://user-images.githubusercontent.com/55424/68531950-4b8f4380-0318-11ea-8f85-24a84efaf23b.png">
<img width="468" alt="Screenshot 2019-11-09 at 17 40 29" src="https://user-images.githubusercontent.com/55424/68531952-51852480-0318-11ea-800a-6ae9215f5368.png">


These will merge with the default ones coming with the extension, so you don't have to implement all of them and works well with overrides defined in settings. 

```jsonc
    "editor.tokenColorCustomizations": {
        "textMateRules": [
            {
                "scope": "keyword",
                "settings": {
                    "fontStyle": "bold",
                }
            },
        ]
    },
```


Edit: The idea is to work with 90% of the themes out there by working within existing scopes available that are generally styled. It's not to say I want to erase the custom Rust scopes - those should still remain and eventually worked into a custom grammar bundle for Rust specific themes that target those, I just want to make it work with generic themes offered on the market place for now. 

A custom grammar bundle and themes for Rust specific scopes is out of... scope for this PR. 
We'll make another round to tackle those issues. 


Current fallbacks implemented

```typescript
    [
        'comment',
        [
            'comment',
            'comment.block',
            'comment.line',
            'comment.block.documentation'
        ]
    ],
    ['string', ['string']],
    ['keyword', ['keyword']],
    ['keyword.control', ['keyword.control', 'keyword', 'keyword.other']],
    [
        'keyword.unsafe',
        ['storage.modifier', 'keyword.other', 'keyword.control', 'keyword']
    ],
    ['function', ['entity.name.function']],
    ['parameter', ['variable.parameter']],
    ['constant', ['constant', 'variable']],
    ['type', ['entity.name.type']],
    ['builtin', ['variable.language', 'support.type', 'support.type']],
    ['text', ['string', 'string.quoted', 'string.regexp']],
    ['attribute', ['keyword']],
    ['literal', ['string', 'string.quoted', 'string.regexp']],
    ['macro', ['support.other']],
    ['variable', ['variable']],
    ['variable.mut', ['variable', 'storage.modifier']],
    [
        'field',
        [
            'variable.object.property',
            'meta.field.declaration',
            'meta.definition.property',
            'variable.other'
        ]
    ],
    ['module', ['entity.name.section', 'entity.other']]
```


Co-authored-by: Seivan Heidari <seivan.heidari@icloud.com>
bors bot added a commit that referenced this issue Dec 29, 2019
@bors @seivan
Merge #2061
2327852 
2061: Theme loading and "editor.tokenColorCustomizations" support. r=matklad a=seivan

Fixes: [Issue#1294](#1294 (comment))

TODO: 
- [x] Load themes
- [x] Load existing `ralsp`-prefixed overrides from `"workbench.colorCustomizations"`.
- [x] Load overrides from `"editor.tokenColorCustomizations.textMateRules"`.
- [x] Use RA tags to load `vscode.DecorationRenderOptions` (colors) from theme & overrides.
- [x] Map RA tags to common TextMate scopes before loading colors.
- [x] Add default scope mappings in extension.
- [x] Cache mappings between settings updates. 
- [x] Add scope mapping configuration manifest in `package.json`
- [x] Load configurable scope mappings from settings.
- [x] Load JSON Scheme for text mate scope rules in settings.
- [x] Update [Readme](https://github.com/seivan/rust-analyzer/blob/feature/themes/docs/user/README.md#settings).

Borrowed the theme loading (`scopes.ts`) from `Tree Sitter` with some modifications to reading `"editor.tokenColorCustomizations"` for merging with loaded themes and had to remove the async portions to be able to load it from settings updates. 

~Just a PoC and an idea I toyed around with a lot of room for improvement.~
For starters, certain keywords aren't part of the standard TextMate grammar, so it still reads colors from the `ralsp` prefixed values in `"workbench.colorCustomizations"`. 

But I think there's more value making the extension work with existing themes by maping some of the decoration tags to existing key or keys. 

<img width="453" alt="Screenshot 2019-11-09 at 17 43 18" src="https://user-images.githubusercontent.com/55424/68531968-71b4e380-0318-11ea-924e-cdbb8d5eae06.png">
<img width="780" alt="Screenshot 2019-11-09 at 17 41 45" src="https://user-images.githubusercontent.com/55424/68531950-4b8f4380-0318-11ea-8f85-24a84efaf23b.png">
<img width="468" alt="Screenshot 2019-11-09 at 17 40 29" src="https://user-images.githubusercontent.com/55424/68531952-51852480-0318-11ea-800a-6ae9215f5368.png">


These will merge with the default ones coming with the extension, so you don't have to implement all of them and works well with overrides defined in settings. 

```jsonc
    "editor.tokenColorCustomizations": {
        "textMateRules": [
            {
                "scope": "keyword",
                "settings": {
                    "fontStyle": "bold",
                }
            },
        ]
    },
```


Edit: The idea is to work with 90% of the themes out there by working within existing scopes available that are generally styled. It's not to say I want to erase the custom Rust scopes - those should still remain and eventually worked into a custom grammar bundle for Rust specific themes that target those, I just want to make it work with generic themes offered on the market place for now. 

A custom grammar bundle and themes for Rust specific scopes is out of... scope for this PR. 
We'll make another round to tackle those issues. 


Current fallbacks implemented

```typescript
    [
        'comment',
        [
            'comment',
            'comment.block',
            'comment.line',
            'comment.block.documentation'
        ]
    ],
    ['string', ['string']],
    ['keyword', ['keyword']],
    ['keyword.control', ['keyword.control', 'keyword', 'keyword.other']],
    [
        'keyword.unsafe',
        ['storage.modifier', 'keyword.other', 'keyword.control', 'keyword']
    ],
    ['function', ['entity.name.function']],
    ['parameter', ['variable.parameter']],
    ['constant', ['constant', 'variable']],
    ['type', ['entity.name.type']],
    ['builtin', ['variable.language', 'support.type', 'support.type']],
    ['text', ['string', 'string.quoted', 'string.regexp']],
    ['attribute', ['keyword']],
    ['literal', ['string', 'string.quoted', 'string.regexp']],
    ['macro', ['support.other']],
    ['variable', ['variable']],
    ['variable.mut', ['variable', 'storage.modifier']],
    [
        'field',
        [
            'variable.object.property',
            'meta.field.declaration',
            'meta.definition.property',
            'variable.other'
        ]
    ],
    ['module', ['entity.name.section', 'entity.other']]
```


Co-authored-by: Seivan Heidari <seivan.heidari@icloud.com>
@bors bors bot closed this as completed in #2061 Dec 29, 2019
@lnicola lnicola mentioned this issue Dec 29, 2019
Closed
matklad pushed a commit to matklad/vscode-rust that referenced this issue Jul 13, 2020
@bors @seivan
Merge #2061
7f6751b 
2061: Theme loading and "editor.tokenColorCustomizations" support. r=matklad a=seivan

Fixes: [Issue#1294](rust-lang/rust-analyzer#1294 (comment))

TODO: 
- [x] Load themes
- [x] Load existing `ralsp`-prefixed overrides from `"workbench.colorCustomizations"`.
- [x] Load overrides from `"editor.tokenColorCustomizations.textMateRules"`.
- [x] Use RA tags to load `vscode.DecorationRenderOptions` (colors) from theme & overrides.
- [x] Map RA tags to common TextMate scopes before loading colors.
- [x] Add default scope mappings in extension.
- [x] Cache mappings between settings updates. 
- [x] Add scope mapping configuration manifest in `package.json`
- [x] Load configurable scope mappings from settings.
- [x] Load JSON Scheme for text mate scope rules in settings.
- [x] Update [Readme](https://github.com/seivan/rust-analyzer/blob/feature/themes/docs/user/README.md#settings).

Borrowed the theme loading (`scopes.ts`) from `Tree Sitter` with some modifications to reading `"editor.tokenColorCustomizations"` for merging with loaded themes and had to remove the async portions to be able to load it from settings updates. 

~Just a PoC and an idea I toyed around with a lot of room for improvement.~
For starters, certain keywords aren't part of the standard TextMate grammar, so it still reads colors from the `ralsp` prefixed values in `"workbench.colorCustomizations"`. 

But I think there's more value making the extension work with existing themes by maping some of the decoration tags to existing key or keys. 

<img width="453" alt="Screenshot 2019-11-09 at 17 43 18" src="https://user-images.githubusercontent.com/55424/68531968-71b4e380-0318-11ea-924e-cdbb8d5eae06.png">
<img width="780" alt="Screenshot 2019-11-09 at 17 41 45" src="https://user-images.githubusercontent.com/55424/68531950-4b8f4380-0318-11ea-8f85-24a84efaf23b.png">
<img width="468" alt="Screenshot 2019-11-09 at 17 40 29" src="https://user-images.githubusercontent.com/55424/68531952-51852480-0318-11ea-800a-6ae9215f5368.png">


These will merge with the default ones coming with the extension, so you don't have to implement all of them and works well with overrides defined in settings. 

```jsonc
    "editor.tokenColorCustomizations": {
        "textMateRules": [
            {
                "scope": "keyword",
                "settings": {
                    "fontStyle": "bold",
                }
            },
        ]
    },
```


Edit: The idea is to work with 90% of the themes out there by working within existing scopes available that are generally styled. It's not to say I want to erase the custom Rust scopes - those should still remain and eventually worked into a custom grammar bundle for Rust specific themes that target those, I just want to make it work with generic themes offered on the market place for now. 

A custom grammar bundle and themes for Rust specific scopes is out of... scope for this PR. 
We'll make another round to tackle those issues. 


Current fallbacks implemented

```typescript
    [
        'comment',
        [
            'comment',
            'comment.block',
            'comment.line',
            'comment.block.documentation'
        ]
    ],
    ['string', ['string']],
    ['keyword', ['keyword']],
    ['keyword.control', ['keyword.control', 'keyword', 'keyword.other']],
    [
        'keyword.unsafe',
        ['storage.modifier', 'keyword.other', 'keyword.control', 'keyword']
    ],
    ['function', ['entity.name.function']],
    ['parameter', ['variable.parameter']],
    ['constant', ['constant', 'variable']],
    ['type', ['entity.name.type']],
    ['builtin', ['variable.language', 'support.type', 'support.type']],
    ['text', ['string', 'string.quoted', 'string.regexp']],
    ['attribute', ['keyword']],
    ['literal', ['string', 'string.quoted', 'string.regexp']],
    ['macro', ['support.other']],
    ['variable', ['variable']],
    ['variable.mut', ['variable', 'storage.modifier']],
    [
        'field',
        [
            'variable.object.property',
            'meta.field.declaration',
            'meta.definition.property',
            'variable.other'
        ]
    ],
    ['module', ['entity.name.section', 'entity.other']]
```


Co-authored-by: Seivan Heidari <seivan.heidari@icloud.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
E-easy E-hard E-medium fun A technically challenging issue with high impact
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Theme loading and "editor.tokenColorCustomizations" support.
7 participants
@killercup @seivan @lnicola @univerz @matklad @chriskrycho @kjeremy