Metals: After switching focus to another file, worksheets no longer work in non vscode editors

Created on 6 Jan 2020  路  9Comments  路  Source: scalameta/metals

Describe the bug
We'll use vim for example. If I open test.worksheet.sc, throw down some code, save, and then see the feature in action and I see the evaluations appear. Awesome. I change a couple of things, save again, and boom, new evaluations appear. I then jump into another file in a new buffer, then after a bit decide I want to test something else in test.worksheet.sc, so I go back to it. I change some stuff, save, and no updated evaluations appear :(

It looks like we are using onChangeActiveTextEditor in the vs code extension to capture the change in focus on the document, and then evaluating the worksheets and publishing the decorations. However, this is reliant on the focus being set. In other editors, this doesn't seem to be working. I've posted a gif below in both Vim and Sublime to illustrate this.

vim

sublime

To Reproduce
Steps to reproduce the behavior:

  1. Open a worksheet and evaluate
  2. Open another file and do some stuff
  3. Go back to the worksheet and try to evaluate
  4. See error

Expected behavior
I would expect to be able to go back to the worksheet and get new evaluations/decorations returned on save.

Installation:

  • Operating system: macOS
  • Editor: Vim and Sublime (I'm assuming all non VS Code editors)
  • Metals version: v0.7.6-LATEST-SNAPSHOT

Search terms
worksheets, decorations

bug

Most helpful comment

I think it鈥檚 worthwhile to fix the experience when the editor doesn鈥檛 support didDocus notifications because the functionality currently doesn鈥檛 work correctly without it.

All 9 comments

Thinking more about this, I was able to get it fixed in coc-metals, but I still worry about the other clients. I'm ok closing this if you guys agree that it should be the client's responsibility to support this, but I'm not sure.

I think it鈥檚 worthwhile to fix the experience when the editor doesn鈥檛 support didDocus notifications because the functionality currently doesn鈥檛 work correctly without it.

I can confirm, this seems to be an issue in emacs as well.

Yep, emacs now supports decoration protocol with the same problem.

It's very cool to see that Emacs supports the decoration protocol! The metals/didFocusTextDocument extension is needed to fix this issue, see https://scalameta.org/metals/docs/editors/new-editor.html#metalsdidfocustextdocument

I think we should change this guard here in Metals

https://github.com/scalameta/metals/blob/8df37b024b9674703bc311452906796dfd23e08a/metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala#L1793

to not trigger if the client doesn't support didFocusTextDocument

I was thinking about this tonight since I thought it'd be a quick fix. However, when I started looking into it, how do we actually know if an editor supports didFocusTextDocument? For example with both coc-metals and our vs code extension, we don't "register" saying we support it or have server config for it, so how would you suggest to detect if a client supports didFocusTextDocument @olafurpg?

@ckipp01 could we assume the client doesn鈥檛 support it until the first notification?

Yea that makes sense.

Is there a preferable immutable way you'd suggest to do this, or is literally just having a private var set to false and then true when the first didFocusTextDocument comes in sufficient?

private var should be fine, lsp4j doesn't make it easy to transform immutable data between requests

Was this page helpful?
0 / 5 - 0 ratings

Related issues

romanowski picture romanowski  路  4Comments

tgodzik picture tgodzik  路  4Comments

banzecrew picture banzecrew  路  3Comments

laughedelic picture laughedelic  路  4Comments

oscarvarto picture oscarvarto  路  3Comments