Metals improvements feedback

Tooling
1

I realised recently that there is a lot of smaller improvements that we could do, which would improve people’s lives when using Metals or Intellij (they would probably be glad for similar inspiration).

What prompted it is some great work done in by Michał Pawlik in Convert sbt style deps on paste in for scala-cli by majk-p · Pull Request #7176 · scalameta/metals · GitHub

He told me what he felt like a useful improvement and it turned out not be hard to do at all. With some small guidance from me we managed to get it merged in a few days.

My question is this: if you see something that metals didn’t do and it feels that it should be able to do that please let us know! This could also be small annoyances like “Why didn’t metals not realise that only this import makes sense”.

These improvements could be:

  • metals did suggest too much while some of the suggestions didn’t make sense
  • you had to do a lot of repetitive manual work like changing dependency import syntax
  • probably more that I can’t currently think of…

We can even fix some of those during the sprees if anyone fancy working on them themselves.

I would be glad if anyone can think of some more feedback that can pop up when editing your code or just using your ide

10 Likes
2

We need sbt because of plugins.
When I try to switch to metals I always ask myself:
Is it possible to compile only when I save files(or any other hotkey)?

  • May be it can save battery
  • I want to know when I can reload classloader in an existing java process, I do not understand when files are correctly compiled( May be it is obviouse but I am not expirence user in metals)

In idea I just press the key and after compilation. I am happy ).

Is it possible to defer compilation without dramatical degradation of code assistant?

We have plans to use openvscode-server in containers.

  • Is it possible to prepare project in background to open it faster in vscode for web?
  • Should the Doctor work in vscode for web(It is not working when we test it in some openvscode-server version)?
3

That’s what happens normally aside from presentation compiler, which needs to have up to date compilation of the current file.

We do try it in Scala 2 with outline compilation, but it’s not perfect. However, if working within a single file you should not have any issues.

We do a similar thing in GitHub - scalameta/metals-gitpod-sample though I haven’t updated it for a while. I haven’t used openvscode so not sure we can have the same behaviour.

The doctor should work since it’s just webview being sent to the client.

2 Likes
4
  • The “extract” code features need work, when in java I regularly extract expressions onto variables, fields and methods. When doing so I’d like to be able to specify name and visibility for the extracted expression (when appropriate). In case of methods, parameters should have meaningful names based on the variables that were being used in the original expression.
  • I’d love to have a “explore AST” feature when I work with macros, where I can go to actual code, right click on a portion of code and select that option and it’d take me to the full ast of the file with that particular region highlighted.
    This feature can fulfill the double role of also letting me see the result of the macro expansion.
  • When creating the exhaustive cases for match, metals will add imports violating my coding style, I prefer doing case SomePrefix.SomeConstant => than having import SomePrefix.*, particularly because constant names do typically clash with libraries… like “Success”. I’d use this feature a lot more if I didn’t feel I have to defend against the code it generates. Oh, it should also respect my coding style with braces, instead of generating it without them.
  • Replace braces with parenthesis and viceversa often doesn’t do a good job, plus it never adds the reasonable spacing.
  • I have a macro to show me “desugared” (dealiased + widened versions) alias types, the macro is trivial to implement but I feel the IDE should be doing it. When working with match types this feels like a must, since they often just don’t expand anymore
  • I use worksheets a lot, and I often ran out of memory after several evaluations. When this happens, metals goes into full GC cycle and stops responding so I typically have to restart it (by reloading the window), but note that the old metals process doesn’t go away and remains there using as much cpu as it can trying to GC. I’d rather have metals monitor its ram usage and warn when it’s above a certain threshold so I can restart before it becomes a problem.

This is off the top of my head, hope it helps.

1 Like
5

This is a good one. Probably a good rule of thumb (for me) would be in case of enums or hierarchies where the case classes or objects are defined in the companion object of the sealed trait/class, don’t import the cases but import the prefix.

6

Would it be possible to make buildTarget in remote debugging optional?

In multi project build when we need to debug all project simultaneously. This option is confusing. If we set buildTarget randomly some breakpoints do not work steadily.

1 Like
7

That’s a lot of great examples, I will try to convert them into issues over the coming week, though would be very thankful for some more details.

  • Replace braces with parenthesis and viceversa often doesn’t do a good job, plus it never adds the reasonable spacing.

Would you be able to come up with examples where it breaks and what you would expect?

  • I’d love to have a “explore AST” feature when I work with macros, where I can go to actual code, right click on a portion of code and select that option and it’d take me to the full ast of the file with that particular region highlighted.
    This feature can fulfill the double role of also letting me see the result of the macro expansion.

