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

Fix whitespace. #146

Merged
merged 2 commits into from
Feb 23, 2016
Merged

Conversation

jkinkead
Copy link
Collaborator

This fixes a long-standing bug in how comments get formatted if you have the suggested Scaladoc style enabled (both MultilineScaladocCommentsStartOnFirstLine and PlaceScaladocAsterisksBeneathSecondAsterisk set to true). Note the unit test commit, which fails on master.

This manifested if you had > 1 space leading a scaladoc comment. The worst part was that the formatting would only remove one space at a time - meaning each subsequence scalariform run deleted one additional space.

@kiritsuku
Copy link
Member

LGTM (I'm probably going to merge soon but first I want to check how long it takes to run PRs against projects that use scalariform)

@kiritsuku
Copy link
Member

Please mention in the commit message that it fixes #149

@ghprb-bot
Copy link

Refer to this link for build results (access rights to CI server needed):
https://jenkins.scala-ide.org:8496/jenkins//job/ghprb-scalariform-sbt/8/

…tLine and PlaceScaladocAsterisksBeneathSecondAsterisk are true.

Fixes scala-ide#147 .
@jkinkead
Copy link
Collaborator Author

Updated commit message - although this fixes #147 :)

@ghprb-bot
Copy link

Refer to this link for build results (access rights to CI server needed):
https://jenkins.scala-ide.org:8496/jenkins//job/ghprb-scalariform-sbt/9/

@kiritsuku
Copy link
Member

Ok, thanks.

@ghprb-bot
Copy link

Refer to this link for build results (access rights to CI server needed):
https://jenkins.scala-ide.org:8496/jenkins//job/ghprb-scalariform/23/
Test PASSed.

@jkinkead
Copy link
Collaborator Author

Will this be merged soon? It's one of the most annoying bugs we've run into when using Scalariform, and I'd love it if the next release had it fixed.

@fommil
Copy link
Contributor

fommil commented Feb 23, 2016

The blockers at the moment are all regression testing related. If you'd like to speed up the next release, have a look in Milestone 1.0 and see if you can help.

Critical is that new features don't change old behaviour. The tickets contain more discussion.

@jkinkead
Copy link
Collaborator Author

I was wondering about blockers for merging this - but I'm happy to look at blockers for the release, too!

@fommil
Copy link
Contributor

fommil commented Feb 23, 2016

I shall refer all merging to @sschaef

@jkinkead
Copy link
Collaborator Author

Awesome, thank you!

I'll keep a watch on the milestone tag for areas I can contribute.

@kiritsuku
Copy link
Member

The changes this issue fixes are minor, anyone can easily solve them in their repo. Therefore I'm merging then I don't have to care about it anymore. We are still not near a new release but the snapshots can be consumed, they are hosted on sonatype (but only the one for scalariform, the scalariform-sbt plugin still is not published as a snapshot).

kiritsuku added a commit that referenced this pull request Feb 23, 2016
@kiritsuku kiritsuku merged commit 5f60d50 into scala-ide:master Feb 23, 2016
@jkinkead jkinkead deleted the fix_leading_whitespace branch March 2, 2016 20:29
@jkinkead jkinkead mentioned this pull request Mar 7, 2016
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

4 participants