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

Highlight mutable variables differently #1544

Merged
merged 1 commit into from Jul 18, 2019

Conversation

@viorina
Copy link
Contributor

commented Jul 18, 2019

Screenshot from 2019-07-18 19-04-57

@matklad

This comment has been minimized.

Copy link
Collaborator

commented Jul 18, 2019

Cool!

To bad we can't just underline the identifier like IntelliJ does. That would certainly work better with rainbow highlighting.

bors r+

bors bot added a commit that referenced this pull request Jul 18, 2019

Merge #1544
1544: Highlight mutable variables differently r=matklad a=viorina

![Screenshot from 2019-07-18 19-04-57](https://user-images.githubusercontent.com/6714973/61473539-3f5d3000-a98f-11e9-99ec-a4115b2ba66b.png)


Co-authored-by: Ekaterina Babshukova <ekaterina.babshukova@yandex.ru>
@bors

This comment has been minimized.

Copy link
Contributor

commented Jul 18, 2019

@bors bors bot merged commit 4abe038 into rust-analyzer:master Jul 18, 2019

2 checks passed

bors Build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@viorina viorina deleted the viorina:mut-highlighting branch Jul 18, 2019

@matklad

This comment has been minimized.

Copy link
Collaborator

commented on crates/ra_syntax/src/lib.rs in 4abe038 Jul 18, 2019

Haven't noticed this during review: types from ast are intended to be used in "semi-qualified" form: ast::Pat, ast::PatKind

This comment has been minimized.

Copy link
Collaborator

replied Jul 18, 2019

fixed in #1547

@lnicola

This comment has been minimized.

Copy link
Contributor

commented Jul 19, 2019

To bad we can't just underline the identifier like IntelliJ does.

Just wondering, why can't we?

@matklad

This comment has been minimized.

Copy link
Collaborator

commented Jul 19, 2019

That's VS Code restriction: themes can only specify colors, and not decorations, like bold or underline

@lnicola

This comment has been minimized.

Copy link
Contributor

commented Jul 19, 2019

But the extension uses Code text decorations when highlighting is enabled, and they seem to support text-decoration?

@matklad

This comment has been minimized.

Copy link
Collaborator

commented Jul 19, 2019

Hm, hopefully I am wrong then :) I am thinking about colorCustomization https://github.com/matklad/config/blob/ed588057545276e05ea4979ea7086addc3724a4e/vscode/settings.json#L145-L159.

Looks like we definitely should create a separate decoration for mutable things

@lnicola

This comment has been minimized.

Copy link
Contributor

commented Jul 19, 2019

Like this?

image

diff --git i/editors/code/src/highlighting.ts w/editors/code/src/highlighting.ts
index f3ed6636..795d9b1f 100644
--- i/editors/code/src/highlighting.ts
+++ w/editors/code/src/highlighting.ts
@@ -28,12 +28,14 @@ export class Highlighter {
         string,
         vscode.TextEditorDecorationType
     > {
-        const colorContrib = (
-            tag: string
+        const decoration = (
+            tag: string,
+            textDecoration?: string
         ): [string, vscode.TextEditorDecorationType] => {
             const color = new vscode.ThemeColor('ralsp.' + tag);
             const decor = vscode.window.createTextEditorDecorationType({
-                color
+                color,
+                textDecoration,
             });
             return [tag, decor];
         };
@@ -41,24 +43,24 @@ export class Highlighter {
         const decorations: Iterable<
             [string, vscode.TextEditorDecorationType]
         > = [
-            colorContrib('comment'),
-            colorContrib('string'),
-            colorContrib('keyword'),
-            colorContrib('keyword.control'),
-            colorContrib('keyword.unsafe'),
-            colorContrib('function'),
-            colorContrib('parameter'),
-            colorContrib('constant'),
-            colorContrib('type'),
-            colorContrib('builtin'),
-            colorContrib('text'),
-            colorContrib('attribute'),
-            colorContrib('literal'),
-            colorContrib('macro'),
-            colorContrib('variable'),
-            colorContrib('variable.mut'),
-            colorContrib('field'),
-            colorContrib('module')
+            decoration('comment'),
+            decoration('string'),
+            decoration('keyword'),
+            decoration('keyword.control'),
+            decoration('keyword.unsafe'),
+            decoration('function'),
+            decoration('parameter'),
+            decoration('constant'),
+            decoration('type'),
+            decoration('builtin'),
+            decoration('text'),
+            decoration('attribute'),
+            decoration('literal'),
+            decoration('macro'),
+            decoration('variable'),
+            decoration('variable.mut', 'underline'),
+            decoration('field'),
+            decoration('module')
         ];
 
         return new Map<string, vscode.TextEditorDecorationType>(decorations);
@matklad

This comment has been minimized.

Copy link
Collaborator

commented Jul 19, 2019

Yeah, seems like this should do the trick. Does it compose well with rainbow highlighting? Could you send a PR? ^^

@matklad

This comment has been minimized.

Copy link
Collaborator

commented Jul 19, 2019

@lnicola ok, I'll do this myself because I want this sooo much! Thanks for pointing out that this is possible!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.