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

Alignment in mixed owner-type blocks #184

Closed
densh opened this Issue Apr 18, 2016 · 4 comments

Comments

Projects
None yet
2 participants
@densh
Member

densh commented Apr 18, 2016

Diff:

-    var deps       = mutable.Stack[Global](entry)
-    var defns      = mutable.UnrolledBuffer.empty[Defn]
-    val resolved   = mutable.Set.empty[Global]
+    var deps  = mutable.Stack[Global](entry)
+    var defns = mutable.UnrolledBuffer.empty[Defn]
+    val resolved = mutable.Set.empty[Global]

I'd like for code to stay the way it was. This means that grouped blocks of vals/vars and defs would benefit from being aligned by equals sign.

The similar issue applies for aligning with "extends" over a group of objects/traits and classes. For example in following code formatted by scalafmt:

object Val {
  // low-level
  final case object None  extends Val
  final case object True  extends Val
  final case object False extends Val
  final case class Zero(zeroty: nir.Type)                     extends Val
  final case class I8(value: Byte)                            extends Val
  final case class I16(value: Short)                          extends Val
  final case class I32(value: Int)                            extends Val
  final case class I64(value: Long)                           extends Val
  final case class F32(value: Float)                          extends Val
  final case class F64(value: Double)                         extends Val
  final case class Struct(name: nir.Global, values: Seq[Val]) extends Val
  final case class Array(elemty: nir.Type, values: Seq[Val])  extends Val
  final case class Chars(value: java.lang.String)             extends Val
  final case class Local(name: nir.Local, valty: nir.Type)    extends Val
  final case class Global(name: nir.Global, valty: nir.Type)  extends Val
}

I'd like extends Val to be aligned throughout the whole block for both objects and classes.

Using: 0.2.1

@olafurpg olafurpg added the alignment label Apr 18, 2016

@densh

This comment has been minimized.

Show comment
Hide comment
@densh

densh Jul 11, 2016

Member

I think I'm annoyed enough by mixed alignment of val/var-s that I would pay to get this bug fixed. Just name the price. 😉

Member

densh commented Jul 11, 2016

I think I'm annoyed enough by mixed alignment of val/var-s that I would pay to get this bug fixed. Just name the price. 😉

@olafurpg

This comment has been minimized.

Show comment
Hide comment
@olafurpg

olafurpg Jul 15, 2016

Member

We can discuss this in August ;) not sure how the configuration for it should work (or if there should be any configuration for it).

Member

olafurpg commented Jul 15, 2016

We can discuss this in August ;) not sure how the configuration for it should work (or if there should be any configuration for it).

@olafurpg olafurpg added this to the 0.2.13 milestone Aug 6, 2016

@olafurpg

This comment has been minimized.

Show comment
Hide comment
@olafurpg

olafurpg Aug 6, 2016

Member

I'm happy to hardcode the most common-sense cases, and then let #172 take care of adding more customization to how tokens align together. Are there other cases than val/var and object/class bugging you @densh ?

This can be fixed by changing a single line of code:

https://github.com/olafurpg/scalafmt/blob/master/core/src/main/scala/org/scalafmt/internal/FormatWriter.scala#L157

Member

olafurpg commented Aug 6, 2016

I'm happy to hardcode the most common-sense cases, and then let #172 take care of adding more customization to how tokens align together. Are there other cases than val/var and object/class bugging you @densh ?

This can be fixed by changing a single line of code:

https://github.com/olafurpg/scalafmt/blob/master/core/src/main/scala/org/scalafmt/internal/FormatWriter.scala#L157

@densh

This comment has been minimized.

Show comment
Hide comment
@densh

densh Aug 6, 2016

Member

I'd say val/var/def and class/trait/object are the two major groups I'd want to see aligned together.

Member

densh commented Aug 6, 2016

I'd say val/var/def and class/trait/object are the two major groups I'd want to see aligned together.

olafurpg added a commit that referenced this issue Aug 6, 2016

Allow limited mixed owner alignment, fixes #184
Once #172 is implemented, we can add more powerful configuration
for which tokens can align together.

@olafurpg olafurpg self-assigned this Aug 6, 2016

@olafurpg olafurpg added the in progress label Aug 6, 2016

@olafurpg olafurpg closed this in 1322a77 Aug 6, 2016

olafurpg added a commit that referenced this issue Aug 6, 2016

Merge pull request #389 from olafurpg/184
Allow limited mixed owner alignment, fixes #184

@olafurpg olafurpg removed the in progress label Aug 6, 2016

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