Skip to content

Forward ScalaDiagnostic#7264

Merged
eed3si9n merged 1 commit intosbt:1.9.xfrom
eed3si9n:wip/diagnostic
May 26, 2023
Merged

Forward ScalaDiagnostic#7264
eed3si9n merged 1 commit intosbt:1.9.xfrom
eed3si9n:wip/diagnostic

Conversation

@eed3si9n
Copy link
Member

@eed3si9n eed3si9n commented May 22, 2023

See also BSP spec update build-server-protocol/build-server-protocol#527

Problem
Zinc added actions in Problem, but it's not yet forwarded to BSP clients.

Solution
As discussed in
https://contributors.scala-lang.org/t/roadmap-for-actionable-diagnostics/6172, the plan seems to be to use the data field with actions inside it, so this implements that.

**Problem**
Zinc added actions in Problem, but it's not yet forwarded to BSP
clients.

**Solution**
As discussed in
https://contributors.scala-lang.org/t/roadmap-for-actionable-diagnostics/6172,
the plan seems to be to use the `data` field with `actions` inside it,
so this implements that.

## A Scala action represents a change that can be performed in code.
## See also LSP: Code Action Request (https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_codeAction).
type ScalaAction {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As pointed out by @ckipp01 in build-server-protocol/build-server-protocol#527 (review) this is a duplication of xsbti.Action. I wonder if it would not be simpler to define the xbsti.Action directly as a contra instead. A counter argument is that maybe we don't want util-interface to depend on sjsonnew?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

util-interface and compiler-interface needs to be in Java, but I guess we can make a second subproject for sjsonnew codecs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if it would be better or worse though. I am fine with the PR as it is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants