zed-industries / zed

Code at the speed of thought – Zed is a high-performance, multiplayer code editor from the creators of Atom and Tree-sitter.
https://zed.dev
Other
48.13k stars 2.84k forks source link

Scala support broken in v0.156.x #18636

Open filipwiech opened 6 days ago

filipwiech commented 6 days ago

Check for existing issues

Describe the bug / provide steps to reproduce it

After PR #17756 support for the Scala LSP became broken: server is no longer notified about saved files.

@osiewicz I have included more details as a comment in you pull request: https://github.com/zed-industries/zed/pull/17756#issuecomment-2387124084 :+1:

Environment

Zed: v0.156.0 (Zed Preview) OS: Linux Wayland ubuntu 24.04 Memory: 31 GiB Architecture: x86_64 GPU: Intel(R) UHD Graphics (CML GT2) || Intel open-source Mesa driver || Mesa 24.0.9-0ubuntu0.1

If applicable, add mockups / screenshots to help explain present your vision of the feature

No response

If applicable, attach your Zed.log file to this issue.

Zed.log


osiewicz commented 6 days ago

Uh oh. Based on your comment I do believe that Metals is registering for scala files too? Aren't the following globs matching Scala source files?

   {
      "globPattern": {
        "left": "file:///to/workspace/*.sc"
      }
    },
    {
      "globPattern": {
        "left": "file:///to/workspace/project/*.{scala,sbt}"
      }
    },

If so, the problem seems to be that these patterns should be root/**/*.{scala,sbt} and not root/*.{scala,sbt}, as the latter are not recursive.

I don't think we should be bringing back the old behaviour, as overnotifying breaks RA. Gotta pick our poison. :P

filipwiech commented 6 days ago

Unfortunately all the patterns are about the "special" definition files only, which describe the project structure and settings for different Scala build tools (like SBT, Mill, etc.). They don't cover the normal source code. I imagine that probably Metals could (should?) register for these as well. :thinking: Will try to create an issue there and get back to you. :+1:

osiewicz commented 6 days ago

Yeah well, I imagine it should be as "easy" as including that /**/ bit in these globs.