We allow to go to tasty representation of every file, but we don’t highlight the currently selected part, that might be possible to do. I guess though you would want something more than just a text representation though?

  • I have a macro to show me “desugared” (dealiased + widened versions) alias types, the macro is trivial to implement but I feel the IDE should be doing it. When working with match types this feels like a must, since they often just don’t expand anymore

Would you be able to provide and example and your macro?

The problem is that sometimes targets might be conflicting and even have the same named class, so there are cases that it’s important. It might be possible to set it automatically based on the currently focused file, would that make sense in your case?

8

I think we reuse the names when using extract method code action. Do you have any specific examples that it didn’t work?

9

Should be simple to do improvement: Auto detect build target in attach if not specified by tgodzik · Pull Request #7277 · scalameta/metals · GitHub

1 Like
10

Let me start by replying to those things that I can get off the top of my head, and I’ll provide examples for the others once I play again with a scala codebase and vscode (daily work sadly no longer includes scala):

Tasty is very much not useful when working with macros, they don’t reflect close enough the tree structure as presented by the compiler in Quotes, and when working with macros I get this tree, not tasty.

I have two, but dumb as it gets:

  inline def logType[T] = ${ logTypeMacro[T] }
  private def logTypeMacro[T: quoted.Type](using q: quoted.Quotes): quoted.Expr[Unit] = {
    val simpleType = quoted.Type.show[T]
    val typeRepr = q.reflect.TypeRepr.of[T]
    q.reflect.report.warning(s"""type = $simpleType
                                |
                                |dealias = ${quoted.Type.show(using typeRepr.dealias.asType)}
                                |
                                |widen = ${quoted.Type.show(using typeRepr.widen.asType)}""".stripMargin)
    '{}
  }

  inline def typeDescrOf[T]: String = ${ typeDescrOfMacro[T] }
  private def typeDescrOfMacro[T: quoted.Type](using q: quoted.Quotes): quoted.Expr[String] = {
    import q.reflect.*
    quoted.Expr(TypeTree.of[T].show(using Printer.TreeShortCode))
  }
1 Like
11

If I understand correctly buildTarget is used to filter classes, it is opposite what we need. We need that classes from all projects is used. we may need to use “virtual project” which aggregate all subprojects. It is not obvious. Auto set can cause misunderstandings when such filtration happens implicitly

12

Even if you show tasty that only shows the results of transparent inline calls

13

Oh, it seems our friendly discourse is not letting me upload gifs to show what I mean… let’s do it the roundabout way, this gist contains the explanation with gifs, sorry I couldn’t upload here.

1 Like
14

I was trying to replicate now my issue with braces but it seems I can’t. The only thing is that it just doesn’t add spaces where the style would indicate (it just replaces the characters) but I guess this understandable. Although while trying to find that, I did hit this:

no-completion

(Also, I just realized I can cheat discourse by using gist to store the attachments and then inline the gif here :roll_eyes: )

2 Likes
15

This one should be fixed in the latest Scala 3 nightly and later in 3.7.0 and 3.3.7

1 Like
16

At the risk of sounding negative, I don’t think more functionality is what Metals needs. What’s needed is to make it more robust because it breaks down All. The. Time. Just now I did “Find all references” for a class in my codebase. It didn’t find any references outside the file that class was declared in. Yesterday I enabled inlay hints for type parameters. Some type parameters were just missing, including the one I was interested in. So I ran clean, restarted Metals eventually it showed up. This isn’t a fluke, IME it’s just what it’s like to work with Metals, and that is sad.

I realize this is anecdotal and not reproducible or actionable. But I don’t want new features, I want to be able to rely on the stuff that is already there.

8 Likes
17

We are focusing on fixes on stability (it’s progressing slowly unfortunately), this is why I mostly ask about some smaller usability improvements, which I can even do while waiting for another thing to compile or as a short break from bigger problems.

I recently did some nice improvements that took literally 5 minutes to do, so it’s sometimes worth to spend them. In contrast there has been cases that I spend half a day on some stability issues for my reproduction to turn out not to be reliable.

I think we are left with some zinc/bloop issues mostly where the recompilation is not done properly. If you encounter those cases it’s worth turning on verbose compilation to see what zinc is doing underneath.

8 Likes
18

Thank you for you work!

Yet other small improvement is to add why metals in documentation :wink:

I recently have discovered that metals is much more comfortable when working on macros.

19

If this is a small improvement - can metals/presentation-compiler try to force evaluation of match types in hovers?

20

oh, this is what I’d typically use the macro I showed above for, getting the dealiased or widened version of a match type would typically give you this (unless the expansion fails)