Fix ionide#209: Remove (* *) from brackets config #210
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
language-configuration.json lets you define various pairs of brackets and braces for autoclosing, surrounding selected text with characters, and, as it turns out, dictating which brackets get colorized by the native bracket colorizer. Currently, having `(* *)` inside the `brackets` field is causing the infix multiplication operator to be treated by the bracket colorizer as two brackets: an opening `(*` and a closing `)`. Two things happen here: * The `(*` does not correspond to a corresponding `*)` bracket, so it is treated as unbalanced (hence why that part of the operator is colored red). * Absent any preceding parens in the expression, the `)` is also treated as an unbalanced paren and is colored red. This is the best case, which colors the entire operator a uniform red. Strange, but normal-looking enough that it actually looked intentional to me when I first started getting into F#. The multiplication operator is weird due to how similar it is to block comments - maybe it's supposed to be that way! * But the bug really gets exposed when there is a preceding paren in the expression such as in the expression `(Seq.fold (*) 1 [1;2;3])`. In this case, there *is* a balancing paren - the paren that precedes `Seq.fold`. Now the multiplication operator is half red (the block comment bracket never gets balanced) and half whatever the colorizer picks for the two parens. And now the closing paren after `[1;2;3]` is unbalanced and made red! As far as I can tell, putting the block comment brackets inside the `brackets` field is pointless. We color our comments green anyway, so the colors don't show anyway. It is true that F# provides support for nested comment blocks, but we aren't taking advantage of the color feature anyway (and I'm unsure if it's even possible to do so). The block comments should, however, remain in the other fields that control autocomplete and surrounding selected text.
mbottini
force-pushed
the
mbottini/fix_block_comment_brackets
branch
from
aa2565a to
81edefe
Compare
|
This seems reasonable to me - @MangelMaxime do you have any concerns here? |
|
I don't have concerns regarding the grammar. However, applying theses changes means: We will lose:
and "auto-indentation": Before CleanShot.2023-12-28.at.11.00.06.mp4After CleanShot.2023-12-28.at.10.58.58.mp4 |
|
This is a good point. The place to solve this might be upstream in VSCode's approach to finding brackets in a file (https://github.com/microsoft/vscode/blob/main/src/vs/editor/common/languages/supports/richEditBrackets.ts). Currently, they do a pretty simple approach - they build a big regex from all of the bracket pairs and apply it in both directions (reversing the regex when they traverse the file toward the beginning). Brackets are matched greedily. This approach fails for this particular edge case, where the regex matches on Regarding auto-indentation - it's not pretty, but the |
|
@mbottini We do indeed have some rules for indentation rules in https://github.com/ionide/ionide-vscode-fsharp/blob/5645d12af2293adaa995a8841c8eec9ad709fbd2/src/Components/LanguageConfiguration.fs So perhaps, we can compensate the removal by adapting the |
mbottini
added a commit
to mbottini/ionide-vscode-fsharp
that referenced
this pull request
Apr 1, 2024
Background: ionide/ionide-fsgrammar#209 and ionide/ionide-fsgrammar#210 This comes from a comment regarding a pull request that I made on the ionide-fsgrammar repository, which removes the `(* *)` bracket pair from the "brackets" field of language-configuration.json. One issue with doing this is that we lose the bracket-like indentation that VSCode provides by default for all bracket pairs. In other words, when I hit Enter in the following configuration, cursor location represented by the white block: (*█*) It should indent the cursor and then put the `*)` on the next line with the same indentation level as the `(*` as follows: (* █ *) --- When I hit Enter with an unaccompanied `(*`: (*█ It should simply indent as follows: (* █ --- Lastly, an unaccompanied `*)` should outdent. That is, *)█ should become *)█
mbottini
added a commit
to mbottini/ionide-vscode-fsharp
that referenced
this pull request
Apr 1, 2024
Background: ionide/ionide-fsgrammar#209 and ionide/ionide-fsgrammar#210 This comes from a comment regarding a pull request that I made on the ionide-fsgrammar repository, which removes the `(* *)` bracket pair from the "brackets" field of language-configuration.json. One issue with doing this is that we lose the bracket-like indentation that VSCode provides by default for all bracket pairs. Since I'd like to remove the block comment brackets from the `language-configuration.json` file, this pull request re-adds the same semantics to the indentation rules. In other words, when I hit Enter in the following configuration, cursor location represented by the white block: (*█*) It should indent the cursor and then put the `*)` on the next line with the same indentation level as the `(*` as follows: (* █ *) --- When I hit Enter with an unaccompanied `(*`: (*█ It should simply indent as follows: (* █ --- Lastly, an unaccompanied `*)` should outdent. That is, *)█ should become *)█
mbottini
added a commit
to mbottini/ionide-vscode-fsharp
that referenced
this pull request
Apr 1, 2024
Background: ionide/ionide-fsgrammar#209 and ionide/ionide-fsgrammar#210 This comes from a comment regarding a pull request that I made on the ionide-fsgrammar repository, which removes the `(* *)` bracket pair from the "brackets" field of language-configuration.json. One issue with doing this is that we lose the bracket-like indentation that VSCode provides by default for all bracket pairs. This pull request re-adds the same semantics to the indentation rules. In other words, when I hit Enter in the following configuration, cursor location represented by the white block: (*█*) It should indent the cursor and then put the `*)` on the next line with the same indentation level as the `(*` as follows: (* █ *) --- When I hit Enter with an unaccompanied `(*`: (*█ It should simply indent as follows: (* █ --- Lastly, an unaccompanied `*)` should outdent. That is, *)█ should become *)█
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Remove
(* *)from thebracketsfield oflanguage-configuration.json. This prevents the(*part of the(*)operator from being treated as an unclosed bracket, followed by the)bracket being associated with a previous paren.The only side effect of this is that you'd lose nested bracket colorization for block comments, (which F# does support!) but we currently color comments a uniform green anyway.
Before:
After:
This fixes #209.