Show implicit decorations in worksheets #3582
Conversation
Previously, we would have a separate type of decorations for worksheets. Now, we have a separate type for worksheets evaluations and synthetic decorations. Needed for scalameta/metals#3582
tgodzik
force-pushed
the
worksheet-decorations
branch
from
ce6d4bc
to
ecae315
Compare
metals/src/main/scala/scala/meta/internal/worksheets/DecorationWorksheetPublisher.scala
Outdated
Show resolved
Hide resolved
tgodzik
added
the
affects clients
tgodzik
force-pushed
the
worksheet-decorations
branch
from
ecae315
to
ec17a36
Compare
Previously, we would not show implicit decorations in worksheet files, because they could get overriden by the worksheet decorations. Now, we send two different types of decorations, inline and not inline, which makes sure that one will not override the other. This got tested in VS Code, but we would also need a confirmation on how to make it work in Sublime text.
tgodzik
force-pushed
the
worksheet-decorations
branch
from
ec17a36
to
8890408
Compare
metals/src/main/scala/scala/meta/internal/decorations/SyntheticsDecorationProvider.scala
Outdated
Show resolved
Hide resolved
| @@ -48,7 +49,9 @@ class SemanticdbTextDocumentProvider( | |||
| } else { | |||
| None | |||
| } | |||
| val document = unit.toTextDocument(explicitDialect) | |||
| // we recalculate md5, since there seems to be issue with newlines sometimes | |||
There was a problem hiding this comment.
What problems were caused by this newlines problem?
There was a problem hiding this comment.
Different md5 results produced by default when no newline was at the end. Not sure exactly why.
There was a problem hiding this comment.
It makes sense that inserting a character changes the document hash, but I still don't see when the problem starts. When is this newline inserted? unit.toTextDocument has md5 = unit.source.toMD5, so it indicates that file content is changed after compilation unit is created. Is this vscode problematic behavior?
There was a problem hiding this comment.
When is this newline inserted?
I don't know :D That is the reason, and I am not even sure if it's a newline. Just that on the same text we were calculating two different md5 results.
There was a problem hiding this comment.
Do you have steps for reproduction? It bothers me a lot :c
There was a problem hiding this comment.
When I create worksheet with last statement without a newline this started popping up. It's not strickly related tot his PR so I can revert the change
There was a problem hiding this comment.
It's okay, there is no need for reverting. I just want to investigate it ;)
| if (decorationTypes == null) { | ||
| Set(params) | ||
| } else { | ||
| decorationTypes.filter(p => p.isInline != params.isInline) + params |
There was a problem hiding this comment.
Why are you filtering decorations?
There was a problem hiding this comment.
To replace the particular inline type. If isInline=true we will only replace decorations with inline=true and vice versa
Co-authored-by: Kamil Podsiadło <37124721+kpodsiad@users.noreply.github.com>
|
Pretty nice :D I understand there's no hover yet? |
There should be, though it also shouldn't yet work :D We also need the VS Code PR |
|
@tgodzik How is the editor supposed to treat the new |
It's just an information for the editor so that we don't override worksheet decorations with synthetic ones and vice versa. They are being sent at a different time. |
Previously, we would not show implicit decorations in worksheet files, because they could get overriden by the worksheet decorations. Now, we send two different types of decorations, inline and not inline, which makes sure that one will not override the other.
This got tested in VS Code, but we would also need a confirmation on how to make it work in Sublime text.
Fixes #3581