Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ScalafmtDynamic: use dependency injection #2925

Merged
merged 1 commit into from
Nov 26, 2021
Merged

Conversation

kitbellew
Copy link
Collaborator

No description provided.

Copy link
Contributor

@tgodzik tgodzik left a comment

Choose a reason for hiding this comment

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

Looks good, but could you add a bit of background on why it's needed?

@kitbellew
Copy link
Collaborator Author

@tgodzik initially, attempted to follow @unkarjedy suggestion, made in #2919, to pave the way for intellij to use scalafmt-dynamic.

one small part of this commit (extracted dependencies) has helped achieve that goal, but i found that intellij uses a vastly different approach to referring to files (uses virtual files which might not be represented by an nio.Path), which for now limits the ability to adopt the rest.

wonder how scalafmt is invoked in metals, could you help me find that, @tgodzik?

@tgodzik
Copy link
Contributor

tgodzik commented Nov 26, 2021

wonder how scalafmt is invoked in metals, could you help me find that, @tgodzik?

It's being done here: https://github.com/scalameta/metals/blob/main/metals/src/main/scala/scala/meta/internal/metals/FormattingProvider.scala#L551

Split both ScalafmtDynamic and ScalafmtDynamicDownloader into traits and
implementations:
- ScalafmtDynamicDownloader: replaced with:
  - Dependency: exposes dependencies, doesn't force coursier
  - DependencyDownloader: downloads dependencies, has coursier implementation
- ScalafmtDynamic: split into below types, some injected as dependencies
  - ScalafmtProperties: contains all properties set via Scalafmt interface
  - ScalafmtModuleLoader: loads ScalafmtReflect, uses cache and dependency downloader
  - ScalafmtConfigLoader: loads ScalafmtReflectConfig, uses cache and module loader
  - ScalafmtDynamicSession: implements ScalafmtSession, uses properties and reflect config

This will allow more complex clients (such as Intellij scala plugin) to
remove some legacy custom code and use scalafmt-dynamic better.
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