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

improvement: Try to use scala CLI format if it exists #5559

Merged
merged 4 commits into from Oct 27, 2023
Merged

Conversation

tgodzik
Copy link
Contributor

@tgodzik tgodzik commented Aug 16, 2023

Scalafmt needs to habe a config file to run formatting. Previously, metals required it to be in the main workspace directory or to be defined via configuration. Now, we will also try to use the default scalafmt config from Scala CLI

@tgodzik
Copy link
Contributor Author

tgodzik commented Aug 16, 2023

Should we run scala format by hand if the file doesn't exist yet and we are in a scalacli workspace? Though currently, we default to scala CLI, so essentially we would never create the scalafmt file unless someone used a build tools (maybe that is ok?)

@kasiaMarek
Copy link
Contributor

Should we run scala format by hand if the file doesn't exist yet and we are in a scalacli workspace?

I think it's better we stay consistent. If we want to have the scala-cli like fmt behaviour and format w/o creating .scalafmt.conf in the root we could create the configuration in .metals and use that to perform formatting no matter the build tool. Then creating .scalafmt.conf could be a different command or we could ask the user if the want to keep the configuration the first time they run formatting.

@ckipp01
Copy link
Member

ckipp01 commented Aug 17, 2023

Should we run scala format by hand if the file doesn't exist yet and we are in a scalacli workspace?

I think it's better we stay consistent. If we want to have the scala-cli like fmt behaviour and format w/o creating .scalafmt.conf in the root we could create the configuration in .metals and use that to perform formatting no matter the build tool. Then creating .scalafmt.conf could be a different command or we could ask the user if the want to keep the configuration the first time they run formatting.

Yea I'm thinking sort of along the same thing. Consistency is important here. Maybe you have, but I haven't heard complaints about Metals prompting you to create a .scalafmt.conf file unless it's in a project (👀 Dotty) that is purposefully not using scalafmt. Maybe a nice middle ground would be to do what @kasiaMarek said, but with more options. Basically if I'm in a project that doesn't have a .scalafmt.conf file (no matter the build tool) and I try to format I would get something like the following:

No .scalafmt.conf file detected. How would you like to proceed:

1. Create a .scalafmt.conf file in the root directory
2. Format with no .scalafmt.conf file
3. Not now
4. Don't show again

If they choose 1, then we do what we did before and create one in the root.
If they choose 2, then we create one in .metals/scalafmt.conf and then just use that for the workspace unless later on we find one in the root.

Copy link
Contributor

@kasiaMarek kasiaMarek 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. I only wonder if we should always fallback to .scala-cli or only if scala-cli is the build tool in use. Can it happen that .scala-build/.scalafmt.conf exists but is completely outdated since the user doesn't use scala-cli anymore? I feel like that's both unlikely and still .scala-build is a fallback for searching for config, so it's fine to use it always.

Scalafmt needs to habe a config file to run formatting. Previously, metals required it to be in the main workspace directory or to be defined via configuration. Now, we will also try to use the default scalafmt config from Scala CLI
@tgodzik
Copy link
Contributor Author

tgodzik commented Oct 26, 2023

@ckipp01 @kasiaMarek I reworked it using your suggestions. Now when the file doesn't exist we get:

No .scalafmt.conf file detected. How would you like to proceed:

  1. Create a .scalafmt.conf
  2. Run anyway
  3. Not now

I removed the last option as 4 was too much for the message request and wasn't too visible.

In case of 2 we will create .scalafmt.conf file in .metals. That will also be reported via message to the user. Then we have 3 options, first scalafmt config will the one from root, next the one from scala-build and lastly from .metals.

@tgodzik tgodzik merged commit f985a07 into main Oct 27, 2023
23 of 24 checks passed
@tgodzik tgodzik deleted the handle-scalacli branch October 27, 2023 16:17
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.

None yet

3 participants