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

Automated formatting #510

Open
cquiroz opened this issue Apr 25, 2018 · 14 comments
Open

Automated formatting #510

cquiroz opened this issue Apr 25, 2018 · 14 comments

Comments

@cquiroz
Copy link
Contributor

cquiroz commented Apr 25, 2018

This is a place to discuss formatting. I've been using scalafmt but not very systematically. It is not integrated into the build and I've used it only for new code.
I'm using the format rules selected for http4s with the only difference that I'm doing vertical alignment as much as possible.

style = default

maxColumn = 100

align = most // In http4s they use align = none

rewrite.rules = [
  AvoidInfix
  RedundantBraces
  RedundantParens
  AsciiSortImports
  PreferCurlyFors
]

What do you think?
/cc @tpolecat @swalker2m @sraaphorst @jluhrs @jdnavarro

Some links:
scalafmt
http4s config

@tpolecat
Copy link
Member

I messed around with it and don't like the results, but I won't put up a fight.

@cquiroz
Copy link
Contributor Author

cquiroz commented Apr 26, 2018

Are you unhappy with the concept of formatting with scalafmt or with the given formatting? Could it be improved by a better config?

@sraaphorst
Copy link
Contributor

When you say vertical alignment, do you mean that spacing is preserved so that, for example, cases in a match have their arrows line up?

I tried scalafmt with the default settings yesterday on some personal code and wasn't pleased with the results, but maybe with your config I would like it better.

@tpolecat
Copy link
Member

I'm playing around with the config but the interactions between settings are unpredictable.

@swalker2m
Copy link
Contributor

Auto-formatting seems like a great idea, if we can get it to produce reasonable results. My view is that we stick with the Scala guide as much as possible. It's arbitrary, but since everyone has their own preferences it gives us a way to resolve disagreements.

Two items that have come up where we might want to differ from the guide are comment style and line length. I have a slight preference for the Scala doc style (option 2):

Scaladoc style, with gutter asterisks aligned in column three:

/** Provides a service as described.
  *
  * This is further documentation of what we're documenting.
  * Here are more details about how it works and what it does.
  */
def member: Unit = ()

but the Javadoc style that Rob prefers is also fine. Does anybody else have a preference? I'll switch to Javadoc style if not.

For line length, going much over 80 will be a hardship for me since I have to magnify the text and like to see two source files side-by-side. As long as auto-formatting won't lengthen 80 character lines that I write I can just deal with the horizontal scrolling induced by other people using longer lines.

@sraaphorst
Copy link
Contributor

I switched to the asterisk commenting style for Scala code after this was brought to my attention (I believe by Shane some time ago) and now only use the Javadoc style when writing Java code.

I'll do my best to keep my lines to 80 characters as I've definitely been guilty of going well over that, and often have stuck to the 100 vertical line that IDEA suggests by default. (This must be configurable.)

Other than that, I have no real strong feelings, except that I despise having to write short functions on 2+ lines, and use curly braces for single line statements, but it seems that, according to the Scala guide, writing short functions on one line is acceptable, and there are no recommendations I saw on doing a quick scan about braces. (scalafmt definitely, IIRC, with the default settings cut all functions to 2+ lines and added braces.)

@jdnavarro
Copy link
Contributor

👍 For 80 column length. That way I can have 2 editor panels open side by side, but if you prefer any other length I'm alright with it.

I'm fine with whatever style as long as the rules are clear and consistent.

If there is no easy way to integrate the auto-formatter in the build, we could always check in a git hook, maybe a pre-commit hook, so that we don't ever forget formatting.

@sraaphorst
Copy link
Contributor

For anyone using IDEA (if anyone else still is except me), Preferences -> Editor -> Code Style lets you change the line length vertical bar from 120 (I thought it was 100) to 80.

@tpolecat
Copy link
Member

We need to do 80 as our line length to accommodate Shane's vision issue, so that's fine. I have some suggestions for the other settings that I'll paste here once I figure them out a little more. Since we're going to be wrapping stuff a lot more we need to be sure we end up with something readable.

@cquiroz
Copy link
Contributor Author

cquiroz commented Apr 27, 2018

80's length is ok. i'm not sure about the original reason for 100 in http4s.

@tpolecat
Copy link
Member

GH listings show a little over 100 before you have to scroll horizontally, so that's why it's my preference.

@sraaphorst
Copy link
Contributor

Did we ever decide between Javadoc style comments and the scala-style comments?

I've been doing the scala-style because that's the default in IntelliJ IDEA Pro, but I'll switch if everyone else has.

@rpiaggio
Copy link
Contributor

Hi everyone! Let me revive this thread.

As a heavy user of auto formatting, I'm catching myself being extra careful not to introduce unnecessary changes to code by the auto format, which kind of defeats the purpose.

I think it would be great to have a common formatting style so that we can use auto format and not worry about the formatting details while coding. I don't really care much about what style we settle on. I'd be glad to help come up with a .scalafmt.conf (or adapt http4s's).

I've already set line length to 80 in IntelliJ.

@tpolecat
Copy link
Member

Line length of 80 is an accommodation for @swalker2m … is this still an issue since you got your eye fixed? I prefer 100 as above, if you're able to see it.

I am going to hate this and complain about it forever. Just warning you all.

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

No branches or pull requests

6 participants