ionide / ionide-fsgrammar

Shared Textmate Regex Style F# Language Grammar
MIT License
35 stars 36 forks source link

Infix multiplication operator screws up parentheses coloring and is highlighted in red #209

Open mbottini opened 8 months ago

mbottini commented 8 months ago

Describe the bug First, the infix multiplication operator (*) is highlighted in red, differently from the other operators.

image

And second, likely as a result of not being captured as a parenthesized operator, the opening paren of the operator is not counted, and the closing paren is counted for the purposes of matching corresponding parentheses. In this screenshot, the closing paren of the operator is matched with the parenthesis before Seq.fold:

image

To Reproduce Steps to reproduce the behaviour:

  1. Open up a new file and type (*) to show the red-highlighted operator.

     (*)
  2. Call the (*) operator inside of another parenthesized expression, which will cause the operator's closing paren to match with the opening paren of the outer expression.

    (Seq.fold (*) 1 [1;2;3])

Expected behaviour The (*) operator should be treated identically to the other operators. At a minimum, it should not affect the parenthesis coloring of outer expressions.

Environment (please complete the following information):

mbottini commented 8 months ago

It appears to me that this is correctly captured by the grammar. See the following comparisons between (*) and (+) with regard to what the Scope Inspector has to say. They're the same.

image

image

Note that the color (which should be something like #c21c1a )doesn't line up. Something else is formatting the paren color this way.

mbottini commented 8 months ago

This did it. I done goofed - Ionide has nothing to do with this. This is a VSCode bug, I'll go bug them.

image

mbottini commented 8 months ago

After a bunch of work to download and compile VSCode and adding the updated grammar file, the issue persists with the bracket pair colorization. There's something in the grammar that's making it unhappy with the parens. I'll keep digging!

mbottini commented 8 months ago

Found it - had to dig through VSCode's source for how it's doing its bracket colorization.

fsharp.syntaxtest/language-configuration.json defines extra bracket pairs from what is in the base VSCode extension package. Here's what VSCode has:

"brackets": [
    ["{", "}"],
    ["[", "]"],
    ["(", ")"]
]

And here's what Ionide has:

"brackets": [
   ["(", ")"],
   ["(*", "*)"],
   ["{", "}"],
   ["[", "]"],
   ["[|", "|]"],
   ["<@", "@>"],
   ["<@@", "@@>"]
}

Should the block comments be in there? They usually aren't in there for other languages.

mbottini commented 8 months ago

Sure enough, removing ["(*", "*)"], from the "brackets" field fixes it.

image

I'll put in the pull request - other folks can opine on whether this is a good idea or not.

MangelMaxime commented 8 months ago

I think the block comments are in there to support automatic closing of the "brackets".

https://github.com/ionide/ionide-fsgrammar/assets/4760796/31dfb6b0-361c-4c00-a52a-2db3af2778db

You can spot an highlighting error in my video 🙈

mbottini commented 8 months ago

I think the block comments are in there to support automatic closing of the "brackets". CleanShot.2023-12-22.at.09.53.48.mp4

You can spot an highlighting error in my video 🙈

That's a separate field (autoClosingPairs) in the language_configuration.json file, which I haven't touched. Similarly, surroundingPairs allows you to highlight a selection and then enclose it, but it doesn't seem to work with (* *) whether it's in the brackets field or not. That might be another issue for me to try to chase down!

MangelMaxime commented 8 months ago

@mbottini Thank you for the confirmation.

mbottini commented 8 months ago

You can spot an highlighting error in my video 🙈

I missed this part - that's interesting. Consider that when apart, VSCode handles them correctly:

image

But when they're together, VSCode's bracket balancer greedily matches on <@@ and complains that it's unbalanced. I should go bug them about that - it's a subtle edge case that they should be able to test for.

mbottini commented 5 months ago

Oh no, I didn't realize that all of my edits to the commit message were going to get shown here! I was using the pull request window to render the Markdown! Whoops. Maybe a maintainer can collapse all of those commits...