eirikpre / VSCode-SystemVerilog

SystemVerilog support in VS Code
MIT License
128 stars 50 forks source link

Syntax Highlighting #113

Closed sconwayaus closed 3 years ago

sconwayaus commented 3 years ago

Hi,

Updated to version 11 and noticed a few odd things with the syntax highlighting. I've screen grabbed version 0.11.2 (latest as of today on the right) and version 0.10.11 (left).

Screenshot 2021-02-01 140130

Case statement shows different colors of STATE_1 an STATE_2 on line 32.

FWIW, I also preferred the light green used for the parameter and names in the enumeration (lines 2, 3 and 12-16).

As an aside, the new highlighter didn't pick up that I typed "parameter" incorrectly on lines 2 and 3, where as the old syntax highlighting did.

jecassis commented 3 years ago

Thank you for the example. Found a couple of other cases that needed correcting and will push an update. Specifically from the example line 32 has already been fixed by the port-net-parameter grammar. Also note that typedef enum unsigned int is not legal according to my reading of the LRM. The unsigned keyword should go last in this case.

As for the color preference, all the syntax highlighting does is classify each tokens to a specific scope. The colors are determined by your theme. You can override the theme's colors using editor.tokenColorCustomizations in settings.json to suit your tastes. For example the STATE_* parameters you mention are classified as constant.other.net.systemverilog. Note that this does not include any semantic information. To find the scope for each token run editor.action.inspectTMScopes.

The example from this issue using a minimally-modified One Dark Pro theme is shown below:

formatting
sconwayaus commented 3 years ago

Thanks Jecassis,

You're right, enum should be "int unsigned" and thanks for the tip on the theme settings. Looking forward to the update.

Cheers Steve

On Mon, 1 Feb 2021 at 19:32, jecassis notifications@github.com wrote:

Thank you for the example. Found a couple of other cases that needed correcting and will push an update. Specifically from the example line 32 has already been fixed by the port-net-parameter grammar. Also note that typedef enum unsigned int is not legal according to my reading of the LRM. The unsigned keyword should go last in this case.

As for the color preference, all the syntax highlighting does is classify each tokens to a specific scope. The colors are determined by your theme. You can override the theme's colors using editor.tokenColorCustomizations in settings.json to suit your tastes. For example the STATE_* parameters you mention are classified as constant.other.net.systemverilog. Note that this does not include any semantic information. To find the scope for each token run editor.action.inspectTMScopes.

The example from this issue using a minimally-modified One Dark Pro theme is shown below:

[image: formatting] https://user-images.githubusercontent.com/36133/106432468-9e747f80-6423-11eb-9c50-812d29e19386.png

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/eirikpre/VSCode-SystemVerilog/issues/113#issuecomment-770673652, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABVJSI5USAEGMPSTGHWV77TS4ZRJ7ANCNFSM4W4ASHTA .

roccomao commented 3 years ago

There is a related syntax highlighting issue.

When the definition of enum type is written on one line, the syntax highlighting is wrong, but it is correct in lowercase, as shown below:

image

jecassis commented 3 years ago

Thanks @sconwayaus. @bitrocco, single-line typedefs like this are fixed in #112. That said, let me clarify what you should expect. There is a small concession made in the syntax matcher to mark identifiers in all caps using the constant classification/coloring. Without semantic information, the matcher prioritizes this as a best guess based on the various coding styles I have come across in the industry such as this one. Many lint tools even enforce these kind of conventions in their default ruleset.

Typedef enums

roccomao commented 3 years ago

@jecassis Thanks for your reply.

Sorry for the previous unclear explanation, but the original intention is that the enum_identifier in the enumeration declaration should have the same highlight color. However, the #112 solution is also accepted, because it generally follows the same code style. So look forward to the next version update :smile:.

eirikpre commented 3 years ago

Should be ok with the last release 0.11.3 :)