mshr-h / vscode-verilog-hdl-support

HDL support for VS Code
MIT License
288 stars 75 forks source link

[BUG] `property` token breaks bracket colorization #348

Open djoffe opened 1 year ago

djoffe commented 1 year ago

Describe the bug A fairly recent feature of vscode is bracket pair colorization. It looks like the property token counts as an opening bracket that, if used inside an assert or cover statement. However, in these cases, there is no endproperty token to balance the opening token.

This leads to disgracefully bracket colorization that looks like a christmas tree 🎄: image

Environment (please complete the following information):

Steps to reproduce Steps to reproduce the behavior:

Expected behavior The brackets on the two similar consecutive lines should have the same color: image

Actual behavior The brackets on the two similar consecutive lines have different color: image

mshr-h commented 1 year ago

That's definitely a bug!

I think the only solution is to remove the property endproperty pair from the bracket definition here. I'm not an active SystemVerilog user, so I don't know if doing that is ok.

What do you think? @djoffe @Raamakrishnan

Raamakrishnan commented 1 year ago

Ya, I feel the same. As @djoffe said, property can have a closing endproperty or not, depending on the way it is used. And I don't see a way to make the brackets optional in vscode language definitions.

mshr-h commented 1 year ago

We have 2 options.

  1. remove the property...endproperty pair from the bracket definition.
    • bracket colorization will work. but for example, we can't jump property <-> endproperty.
  2. do nothing.
    • Opposite to option 1

I've checked some code on chipsalliance. Most of the property...endproperty has less than 10 lines of body. In my opinion, option 1 is better.

Raamakrishnan commented 1 year ago

Option 3: Add a colorizedBracketPairs tag in systemverilog.configuration.json. Add all the brackets, begin..end and others, except property...endproperty. Bracket colorization will work for all brackets except for property...endproperty. But you can still jump between property <-> endproperty

I can't find any docs about this tag in language definition file, but there is a editor.language.colorizedBracketPairs info here

I tried this tag in language definition file and it works. Do you want me to send a PR?

mshr-h commented 1 year ago

@Raamakrishnan Sounds good to me. Can you make the PR?

DeflateAwning commented 6 months ago

There's no way to specify with a regex or similar that sometimes "brackets" are used for things other than pairs? For example, in Python, consider this example:

a = 1
print(f"These curly braces are start/end brackets: {a}")
print("But this random curly brace { does not mess with the upcoming curly braces.")
print(f"Here's a again, with more curly braces that highlight properly: {a}")

This is still an issue; almost opened a duplicate issue. Here's yet another example:

image