-
Notifications
You must be signed in to change notification settings - Fork 36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merge vscode-dotty-syntax in this repo to get Scala 3 indentation to work nicely #179
Comments
I've added some simpler rules for indentation here: https://github.com/scalameta/metals-vscode/blob/main/src/extension.ts#L1181-L1196 We shouldn't use the indentation rules from https://github.com/smarter/vscode-dotty-syntax/ as there is no way to figure out deindent. We could migrate what Metals does into |
Is that a problem? To deindent, users press backspace. |
Yes, because VS Code will automatically indent to the latest indentation if you do enter, which means the users will have to do backspace a lot, which is not a perfect UX. For example if you have:
if you press enter before end a it will get indented, which is not expected. |
ah I see what you mean thank you, so what happens with your version if you press enter at various points? |
It will remain the same as the previous indentation, but increase after pressing enter after the specific keywords. |
OK, that sounds fine, so we should move all that logic from metals-vscode into this repo as discussed in scalameta/metals-vscode#512. vscode-scala-syntax also does this:
which still looks like it's required and will also need to be copied in this repo. |
Should we indeed add it? vscode syntax doesn't know what Scala version you have, while Metals can provide keyword completions already for Scala 2. We will need to add them for Scala 3 for sure, but that is coming. |
Editing Scala 3 code with just vscode-scala-syntax should be pleasant to use, but right now it isn't because if I do: val thenable = 1
if foo then[ENTER] vscode will auto-complete to |
(And we can't rely on fixing this in metals since web versions of vscode like the one you get on github by pressing '.' on a repository can't use vscode-metals but can use this extension) |
So what will happen when you do:
? It would only autocomplete
On the other hand, we can't start treating Scala 2 as a second class citizen.
So this should be a bit more complex logic than just hardcoding keywords, we should take into account the context. |
Have you tried it? If
I agree, the only reason to have this keyword completion logic is to workaround the weird behavior of vscode of autocomplete-on-enter, maybe alternatively we could turn off that behavior in vscode-scala-syntax, all I care about is that users writing Scala 3 code don't need to fight their editor.
All I'm saying is we shouldn't make Scala 3 worse because of Scala 2, Scala 2 will be legacy eventually so this would be counter-productive. |
@fommil and I are both working on scala LSPs now, so moving this out of metals would be useful. There's probably also a broader conversation to be had about getting metals, ensime-tng, and whatever I'm doing to play nicely together. Perhaps something like with BSP support where there's an option to use Bloop or SBT as the build server? VSCode doesn't really let you manage conflicts between extensions. To try out Ensime I had to manually disable metals, install the Ensime .VSIX and restart vscode. Going back was more difficult - I had disable ensime, then actually uninstall and reinstall metals. Simply re-enabling the extension didn't work for some reason. I didn't try too hard to figure out why since I needed a functional IDE and ensime wasn't handling the repo I was working on at the time (still early days for ensime). Ideally I'd like see a comment pallet option "choose LSP", then when I know I need to save as much juice as possible I can easily switch to Ensime, when I want a more fully featured experience I can swap back to metals. I'm aiming for somewhere in between, but with more focus on staying as useful as possible when code isn't compiling. |
👍 Actually, there's an issue in
The rest part sounds more like providing a nice way to switch metals and ensime, which should be discussed in another issue. I'm not familiar with the ensime vscode extension well, but the idea off the top of my head would be merging the metals-vscode and ensime's vscode extension into one extension and letting users switch (that I'm not sure it's a good idea). |
Thanks - I'll create an issue in vscode-metals. I don't think merging them is a good idea but I have an idea which I'll mention in the new issue. |
I think moving some of the rules out of Metals is a good idea, but I haven't had the time to do it. Especially, that it was mostly needed for Metals and we could do more experimentation there. |
https://github.com/smarter/vscode-dotty-syntax/ does two things:
indentationRules
so that vscode auto-indents a newline after a keyword, and auto-unindent after end tokens (this one is maybe more likely to be annoying and could be dropped). Note that this one won't work if it is added in this extension right now when metals-vscode is also used because metals-vscode will override it (https://github.com/scalameta/metals-vscode/blob/acbb383e0ee606da45aeb56803de0f82b1cb4a1b/src/extension.ts#L925), so we should also integrate the indentation rules from metals in this extension and then remove them from metals-vscodeThe text was updated successfully, but these errors were encountered: