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

Add optIn.blankLineBeforeDocstring setting #1101

Merged
merged 2 commits into from Jan 21, 2018

Conversation

3 participants
@olafurpg
Member

olafurpg commented Jan 19, 2018

Previously, scalafmt always forced a blank line before docstrings. There
was no good reason for this, it was just a random decision I made before
v0.1 that no one has questioned until now. This behavior has in fact bugged
myself.

I think this setting is a good candidate to become default at some
point, but I'd like to receive a bit more feedback before making the
switch.

Review @alexandru

Add optIn.blankLineBeforeDocstring setting to control docstring blank…
… line.

Previously, scalafmt always forces a blank line before docstrings. There
was no good reason for this, it was just a random decision I made before
v0.1 that no one has questioned until now. This behavior has in fact bugged
myself.

I think this setting is a good candidate to become default at some
point, but I'd like to receive a bit more feedback before making the
switch.
* /** Docstring */
* def foo = 2
* }
* // after, if blankLineBeforeDocstring=true

This comment has been minimized.

@alexandru

alexandru Jan 20, 2018

Shouldn't this be after, if blankLineBeforeDocstring=false?

This comment has been minimized.

@olafurpg

olafurpg Jan 21, 2018

Member

You're right! Swapped true/false

@alexandru

This comment has been minimized.

alexandru commented Jan 20, 2018

So when can I use it? 😀

annotationNewlines: Boolean = true
annotationNewlines: Boolean = true,
// Candidate to become default true at some point.
blankLineBeforeDocstring: Boolean = false

This comment has been minimized.

@gabro

gabro Jan 20, 2018

Contributor

Maybe I'm reading it wrong, but doesn't this invert the current default?

This comment has been minimized.

@olafurpg

olafurpg Jan 21, 2018

Member

The current default forces a blank line before docstrings, no way to work around it. OptIn settings bring more control to the user over how formatting should look like.

@olafurpg

This comment has been minimized.

Member

olafurpg commented Jan 21, 2018

@alexandru It will likely be a while until the next stable Maven release, because publishing the IntelliJ plugin is super tricky. The CI publishes a pre-release to Bintray on merge http://scalameta.org/scalafmt/#Pre-release that you can use to give this a try, but the IJ plugin cannot use pre-releases

@olafurpg

This comment has been minimized.

Member

olafurpg commented Jan 21, 2018

I set the deadline for v1.5.0 at February 16th, I try to keep releases ~6 weeks apart

@olafurpg olafurpg merged commit 9687c59 into scalameta:master Jan 21, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@olafurpg olafurpg deleted the olafurpg:docstring-blank-line branch Jan 21, 2018

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