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

Default formatting produces too much code for arrays of literals #347

Closed
densh opened this Issue Jun 27, 2016 · 4 comments

Comments

Projects
None yet
2 participants
@densh
Member

densh commented Jun 27, 2016

Here a few examples of Scala Native codebase where the quality of formatted code leaves a lot to be desired size-wise:

https://github.com/scala-native/scala-native/blob/master/javalib/src/main/scala/java/lang/Character.scala#L547

https://github.com/siriux/scala-native/blob/7ea66ec1afebf5df19ae5896bbe462f8f96ea53f/javalib/src/main/scala/niocharset/UTF_8.scala#L42

Using:

  • 0.2.5
  • --style defaultWithAlign --javaDocs
@densh

This comment has been minimized.

Show comment
Hide comment
@densh

densh Jun 27, 2016

Member

If it's array of primitive numbers or chars, it should just bin-pack it by default in my opinion. Having one literal per line is a massive overkill.

Member

densh commented Jun 27, 2016

If it's array of primitive numbers or chars, it should just bin-pack it by default in my opinion. Having one literal per line is a massive overkill.

@olafurpg

This comment has been minimized.

Show comment
Hide comment
@olafurpg

olafurpg Jun 27, 2016

Member

I agree that the output for Arrays.scala does not help readability and it would be great if scalafmt could automatically detect these situations to produce better formatting output.

If it's array of primitive numbers or chars, it should just bin-pack it by default in my opinion

That's an interesting idea, but in terms of detecting when bin-packing should be used I think that rule will produce too many false positives and negatives. IMO, for now, Arrays.scala is a case where scalafmt should be disabled. Maybe with #315, instead of disabling scalafmt you will be able to enable bin-packing for that file only.

Member

olafurpg commented Jun 27, 2016

I agree that the output for Arrays.scala does not help readability and it would be great if scalafmt could automatically detect these situations to produce better formatting output.

If it's array of primitive numbers or chars, it should just bin-pack it by default in my opinion

That's an interesting idea, but in terms of detecting when bin-packing should be used I think that rule will produce too many false positives and negatives. IMO, for now, Arrays.scala is a case where scalafmt should be disabled. Maybe with #315, instead of disabling scalafmt you will be able to enable bin-packing for that file only.

@densh

This comment has been minimized.

Show comment
Hide comment
@densh

densh Jun 27, 2016

Member

These two are normal human-maintained code, they are not generated in any way.

Member

densh commented Jun 27, 2016

These two are normal human-maintained code, they are not generated in any way.

@densh

This comment has been minimized.

Show comment
Hide comment
@densh

densh Jun 28, 2016

Member

#315 is good enough fix for this issue.

Member

densh commented Jun 28, 2016

#315 is good enough fix for this issue.

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