scalameta / metals-vscode

Visual Studio Code extension for Metals
https://marketplace.visualstudio.com/items?itemName=scalameta.metals#overview
Apache License 2.0
298 stars 75 forks source link

Move custom indentation rules to vscode-scala-syntax instead of overwriting its rules #512

Open smarter opened 3 years ago

smarter commented 3 years ago

Currently metals-vscode calls setLanguageConfiguration to provide indentation rules for scaladoc: https://github.com/scalameta/metals-vscode/blob/b68b2f5eddd4e19c6626f24d9f62500f95b8493e/src/extension.ts#L937-L982 The problem with that is that it overwrites any rules set for the scala language by vscode-scala-syntax. Currently there isn't any but we would like to add Scala 3 indentation rules there, see https://github.com/scala/vscode-scala-syntax/issues/179. Step 1 is to get a language-configuration.json in vscode-scala-syntax with the scaladoc rules, step 2 will be to remove these rules from metals-vscode.

tgodzik commented 3 years ago

Thanks for reporting! For sure it makes sense to move the indentation rules to scala-syntax. Do you plan on adding the language configuration for Dotty there or should we take ownership of the full effort?

smarter commented 3 years ago

should we take ownership of the full effort?

Please go ahead and do that if you're interested!

tgodzik commented 3 years ago

I will put it on the list, but I've got some things that are more urgent (migrating Bloop to newest zinc :fearful: )

tgodzik commented 2 years ago

I tried some of the indentation rules and it seems we cannot use them as those rules expect to have indent as well as deindent pairs. Wit optional braces this will not work, since there can be no end marker. I did add some additional rules about increasing indentation though and that seems to work for now.

smarter commented 2 years ago

I don't understand why this issue was closed. Regardless of which rules should be used the problem here is that metals-vscode overwrites any rule set by vscode-scala-syntax. The solution is to move all rules to vscode-scala-syntax where we can then evolve things.

tgodzik commented 2 years ago

I only added some simple onEnterRules here https://github.com/scalameta/metals-vscode/blob/main/src/extension.ts#L1181-L1196

I am not sure if we should migrate it all to Scala syntax? I will mention the changes in the other issue, but this anyway needs to be done first in vscode-syntax.

smarter commented 2 years ago

I am not sure if we should migrate it all to Scala syntax?

I'm pretty sure we should, this is about syntax support and is completely independent of metals.

smarter commented 2 years ago

Especially now that scala is indentation-based, it's important that editors support scala editing correctly regardless of whether metals is enabled or not (for example, when editing code in a web-based version of vscode where metals cannot be run)

tgodzik commented 2 years ago

I commented on the other issue as the actual work is required in the syntax repo.

smarter commented 2 years ago

Work is required in both repositories as mentioned in this issue:

Step 1 is to get a language-configuration.json in vscode-scala-syntax with the scaladoc rules, step 2 will be to remove these rules from metals-vscode.

So I believe this issue should be kept open. I haven't worked on step 1 since you mentioned you were going to do it in an earlier comment in this issue.

tgodzik commented 2 years ago

I figured the issue was no longer relevant since we are using different rules, I can reopen it.

tgodzik commented 2 years ago

Sorry about closing prematurely, I was bit tired and that got me cranky + in a mood to close old issues :sweat_smile: