Improved kotlin syntax based on latest documentation #137

per-steinar · 2023-10-26T07:56:27Z

Improved syntax based on documentation from https://kotlinlang.org/

Summary

Improved scopes for imports
Separating val and var
Improved capturing of functions and parameters
Added variable and object capturing
Added capturing of keyword for mapping
Added capturing of optional accessor
Added capturing of elvis operator

themkat · 2023-11-24T16:51:14Z

@fwcd , this PR is almost a month old now. Any chance to look at it soon? I don't really have many permissions in this repo. Seems quite reasonable to me though 🙂

per-steinar · 2023-12-02T08:50:14Z

@fwcd I have added some screenshots to redhat-developer/vscode-java#3407

fwcd

Thanks! Left some quick thoughts below

syntaxes/kotlin.tmLanguage.json

fwcd · 2024-01-14T05:20:10Z

syntaxes/kotlin.tmLanguage.json

            }
        },
        "variable-declaration": {
-            "match": "\\b(val|var)\\b\\s*(?<GROUP><([^<>]|\\g<GROUP>)+>)?",
+            "match": "\\b(var)\\b\\s*(?<GROUP><([^<>]|\\g<GROUP>)+>)?",


The separation of var and val is interesting as it lets us highlight constants differently.

On the other hand, I worry that this duplication will increase the maintainance burden. In general, I think "being liberal" about allowing the same syntactic constructs for var and val should be fine for syntax highlighting.

Curious to know how other languages handle this.

To be honest, I'm not sure where I got the idea to create a separate scope for val. I have checked how the TypeScript grammar handles this, and it combines let, const and var.

C# uses readonly scope, but only for the readonly keyword https://github.com/microsoft/vscode/blob/83127004a87124b1b8b976b9f9022da41814dfd0/extensions/csharp/syntaxes/csharp.tmLanguage.json#L3034

On the other hand, both BuildShip and the RH Java Extension accepted this, so it is up to you if I should revert it.
https://github.com/eclipse/buildship/blob/0800e1c1ea25a3afca2a113a0747efe822a65273/org.eclipse.buildship.kotlindsl.provider/kotlin.tmLanguage.json#L445

I have searched a bit more, and I added the readonly distinction because of the "Semantic Highlight Guide" documentations for VSCode mentions it.
See: https://code.visualstudio.com/api/language-extensions/semantic-highlight-guide

However, it does not look like it is common to be used for the const/val keywords, only for the readonly keyword.
I can revert the change, it you prefer.

I have also made some changes to the definitions for val/var to be more in line with how the java grammar handles local var.

I may be a bit biased when I'm using the Java grammar for guidance, but I'm only using Kotlin in conjunction with Java, so that's what i know.

fwcd · 2024-01-14T22:23:14Z

syntaxes/kotlin.tmLanguage.json

@@ -19,10 +19,13 @@
    ],
    "repository": {
        "import": {
-            "begin": "\\b(import)\\b\\s*",
+            "begin": "\\b(import)\\b\\s*([\\w+.]*\\w+)?\\s*",


Hm, I am not entirely convinced this rule is correct. This will match the package too in imports and highlight it as if it were a modifier:

For comparison, this is the current state which, IMO, is more correct:

I cannot reproduce this. I have tried with both my fork of the extension and by creating an empty extension with only the grammar from the PR:

Edit: what is the scope of the parts in green (Before, Test etc.)?

Yes, your color scheme is different, but the package shouldn't have scope storage.modifier.*. If you enable the Dark+ or Dark Modern scheme, you should get the same result.

The green parts are semantic tokens provided by the language server, not by the grammar:

I have looked on how the java grammar handles this. ~~It looks a bit more sophisticated than this. I am not sure if I will be able to fix it this week~~

Edit: I have pushed an update to the definitions to be more in line with the scoped used by the Java grammar bundled with VSCode:

fwcd · 2024-01-15T21:46:49Z

syntaxes/kotlin.tmLanguage.json

+            }
+        },
+        "variable": {
+            "match": "\\b(\\w+)(?=\\s*[:=])",


This isn't quite right either, this will match anything on the left-hand side of == too, including e.g. numbers:

Anything involving = will likely require being very careful to not accidentally match any equality operators. This includes cases where e.g. the space is left out, i.e. expressions like (x!=3)...

I'm removing this definition completely. It need a bit more work to be implemented correctly.

Update syntaxes/kotlin.tmLanguage.json Our suggestion is the same as the one in the built in definition for java, so I guess it's the proper way to do it https://github.com/microsoft/vscode/blob/12443ca030f93843ca5e9dfb5682ad92d2c6400c/extensions/java/syntaxes/java.tmLanguage.json#L53 Co-authored-by: fwcd <30873659+fwcd@users.noreply.github.com> Removed definitions for reassigning varaibles

…mmar for java

…defined in java grammar

per-steinar · 2024-01-16T17:12:22Z

Thanks for all the feedback. I have tried to fix the issues you have found.

fwcd · 2024-07-28T21:38:01Z

Please rebase onto the latest main branch, #150 made some minor changes to the way keywords are highlighted.

fwcd reviewed Jan 14, 2024

View reviewed changes

fwcd mentioned this pull request Jan 15, 2024

Improved grammar for kotlin based on official documentation redhat-developer/vscode-java#3407

Merged

fwcd added the grammar Related to the TextMate grammar, i.e. syntax highlighting label Jan 15, 2024

fwcd reviewed Jan 15, 2024

View reviewed changes

per-steinar force-pushed the improved-syntax branch 2 times, most recently from 3535583 to eb7c3a3 Compare January 16, 2024 17:02

per-steinar force-pushed the improved-syntax branch from eb7c3a3 to ed3baf8 Compare January 16, 2024 17:04

per-steinar added 3 commits January 16, 2024 18:08

Fixed map-keyword and parameter-declaration

45bbaea

Updated grammar for imports an packages to be in line with vscode gra…

74028cd

…mmar for java

Improved variable and constant declaration based on how local var is …

d95a0a4

…defined in java grammar

per-steinar force-pushed the improved-syntax branch from ed3baf8 to d95a0a4 Compare January 16, 2024 17:09

Consolidated definitions for var and val

2ef0ec5

per-steinar closed this Jul 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improved kotlin syntax based on latest documentation #137

Improved kotlin syntax based on latest documentation #137

per-steinar commented Oct 26, 2023 •

edited

Loading

themkat commented Nov 24, 2023 •

edited

Loading

per-steinar commented Dec 2, 2023

fwcd left a comment

fwcd Jan 14, 2024

per-steinar Jan 15, 2024 •

edited

Loading

per-steinar Jan 16, 2024

fwcd Jan 14, 2024

per-steinar Jan 15, 2024 •

edited

Loading

fwcd Jan 15, 2024 •

edited

Loading

per-steinar Jan 16, 2024 •

edited

Loading

fwcd Jan 15, 2024

per-steinar Jan 16, 2024

per-steinar commented Jan 16, 2024

fwcd commented Jul 28, 2024

Improved kotlin syntax based on latest documentation #137

Improved kotlin syntax based on latest documentation #137

Conversation

per-steinar commented Oct 26, 2023 • edited Loading

Summary

themkat commented Nov 24, 2023 • edited Loading

per-steinar commented Dec 2, 2023

fwcd left a comment

Choose a reason for hiding this comment

fwcd Jan 14, 2024

Choose a reason for hiding this comment

per-steinar Jan 15, 2024 • edited Loading

Choose a reason for hiding this comment

per-steinar Jan 16, 2024

Choose a reason for hiding this comment

fwcd Jan 14, 2024

Choose a reason for hiding this comment

per-steinar Jan 15, 2024 • edited Loading

Choose a reason for hiding this comment

fwcd Jan 15, 2024 • edited Loading

Choose a reason for hiding this comment

per-steinar Jan 16, 2024 • edited Loading

Choose a reason for hiding this comment

fwcd Jan 15, 2024

Choose a reason for hiding this comment

per-steinar Jan 16, 2024

Choose a reason for hiding this comment

per-steinar commented Jan 16, 2024

fwcd commented Jul 28, 2024

per-steinar commented Oct 26, 2023 •

edited

Loading

themkat commented Nov 24, 2023 •

edited

Loading

per-steinar Jan 15, 2024 •

edited

Loading

per-steinar Jan 15, 2024 •

edited

Loading

fwcd Jan 15, 2024 •

edited

Loading

per-steinar Jan 16, 2024 •

edited

Loading