vscode-scala-syntax icon indicating copy to clipboard operation
vscode-scala-syntax copied to clipboard

Merge vscode-dotty-syntax in this repo to get Scala 3 indentation to work nicely

Open smarter opened this issue 4 years ago • 15 comments

https://github.com/smarter/vscode-dotty-syntax/ does two things:

  • Install a completion provider for every keyword so that vscode doesn't auto-complete a keyword into some other word when pressing Enter after typing a keyword.
  • Set a custom 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-vscode

smarter avatar Dec 21 '20 15:12 smarter

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 vscode-scala-syntax if there is anyone that feels like it would be useful.

tgodzik avatar Jan 20 '22 16:01 tgodzik

as there is no way to figure out deindent.

Is that a problem? To deindent, users press backspace.

smarter avatar Jan 20 '22 16:01 smarter

as there is no way to figure out deindent.

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:

def a = 
  def b = 
    def c=
      1
    b
  c
end a

if you press enter before end a it will get indented, which is not expected.

tgodzik avatar Jan 20 '22 16:01 tgodzik

ah I see what you mean thank you, so what happens with your version if you press enter at various points?

smarter avatar Jan 20 '22 16:01 smarter

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.

tgodzik avatar Jan 20 '22 16:01 tgodzik

OK, that sounds fine, so we should move all that logic from metals-vscode into this repo as discussed in https://github.com/scalameta/metals-vscode/issues/512. vscode-scala-syntax also does this:

Install a completion provider for every keyword so that vscode doesn't auto-complete a keyword into some other word when pressing Enter after typing a keyword.

which still looks like it's required and will also need to be copied in this repo.

smarter avatar Jan 20 '22 16:01 smarter

Install a completion provider for every keyword so that vscode doesn't auto-complete a keyword into some other word when pressing Enter after typing a keyword.

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.

tgodzik avatar Jan 20 '22 17:01 tgodzik

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 thenable. IMO this isn't acceptable, and having a few extra keywords available as completion in a Scala 2 project is a price worth paying to avoid this issue. More generally, I think official Scala tooling should commit to providing the best user experience for Scala 3 and not treat it as a second class citizen, otherwise users will never switch and we'll be stuck in migration hell indefinitely.

smarter avatar Jan 20 '22 18:01 smarter

(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)

smarter avatar Jan 20 '22 18:01 smarter

So what will happen when you do:

val other = the[ENTER]

?

It would only autocomplete then, but not thenable ? I don't believe it's a problem to write most of the keywords by hand and o still allow people have names that may or may not include keywords. Also, what about soft keywords?

More generally, I think official Scala tooling should commit to providing the best user experience for Scala 3 and not treat it as a second class citizen, otherwise users will never switch and we'll be stuck in migration hell indefinitely.

On the other hand, we can't start treating Scala 2 as a second class citizen.

(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 this should be a bit more complex logic than just hardcoding keywords, we should take into account the context.

tgodzik avatar Jan 21 '22 08:01 tgodzik

It would only autocomplete then, but not thenable ?

Have you tried it? If the is a valid completion it won't autocomplete anything, otherwise it will pick the first completion it finds which might be then.

I don't believe it's a problem to write most of the keywords by hand

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.

we can't start treating Scala 2 as a second class citizen.

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.

smarter avatar Jan 21 '22 12:01 smarter

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 vscode-scala-syntax if there is anyone that feels like it would be useful.

@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.

bertlebee avatar Oct 25 '22 00:10 bertlebee

moving this out of metals would be useful.

:+1: Actually, there's an issue in metals-vscode https://github.com/scalameta/metals-vscode/issues/512 but requires some work.


There's probably also a broader conversation to be had about getting metals, ensime-tng...

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). AFAIK, there's no way to control other extensions from one extension other than setting Recommended extensions. The first thing we should do would be to check why disabling one extension doesn't work, I guess.

tanishiking avatar Oct 25 '22 05:10 tanishiking

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.

bertlebee avatar Oct 25 '22 08:10 bertlebee

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.

tgodzik avatar Oct 25 '22 08:10 tgodzik