SIP-27 Trailing Commas #533

Merged
merged 9 commits into from Sep 23, 2016

Projects

None yet
@dwijnand
Member
dwijnand commented Jun 27, 2016 edited

This is my first SIP. I'm happy to take feedback and iterate where needed.

An initial implementation has also been proposed in scala/scala#5245.

Link to Scala grammar breakdown: #533 (comment)

Link to proposing trailing commas only for multi-line: #533 (comment)


Summary of subjective pros and cons (added by @cvogt):

Pros:

  • avoids problems with last parameter/argument
  • eases commenting out
  • eases swapping
  • avoids merge conflicts
  • less reliance on tool support or manual labor

Cons:

  • one more terminating grammar rule per construct
  • room for accidental correctness bug def foo(a: Int, b: Int = 0) = 0; foo(1,)
  • harms readability (are there many people who feel this way?)
@olafurpg olafurpg and 2 others commented on an outdated diff Jun 27, 2016
sips/pending/_posts/2016-06-25-trailing-commas.md
+The elements that have are not changed are:
+
+* ValDcl, VarDcl, VarDef via ids
+* Type via FunctionArgTypes
+* SimpleType, TypeArgs via Types
+* Expr, ResultExpr via Bindings
+* SimplePattern via Patterns
+* TypeParamClause, FunTypeParamClause
+* ImportExp
+* PatDef
+
+## Implementation
+
+The implementation is a simple change to the parser, allowing for a trailing comma, for the groups detailed above, and has been proposed in [scala/scala#5245][].
+
+## Drawbacks
@olafurpg
olafurpg Jun 27, 2016

Might be worth to mention that tooling/parsers will have to catch up. I can think of intellij-scala, scalariform, scala.meta and scalaparse.

(I would love to have trailing commas so big +1 from me.)

@dwijnand
dwijnand Jun 27, 2016 Member

By scalaparse do you mean https://github.com/lihaoyi/fastparse/tree/master/scalaparse?

Btw it was my intent to submit changes to those as well.

@olafurpg
olafurpg Jun 27, 2016

Yes, I meant fastparse/scalaparse. My only comment was to include this as a drawback, since updating those tools will require some work.

@dwijnand
dwijnand Jun 27, 2016 Member

@olafurpg I didn't add it because I thought it might be true for all SIPs, but then maybe not. Added, thanks.

@paulp
paulp Jun 27, 2016 Contributor

You should mention that it massively simplifies generating scala source code.

@artempyanykh artempyanykh and 1 other commented on an outdated diff Jun 28, 2016
sips/pending/_posts/2016-06-25-trailing-commas.md
+## Design Decisions
+
+There are a number of different elements of the Scala syntax that are comma separated, but instead of changing them all a subset of the more useful ones was chosen:
+
+* tuples
+* argument and parameter groups, including for implicits, for functions, methods and constructors
+* import selectors
+
+From the spec these are:
+
+* SimpleExpr1, ArgumentExprs via Exprs
+* ParamClause, ParamClauses via Params
+* ClassParamClause, ClassParamClauses via ClassParams
+* ImportSelector
+
+The elements that have are not changed are:
@artempyanykh
artempyanykh Jun 28, 2016

Seems like there's an extra are after have.

@dwijnand
dwijnand Jun 28, 2016 Member

You're right. Done, thanks.

@xeno-by
Member
xeno-by commented Jul 15, 2016

Hello @dwijnand!

Welcome to the new edition of the SIP process. My name's Eugene, and I'm going to be your reviewer. This means that I will be the contact person between you and the SIP committee, outlining your progress and providing you with our feedback.

During the SIP meeting on 13 Jul 2016, I presented your proposal to the committee according to the "Formal presentation" phase described in the new rules (http://docs.scala-lang.org/sips/sip-submission.html). At this phase, we are supposed to decide whether following through your SIP is a good idea in principle.

I have good news. We decided that your proposal has the potential to move Scala in the right direction, so we'd like to clear the "Formal presentation" phase and move the proposal to "Formal evaluation". We will now merge this pull request as SIP-27 and will proceed with evaluation.

During the "Formal evaluation" phase, we will work with you to make sure that the proposal reaches its maximum potential. When we're sure that we have a comprehensive understanding of your proposal and its consequences on Scala, we will have a final vote and will announce our verdict.

As the first step of the evaluation process, we would like to ask you to address the following feedback:

  • Update your implementation to cover all cases in the Scala grammar that have comma-separated lists. You can follow Denys Shabalin's suggestions from his comment on your pull request to the Scala compiler for an exhaustive list of such cases.
  • Update your write-up to state and analyze potential corner cases that will be created by the proposal. During the meeting, the committee discussed one of such corner cases: (x,). We would like to make sure that we understand the potential caveats completely.

Thank you for contributing to the future of Scala. Looking forward to hearing from you again!

@dwijnand
Member
dwijnand commented Jul 15, 2016 edited

Hey @xeno-by, that's excellent news!

I'll spend more time on this soon, but I have a quick question: should I really jump to updating my implementation for all cases? Because I think there are some parts of the grammar where trailing commas don't really make sense nor are common enough to be worth doing it anyways, such as val a, b, = 1.

@xeno-by
Member
xeno-by commented Jul 15, 2016

Thank you for your swift reply! Please also ask further questions in this thread - I believe that open discussion greatly benefits language evolution.

What I meant is that I think it's a good idea to explore all cases, both theoretically and practically. As you rightfully pointed out, there may be cases where trailing commas aren't worth it. Please feel free to ignore these cases in your implementation. But in any case do document your findings - I'm confident that they will be of help to our subsequent discussions.

@dwijnand dwijnand changed the title from SIP - Trailing Commas to SIP-27 Trailing Commas Jul 15, 2016
@jvican
Member
jvican commented Jul 22, 2016 edited

Hello @dwijnand, the design and implementation of this SIP will be discussed in the next SIP meeting on 10th August. I encourage you to provide more insights into the possible corner cases of your implementation and how you would handle them. It would be good to update this document to reflect those and explain all the interactions with existing language features. This will surely speed up the proceess and improve the discussions in our meeting, making them more productive. For more information, please get in touch with your reviewer @xeno-by.

@dwijnand
Member

I didn't realise it was so soon, but, yes, I'll make sure to respond to the initial round of feedback, such that this SIP can (hopefully) progress next meeting.

Wasn't this PR meant to be merged if it's provisionally accepted as is, and then go through some finessing and final decision?

@jvican
Member
jvican commented Jul 22, 2016

Yes Dale, that's the plan, I'm planning to merge it soon. Please, note that even if this PR is merged, all the feedback will still go into this PR, so that all the discussion is centralized in the same place.

@dwijnand
Member
dwijnand commented Aug 5, 2016

I've studied the relevant parts of the Scala grammar with regards to adding trailing commas (breakdown and my thoughts below).

Summary (TL;DR)

In my opinion not all comma-separated elements of the grammar make sense to support trailing commas, but they could, for across-the-board consistency. I leave it to the discretion of the SIP committee to let me know what they think.

I focussed particularly on the interaction between trailing commas and Tuple1. For context the issue is that Scala allows both individual terms and individual types to be surrounded by optional parentheses which causes ambiguity for the Tuple1 case, ambiguity that can be removed by using a trailing comma to signal a Tuple1.

My proposal for these ambiguous cases is to follow @sjrd's advice and make them not compile, like they don't currently. The alternative is make it syntax for Tuple1. I leave it to the discretion to SIP committee to let me know which alternative is preferred.

Once the SIP committee responds to these notes I'll incorporate them into the SIP document.

Breakdown

SimpleExpr1

spec

      Exprs ::= Expr {‘,’ Expr}
SimpleExpr1 ::= ‘(’ [Exprs] ‘)’

examples

(1)
(1,)
(1, 2)
(1, 2,)

Thoughts

As highlighted in the SIP meeting, there is an edge-case here with regards to Tuple1. Currently an expression like (1,) doesn't compile, and a naive implementation of adding trailing commas would make it compile as an Int by simply ignoring the trailing comma. But (1,) would be very convenient syntax for Tuple1 (indeed it's the way the REPL prints Tuple1 and it's the syntax used in Python).

Having a syntax for Tuple1 is of increasing importance as there is a desire to be able to abstract over tuples/hlist-like tuples in a future version of Scala.

I propose we allow trailing commas in expressions, but, following @sjrd's advice, we make expressions like (1,) not compile, like it does today, leaving it available for Tuple1 in the future.

The alternative is to change Scala such that (1,) is the syntax for a Tuple1, but perhaps this is beyond the scope of this SIP.

ArgumentExprs

spec

        Exprs ::= Expr {‘,’ Expr}
ArgumentExprs ::= ‘(’ [Exprs] ‘)’

examples

Seq(1)
Seq(1,)
Seq(1, 2)
Seq(1, 2,)

Thoughts

During the SIP meeting @odersky wondered if a trailing comma should be allowed here, because to avoid auto-tupling ambiguity perhaps a trailing comma could differentiate between a single argument hlist-like tuple and multiple arguments.

Personally I think trailing commas should be optionally available for both argument lists and hlist-like tuples, with clarifying parenteses for the latter, as is needed currently to avoid the warning under -Xlint.

Params and ClassParams

spec

     Params ::= Param {‘,’ Param}
ClassParams ::= ClassParam {‘,’ ClassParam}

examples

def foo(a: Int)
def foo(a: Int,)
def foo(a: Int, b: Int)
def foo(a: Int, b: Int,)

class Foo(a: Int)
class Foo(a: Int,)
class Foo(a: Int, b: Int)
class Foo(a: Int, b: Int,)

There is no concern here about Tuple1.

ImportSelectors

spec

ImportSelectors ::= ‘{’ {ImportSelector ‘,’} (ImportSelector | ‘_’) ‘}’

examples

import foo.{ Bippy }
import foo.{ Bippy, }
import foo.{ Bippy, Dingo }
import foo.{ Bippy, Dingo, }

There is no concern here about Tuple1.

ids, ValDcl, VarDcl, VarDef and PatDef

spec

   ids ::= id {‘,’ id}
ValDcl ::= ids ‘:’ Type
VarDcl ::= ids ‘:’ Type
VarDef ::= ids ‘:’ Type ‘=’ ‘_’
PatDef ::= Pattern2 {‘,’ Pattern2} [‘:’ Type] ‘=’ Expr

examples

val a = 1
val a, = 1
val a, b = 1
val a, b, = 1

var a = 1
var a, = 1
var a, b = 1
var a, b, = 1

var a = _
var a, = _
var a, b = _
var a, b, = _

var Foo(a) = bippy
var Foo(a), = bippy
var Foo(a), Bar(b) = bippy
var Foo(a), Bar(b), = bippy

Thoughts

There is no concern here about Tuple1, but I don't think that a trailing comma would used much here, and so I didn't include it originally in my proposal. But for consistency it could be added.

TypeArgs, TypeParamClause and FunTypeParamClause

spec

             Types ::= Type {‘,’ Type}
          TypeArgs ::= ‘[’ Types ‘]’

  VariantTypeParam ::= {Annotation} [‘+’ | ‘-’] TypeParam
   TypeParamClause ::= ‘[’ VariantTypeParam {‘,’ VariantTypeParam} ‘]’
FunTypeParamClause ::= ‘[’ TypeParam {‘,’ TypeParam} ‘]’

examples

Foo[A]
Foo[A,]
Foo[A, B]
Foo[A, B,]

def foo[A]
def foo[A,]
def foo[A, B]
def foo[A, B,]

Thoughts

There is no concern here about Tuple1, and I think here it makes more sense to allow optional trailing commas, as sometimes the types in a type argument list can be very long, so I would be happy to add support here.

SimpleType and FunctionArgTypes

spec

           Types ::= Type {‘,’ Type}
       ParamType ::= Type | (‘=>’ Type) | (Type ‘*’)
      SimpleType ::= ‘(’ Types ‘)’
FunctionArgTypes ::= ‘(’ [ ParamType {‘,’ ParamType } ] ‘)’

examples

(Int)
(Int,)
(Int, Int)
(Int, Int,)

def m(f: (Int) => Int)
def m(f: (Int,) => Int)
def m(f: (Int, Int) => Int)
def m(f: (Int, Int,) => Int)

Thoughts

The story here is similar to SimpleExpr1: problematic with regards to Tuple1. This is because (Int) means Int and (Int) => Int means Int => Int, where (Int,) and (Int,) => Int would be convenient syntax for the types Tuple1[Int] and Function1[Tuple1[Int], Int].

My proposal here matches my proposal for SimpleExpr1: allow trailing commas but make the Tuple1 cases compilation failures.

Bindings

spec

   Binding ::= (id | ‘_’) [‘:’ Type]
  Bindings ::= ‘(’ Binding {‘,’ Binding} ‘)’
      Expr ::= Bindings ‘=>’ Expr
ResultExpr ::= Bindings ‘=>’ Block

examples

f((x) => 1)
f((x,) => 1)
f((x, y) => 1)
f((x, y,) => 1)

Thoughts

With "funciton arity adaptation" (from @odersky's slides for both Scala Days keynotes) there is also concern here with Tuple1. However I don't know how common, and therefore necessary, trailing commas are here, so I wouldn't change anything. But for consistency it could be added, with the usual exclusing for (x,).

SimplePattern

spec

     Patterns ::= (Pattern [‘,’ Patterns]) | (‘_’ *)
SimplePattern ::= [StableId] ‘(’ [Patterns] ‘)’

examples

case (1)
case (1,)
case (1, 2)
case (1, 2,)

case Seq(1)
case Seq(1,)
case Seq(1, 2)
case Seq(1, 2,)

Thoughts

For the case where StableId is missing, ie. the tuple syntax, again there is ambiguity for the 1-arity case. Again I propose we just make it a compilation failure.

Import

spec

Import ::= ‘import’ ImportExpr {‘,’ ImportExpr}

examples

import foo._
import foo._,
import foo._, Bippy._
import foo._, Bippy._,

Thoughts

There is no concern here, but I really don't think it's necessary to add trailing comma support here, except for consistency.

Final notes

@xeno-by Let me know if there's anything else you need for me for the next SIP meeting. Sorry for leaving it so late.

@jvican
Member
jvican commented Aug 5, 2016

Thanks for the thorough explanation Dale. As a next step, it would be good to add this to the SIP document. We'll discuss this in the next meeting.

@SethTisue SethTisue commented on an outdated diff Aug 10, 2016
sips/pending/_posts/2016-06-25-trailing-commas.md
+{% endhighlight %}
+
+Additionally, this means that there is a lot of noise in diffs, such as:
+
+{% highlight diff %}
+@@ -4,7 +4,8 @@
+ Map(
+ foo,
+ bar,
+- baz
++ baz,
++ quux
+ )
+{% endhighlight %}
+
+Also such feature would massively simplify generating Scala source code.
@SethTisue
SethTisue Aug 10, 2016 Member

s/massively/modestly/ IMO — "massively" just seems like hyperbole. or is it really that big an issue in your experience? (I do have some experience writing code generators that omitted trailing commas and found it annoying, but really not that a big deal, either.)

@SethTisue SethTisue and 3 others commented on an outdated diff Aug 10, 2016
sips/pending/_posts/2016-06-25-trailing-commas.md
+
+## Alternatives
+
+As an alternative, trailing commas support could be added universally to all the comma-separated elements of the syntax. This would mean changing more (but still only in the parser), but it would make it consistent.
+
+As an alternative to changing the language, there already exists today a compiler plugin called [scala-commas][] that provides this feature. It also provides evidence that people would even use unsupported compiler apis and reflection to add this functionality, even when such a plugin won't compose with other plugins well.
+
+## References
+
+1. [SI-4986][]
+2. [scala/scala#5245][]
+3. [scala-commas][]
+
+[SI-4986]: https://issues.scala-lang.org/browse/SI-4986
+[scala/scala#5245]: https://github.com/scala/scala/pull/524://github.com/scala/scala/pull/5245
+[scala-commas]: https://github.com/andyscott/scala-commas
@SethTisue
SethTisue Aug 10, 2016 Member

is https://github.com/47deg/scala-commas a better URL for this? it has 27 stars, the andyscott one only has 1 star.

@odersky
odersky Aug 10, 2016 Contributor

Here's a quick reaction. I think the proposal goes too far in that it allows trailing commas in lots of contexts which IMO do not make sense.

I propose we go slowly and only accept trailing commas for tuples right now and we include Tuple1. Anything else might risk overshooting.

The danger I see is that there is a good prospect that trailing commas will have a semantic meaning connected with Hlists. But if we go overboard now and accept them everywhere we give up the opportunity to establish a nice correspondence between syntax and semantics.

We can come back to the issue of generalizing trailing commas once Hlists are implemented. But Hlists should come first, as they are the truly important part.

Martin

Sent from my iPhone

On 10.08.2016, at 09:16, Seth Tisue notifications@github.com wrote:

In sips/pending/_posts/2016-06-25-trailing-commas.md:

+## Alternatives
+
+As an alternative, trailing commas support could be added universally to all the comma-separated elements of the syntax. This would mean changing more (but still only in the parser), but it would make it consistent.
+
+As an alternative to changing the language, there already exists today a compiler plugin called [scala-commas][] that provides this feature. It also provides evidence that people would even use unsupported compiler apis and reflection to add this functionality, even when such a plugin won't compose with other plugins well.
+
+## References
+
+1. [SI-4986][]
+2. [scala/scala#5245][]
+3. [scala-commas][]
+
+[SI-4986]: https://issues.scala-lang.org/browse/SI-4986
+[scala/scala#5245]: scala/scala#524://github.com/scala/scala/pull/5245
+[scala-commas]: https://github.com/andyscott/scala-commas
is https://github.com/47deg/scala-commas a better URL for this? it has 27 stars, the andyscott one only has 1 star.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@paulp
paulp Aug 10, 2016 Contributor

What would be the point of trailing commas in tuples? Trailing commas are useful for reordering or removing elements. Tuples are fixed arity and heterogeneously typed. If there aren't going to be trailing commas for argument lists, you're better off doing nothing.

@cvogt
cvogt Aug 10, 2016 Member

+1 to parameter and argument lists being the most important use case to cover. Everything else less important.

@SethTisue
SethTisue Aug 11, 2016 Member

If there aren't going to be trailing commas for argument lists, you're better off doing nothing

Yeah, I said nearly exactly that during the SIP meeting.

@SethTisue SethTisue commented on an outdated diff Aug 10, 2016
sips/pending/_posts/2016-06-25-trailing-commas.md
+
+## Implementation
+
+The implementation is a simple change to the parser, allowing for a trailing comma, for the groups detailed above, and has been proposed in [scala/scala#5245][].
+
+## Drawbacks/Trade-offs
+
+The drawback, or trade-off, to this change is that it adds another way in which it is possible to do something in Scala. But it is the opinion of this SIP that the pragmatic advantage of being able to have trailing commas is worth this drawback.
+
+Given that this is a change in syntax, another drawback is that it requires changing the existing tools, such as those that parse Scala: intellij-scala, scalariform, scala.meta and scalaparse.
+
+## Alternatives
+
+As an alternative, trailing commas support could be added universally to all the comma-separated elements of the syntax. This would mean changing more (but still only in the parser), but it would make it consistent.
+
+As an alternative to changing the language, there already exists today a compiler plugin called [scala-commas][] that provides this feature. It also provides evidence that people would even use unsupported compiler apis and reflection to add this functionality, even when such a plugin won't compose with other plugins well.
@SethTisue
SethTisue Aug 10, 2016 Member

well, only very weak evidence — it's an obscure plugin, described by its author as "a proof of concept, for fun". it does have 27 stars on GitHub, but it also has no issues, no PRs, and only a handful of commits.

@dwijnand
Member

@SethTisue thank you very much for the review, I've made some changes from your feedback. I hope you think that's more appropriate and let me know if not.

@jvican Thank you, I hope you're ok with me updating the document with the breakdown once the SIP committee has provided some feedback.

@Ichoran
Ichoran commented Aug 10, 2016

I am not very enthusiastic about this proposal for a number of reasons.

  1. As read now, comma means "there is another one coming". With this SIP, comma means, "I dunno if there's another one coming or not, but there might be". This is bad for several reasons: it is strictly less informative, it does not follow the conventions of Western languages like English,
  2. Commas gain weird special cases. Although the size of the grammar doesn't really change, the complexity of the grammar does as the pattern is not always something like {Item ',' More} | Item. This directly impacts the cognitive burden on people learning the language.
  3. It encourages more verbose code not less, making code generally messier not cleaner. Since it is optional, it will likely be used both ways, raising the cognitive burden of reading others' code and writing in a consistent style on multiple projects.
  4. The problem that it addresses just doesn't seem very serious to me. I move lists of things around all the time. It takes a few extra seconds to fix up the separators. Yes, it's a bit irritating. I wish my editor would handle it for me, along with all sorts of other things like understanding when I'm opening a new pair of braces and when I'm just replacing an opening brace that I accidentally deleted.

I've used this feature in Julia and I don't like it; when it's very important to know the size of one's vector, not having an at-a-glance count of the number of commas be one less than the length is a minor but unwelcome distraction. I can't see why I'd like it any better in Scala.

That said, if it is decided that something like this is desired, I would much prefer if commas and semicolons acted similarly. That is, when you can have multiple semicolons you can throw as many of them in as you want between expressions as long as you have at least one (and empty expressions don't count). For instance:

scala> val x = if (true) {;;; 7 ;;;} else 2
x: Int = 7

Actually using this capability is rare, but it is regular in that semicolon acts like a type of super-whitespace; just as Scala doesn't care about how much whitespace there is or whether there's any at all, as long as the tokens are delimited somehow, it also doesn't care about just how many semicolons there are or whether there's any statement at all.

Commas now are special in that they always and only separate items of some sort. Making them obey the same conceptual rules as semicolons and whitespace would, I think, simplify the language conceptually overall. There's only one separator concept; you just have three kinds of separation (token, item, expression, ) and thus need different tokens to distinguish them (with newline usually sufficing for semicolon).

Thus, we would allow

val ,,,,, x= if (true) Seq(,,, 7 ,,,) else Seq(2)

Not that you should write it this way, but I think picking the "nice" syntax from the spectrum of logically consistent syntax is the job more of linters and style checkers than the language specification itself. (Unless the language is Python.)

Again, I think the benefits of having comma be a singular separator outweigh it being conceptually consistent with whitespace and semicolon, but I think the existing proposal is less desirable than either.

@DavidDudson

If this proposal were to go ahead it would at least be good to have a compiler warning for multiple trailing commas/semi-colons eg (a,,) That way at least the recommendation from scalas language point of view is at most 1 trailing comma

@cvogt
Member
cvogt commented Aug 10, 2016

@Ichoran

I doubt commas could become optional like semi-colons, as that would make it ambiguous when an expressions spans multiple lines or not. So we can't make the rules identical to ;. What makes it ok in my eyes, that {} and () already have different rules as they cater to different use cases (while having overlaps in semantics). ; as a supporting concept for {} and , as a supporting concept for () working differently seems fine as those work differently.

I don't see why the grammar would be much more complicated. It would just need another case terminating repetition including a comma.

I think one of the main reasons for the proposal of this change is what some perceive as a usability problem. Writing text in an editor is the user interface for developing programs. Developers may use this interface (text) differently, but there seems to be a significant group (I among them), who have the following problem: When parameters or arguments are indented one per line, commenting out the last one requires also removing the trailing comma from the previous line. We perceive this as a usability problem, an irregularity, a distraction from getting stuff done. This might not be the biggest problem there is, but annoying enough to a good number of people to bother making an effort to change it.

@retronym
Member
retronym commented Aug 10, 2016 edited

I agree with Rex's sentiments here. We've come to regret having more than one way to do things syntactically (e.g. procedure syntax).

IntelliJ users can move elements within a comma separated list with the "Code -> Move element {left, right}" (or the corresponding shortcut):

image

I'm also uncomfortable about the following code that reports a syntax error into something that typechecks. , currently means "I'm about to add something".

scala> def foo(a: Int, b: Int = 0) = 0; foo(1,)
<console>:1: error: illegal start of simple expression
def foo(a: Int, b: Int = 0) = 0; foo(1,)
                                       ^

Incidentally, I found a similar enhancement request for Java that suggested to generalize its support trailing commas in array initializers to all comma separated parts of the syntax. That request wasn't accepted, although there wasn't much constructive discussion of the reasons why. Scala has a history on mimicing Java's syntactic decisions closely.

@cvogt
Member
cvogt commented Aug 10, 2016

@retronym

Many of us do not use IntelliJ for usability reasons in particular responsiveness. Different people choose different trade-offs here.

Editing the last element of argument lists happens all the time. I can see your described use case, but it seems rare to me personally. But I understand your case, potential correctness problems have gravity. Everything's a trade-off. I personally tend towards usability in this particular case.

Scala tries to have syntax that is easier to work with than Java, yet familiar to Java-style language folks. I see trailing commas supporting the first and yet not violating the second.

@cvogt
Member
cvogt commented Aug 10, 2016 edited

But good point. It's not only commenting out parameters/arguments, also moving them that has this usability issue.

@Ichoran
Ichoran commented Aug 10, 2016

@cvogt - Can't you just

List(
  fiddesticks /*,
  humbug*/
)

instead of changing the language?

Alternatively, can't you fix your editor to make it easy for you so it works for all languages, not just Scala? There are fewer heavily-used editors than languages.

Healing line-by-line commas is simple: if you comment out a line, and the next line is a close of some sort, and the previous line ends in a comma, comment out the comma as well as the line. If you uncomment it, and the next line is a close, and the previous line has a comment that starts with a comma, remove the comment around the comma also. Compared to semantics-aware refactoring, this is a pretty easy operation. Adding a new color for "syntactically necessary comment", and making that be close to the background color, would remove the visual distraction also.

Moving lines around isn't so hard either.

Moving stuff around on the same line is hard because there is nothing to cue off of except a parse of the language at that point. That's much heavier lifting for an editor / IDE. But this seems pretty esoteric for me, and as I mentioned in chat, it doesn't solve the problem of how to comment stuff out on the same line. (For that to be easier you need commas-anywhere.)

@OlegYch
OlegYch commented Aug 10, 2016

@Ichoran this is the sort of gymnastics i've been doing for years, and it's tiresome..
fixing editors might be a good idea, but everyone has a unique set of editors they use (and probably not comfortable at modifying) and we see a new editor popup on a weekly basis...

@cvogt
Member
cvogt commented Aug 10, 2016

@Ichoran This proposal has a significant number of supporters because we find exactly that a usability hinderance. People are different :).

Regarding editor plugins: Many of us prefer syntax that is easily editable as much as possible without external tooling support. For everything that requires tool support we limit ourselves to tools that support this feature and have to remember how to use it and there is a limited memory budget for things like that, I'd prefer to spend on other things.

Suggestion: Recognizing that a significant number of people would feel their life becomes easier with this, let's focus on how other people feel their life would become harder with this and then weigh off the trade-off.

@Ichoran
Ichoran commented Aug 10, 2016

@cvogt - Fair enough. I explained how my life would be harder in my original post in point 1: it's harder for me to parse Scala with trailing commas because there is more to read yet the symbols are less informative. This isn't hypothetical; I've tried it a fair bit with Julia and never found it worth it (personally).

Also, I will have more trouble reading code written that way.

@eed3si9n
Member
eed3si9n commented Aug 10, 2016 edited

Here are some general surveys of other languages.

semicolons

In ALGOL and Pascal semicolons (;) are statement separator.

begin
  doSomething;
  doSomething
end

This makes sense in the punctuation usage of the semicolon. The placement of semicolon apparently confused IBM (it does get tricky with if-else) that PL/I started using semicolons as statement terminator, in other words making semicolon trailing. This influenced B, C, C++, and Java's usage of the semicolon. Syntactically Scala is related to both Pascal and C lineage, and it's interesting that Scala made semicolon mostly optional.

commas

If we focus on trailing commas,

  • K&R and C85 draft contained trailing comma in initializer. (Example)

    char xs[6] = { 104, 101, 108, 108, 111, 0, };

    C99 added trailing comma for enum initializers too for consistency. C++11 allows it too.

  • Naturally Java allows trailing commas in array initializer and enum constants. (Example)

    private static int a[] = { 1, 2, 3, };
    public enum State {
      ACTIVE,
    }
  • C# allows trailing comma in object literal and collection initializer. Tuples are created using Tuple.Create method (Example)

    var d = new {
        a = new [] { 1, 2 },
    };
    var t1 = Tuple.Create(1);
  • ECMAScript 5, published in 2009 added trailing commas in object literal and array literal. Douglas Crockford says trailing commas was always legal in arrays, but IE was returning different length, so ES5 standardized on ignoring the comma. (Example)

    var obj = {
      'foo': [ 1, 2 ],
    };
  • Ruby also seems to allow trailing commas in Array expression and Hash expression as per #273. (Example)

    d = {
      foo: [1, 2],
    }
  • Python allows trailing commas in lists, tuples, and dictionaries. Tuple1 the existence of comma affects the semantics. See x vs y below (Example)

    d = {
        "A": [1, 5],
        "B": [6, 7],
    }
    x = (1)
    y = (1, )
  • Go allows trailing commas in parameter list and composite literals. Due to the semicolon inference rule, trailing commas are commonly used. See root_unix.go:

    var certDirectories = []string{
        "/etc/ssl/certs",               // SLES10/SLES11, https://golang.org/issue/12139
        "/system/etc/security/cacerts", // Android
    }
  • Rust seems to allow trailing commas in many places, including enum initializer, struct literal, tuple literal, parameter list, and pattern matching expression. There's a pending section in the official guide on this topic. For Tuple1 the existence of comma affects the semantics. (Example)

    let x = (0);
    let t1 = (0,);
    let xs = [1, 2,];
  • Facebook's Hack allows trailing commas in collection initializers like Vector and Map. The official tutorial uses trailing commas for multi-line examples. They've also opened RFC on PHP.

  • Swift allows trailing commas in array and dictionary collection literals, but rejected proposal to add them to parameter lists and tuples. Swift does not provide Tuple1 type. (Example)

    let d = [
     "foo": [1, 2],
    ]
  • In Clojure, commas are considered whitespace so not only you can omit them, add them in-between, or trailing, it do not affect the semantics. tryclj

    > '(1)
    (1)
    > '(1,)
    (1)
    > [1]
    [1]
    > [,,1,,]
    [1]
    > {
         :foo [1, 2],
      }
    {:foo [1, 2]}
  • In F#, list and dictionary elements are separated using semicolons, and are optional at the end of the line. Trailing semicolon is allowed. Tuples are separated using commas, and trailing commas are not allowed. I couldn't find literal for Tuple1. (Example)

    let xs = [1; ]
    let d = dict [
      ("foo", [1; 2]);
    ]
    let ts = (1, 2)
    let t1 = System.Tuple.Create(1)
  • In Haskell, list and tuple elements are separated using commas. By default GHCi does not allow trailing commas. It does however provide syntactic extension called tuple section where (, True) becomes a short hand for \x -> (x, True). With this extension, trailing comma has a distinct semantics.

These trends seem to indicate that languages are moving towards allowing trailing commas. This could be related to more prevalent use of version control and distributed development (purely speculation).

Edit: Corrected C's situation. Added Go, Rust, Hack, Swift, Clojure, F#, and Haskell.

@soc
Member
soc commented Aug 10, 2016 edited

I wrote both SBT and ScalaTags recently, so I probably experienced a lot of the pain described as these things come up quite often in those cases.

I still think that this SIP should be rejected, because:

  • It introduces another pointless way to write exactly the same thing. We spent roughly the last 5 years of getting rid of these variations throughout the language. This will undo all the progress we have made in the last half decade.
  • This is the job of the editor/IDE. Just like we shouldn't have to worsen our code-style (don't align rows!!!) to get nice diffs in git, we shouldn't have to worsen our syntax to make editing easier.
  • Code is read a lot more often than it is written. People reading the code shouldn't have to suffer from the syntax which only exists to make editing slightly nicer.
  • What is the style guide supposed to tell users?
    • Always use trailing commas – This will cause more churn than SIP-12, which was rejected based on that (and SIP-12 would have been a transition, not a permanent state of co-existence of old and new syntax).
    • Never use trailing commas – Why should we introduce them in the first place?
    • Whatever – Why do we even have a style guide?
@cvogt
Member
cvogt commented Aug 11, 2016

added a pro/con summary into the description

@dwijnand
Member

@cvogt uhh this is a documentation PR, so the pros/cons are/should be in the document itself, rather than in the description of the PR.

I'll update the document soon with everything. Also, when I've heard back from @xeno-by about the meeting.

@eed3si9n
Member
eed3si9n commented Aug 11, 2016 edited

Here are some quotes from Rationale for International Standard Programming Languages C:

A new feature of C99: a common extension in many implementations allows a trailing comma after the list of enumeration constants. The Committee decided to adopt this feature as an innocuous extension that mirrors the trailing commas allowed in initializers.

K&R allows a trailing comma in an initializer at the end of an initializer-list. The Standard has
retained this syntax, since it provides flexibility in adding or deleting members from an
initializer list, and simplifies machine generation of such lists.

@Ichoran
Ichoran commented Aug 11, 2016 edited

Can we get a comparison of the pros/cons of allowing this only for multi-line expressions, and a comparison of how bad this is compared to forgetting the commas in the first place (which I not infrequently do when I'm writing dependencies into SBT)?

In particular, the rule would be that instead of whatever ~ ')' you could have whatever ~ (',' ~ newline ~ ')' | ')') to use FastParse-inspired syntax. This seems like it would retain all the listed advantages without exposing some of the disadvantages like making it harder to quickly judge the arity.

I think that it wouldn't help with single-line lists, but that wasn't mentioned, and when moving things on single lines you have to be all fiddly with making sure you grab the right comma anyway, so it's less of a big deal than lines where you're grabbing lines anyway, except the commas don't tear away properly when you do it.

@eed3si9n
Member

Here's from Chris Lattner at Apple [swift-evolution-announce] [Rejected] SE-0084: Allow trailing commas in parameter lists and tuples:

Swift currently accepts a trailing comma in array and dictionary collection literals, for three reasons: evolution of a project often adds and removes elements to the collection over time, these changes do not alter the type of the collection (so those changes are typically spot changes), and the closing sigil of the collection (a right square bracket) is often placed on the line following the elements of the collection. Because of these properties, accepting a trailing comma in a collection literal can help reduce spurious diffs when elements are added or removed.

and Sara Golemon former-Facebook Hack engineer in Re: [PHP-DEV] [RFC][DISCUSSION] Revisit trailing commas in function arguments:

Happy to answer, but I need to state a couple things first:

  • I don't really care if this change lands. I'd kinda like it, but
    it's not solving a massive problem for me.
  • There aren't any compelling reasons against this. The only reason
    given of any note that I've seen is: "There are no compelling reasons
    in favor of it." And I'll agree with that. Like I just said, it's
    not solving any major problems, and it's not going to cause any major
    problems. It's just a tiny, vanishingly insignificant piece of
    syntactic sugar which disappears before we even get to the AST.

So again, could scarcely care less, so don't expect me to champion
either side, but you asked "why", so here it is: It makes code reviews
marginally less ugly.

That's it. It's a tiny problem to solve, and likely saves less than
100ms during diff reviews, but it's a solution to a problem.

@som-snytt
Contributor

I enjoyed the evidence from other languages, but I'm not sure that experience entirely aligns. From the Swift rejection:

Finally, the core team does not want to encourage or endorse a coding style that puts the terminating right parenthesis on a line following the arguments to that call.

If I don't follow paulp parens religiously, where parens in place of braces signal an expression, it's due to an apostasy on my part.

I like the simplifying restriction that it apply only for multiline exprs.

@fanf
fanf commented Aug 11, 2016 edited

You all are aware that most of the pros can be achieved just by adopting an other breaking line convention:

List(
  a
, b
, c
) 

Quoting the pros, we have all of them for free (safe for the first line, but arguably it changes much less often) :

avoids problems with last parameter/argument
*   eases commenting out
*   eases swapping
*   avoids merge conflicts
*   less reliance on tool support or manual labor
@jvican
Member
jvican commented Aug 11, 2016 edited

@fanf FTR, this also works:

a ::
b ::
c ::
Nil 
@fanf
fanf commented Aug 11, 2016

@jvican: the "comma at beginning of line" can be made homogeneous for a lot of structures, not only lists: tuples, function parameter declarations and assignments (especially named one, which lead to a clean and easy to parse vertical alignment), case classes, etc

@jvican
Member
jvican commented Aug 11, 2016

@fanf I'm aware of that limitation, just pointing out a more beautiful way (imo) to do it with Lists.

@cvogt
Member
cvogt commented Aug 11, 2016

@fanf then the problem switches from the last element to the first element.

@fanf
fanf commented Aug 11, 2016

@cvogt: yes, but as said, the first element change much less frequently than the last.

On the other hand, if trailing comma are added, I woukd like the support of heading comma to also fully support the style presented.

@SethTisue
Member

@travisbrown on Twitter: "I'm 👎 on this—Scala doesn't need yet more syntax for people to fight over / be surprised by / be inconsistent about." Having read through all of the discussion here as well as the lengthy discussion on the scala/slip Gitter channel, that's the conclusion I'm reaching, too.

@dragos
dragos commented Aug 11, 2016

I'm not sure if this came up during the discussions, but Scala used to have this feature. I couldn't find the commit that removed it, nor a ticket/discussion, but I vaguely remember @paulp implemented the change.

As a little present, here's scala/scala@8de712d that added support for trailing commas in tuples.

@mlopes
mlopes commented Aug 11, 2016

Voted down as it makes for less readable code, and the problem is detected early during parsing, so it isn't really an issue.

@xeno-by
Member
xeno-by commented Aug 11, 2016 edited

Hi everyone! I'm glad to see that the yesterday's SIP meeting (10 Aug 2016) has sparked such a vibrant discussion. Today I'm here to outline the feedback from the SIP committee:

  1. Explicitly summarize the potential conflicts with tuple syntax that you have extensively outlined in #533 (comment). I think editing the comment and adding bullet list should be enough.
  2. Review the initial HList discussion at lampepfl/dotty#964 to figure out potential conflicts with this proposal.
  3. Consider whether we can salvage non-controversial parts of this proposal and reduce this SIP just to them.
  4. Consider whether it is worth having two ways of doing the same thing, e.g. being able to write argument lists both with and without trailing commans (via @soc's comment in the Gitter channel).

@dwijnand I would also like to ask you to summarize this discussion once the dust settles. I realize that this is a lot of work, but in my opinion this work will be extremely beneficial. It will provide the committee with a quick reference for the next SIP meeting and future generations with an organized justification of the status quo.

@nrinaudo

I feel this makes code less readable, and I spend quite a bit more time reading code than writing it.

Having to unlearn to automatically read commas as "there's something else afterwards" is far more disruptive to my coding process than occasionally having to add / remove a , when commenting things out.

@paulp
Contributor
paulp commented Aug 11, 2016

@dragos It is a cunning tactic to implicate me in an effort to provoke me into pinpointing the commit in which support was removed. And on most days it would be an effective tactic. But not today! I categorically deny any involvement, although I do recall the timing being in the neighborhood of when I first became involved.

In fact I doubt I ever unilaterally committed a change of such significance. I wish I had.

@eed3si9n
Member
eed3si9n commented Aug 11, 2016 edited

I have a pro and a con to add.

VCS authorship attribution

As many have noted, we spend more time reading source than writing the code. In a distributed development, I often read the source code using Github by using t shortcut to search, and then using git blame view, and tracking the history of when/how a bug may have been added.
One of the rationale that's often mentioned is diff for code reviews, but I think having correct commit history (because with trailing comma you don't have to fix the last item) would have longer lasting effect for the code base.

Cross building hindrance

One negative point I can think about, assuming this doesn't get back ported to 2.10 and 2.11 is that it diverges the shape of the source and prevents cross building that may have been possible otherwise.

@odersky
Contributor
odersky commented Aug 12, 2016

The heated discussion here reminds me of Wadler's law:

In any language design, the total time spent discussing
a feature in this list is proportional to two raised to
the power of its position.
0. Semantics
1. Syntax
2. Lexical syntax
3. Lexical syntax of comments

Personally, the only issue I care about is a consistent syntax for HLists.
And one of the promising candidates here would be to say that

(a, b, c, )       is a more legible syntax for      HCons(a, HCons(b,

HCons(c, HNil)))

Then in a second step we could say that the trailing comma is optional as
long as there are at least two elements in the list, so

(a, b, c)      is a shorthand for     (a, b, c, )

Note that if we do this, the more general form is with a trailing comma,
not without. But if we do that it would be a travesty to say that all other
forms of comma separated sequences also have an invisible last element. So
that's why I am skeptical.

@jvican
Member
jvican commented Aug 12, 2016 edited

@dwijnand In addition to @xeno-by's suggestions, could you please update the proposal accordingly?

I believe it's time for us to merge the proposal into the SIP website. Even if merged, further discussion on this SIP still takes place here -- we should try to keep all the opinions in favor or against in the same place for future reviews.

@dwijnand
Member

Absolutely, but it might take me a few days.

On Fri, 12 Aug 2016, 16:54 Jorge, notifications@github.com wrote:

@dwijnand https://github.com/dwijnand :In addition to @xeno-by
https://github.com/xeno-by's suggestions, could you please update the
proposal accordingly?

I believe it's time for us to merge the proposal into the SIP website.
Even if merged, further discussion on this SIP still takes place here -- we
should try to keep all the opinions in favor or against in the same place
for future review.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#533 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAVCIuL0FB2kvlGnANaFH3d7pFP_PUE0ks5qfJcngaJpZM4I-088
.

@Ichoran
Ichoran commented Aug 12, 2016

@odersky - The HList issue didn't make it into the description of this SIP, so I'm not quite sure what the implications for HList syntax are.

  • If HList replaces TupleN, then it can take the same syntactic sugar (with no trailing comma). The long form is always available (like the long form of TupleN) when disambiguation is needed e.g. with single-element HLists.
  • If HList does not replace TupleN, then the comma cannot be extended to all comma-separated lists because that's TupleN sugar already, and the two would be ambiguous if TupleN could have a trailing comma.
  • If the issue is to disambiguate Tuples/HLists from parameter lists, then the trailing comma is probably overly-subtle as a way to denote the difference, and in any case the travesty you mention remains.
  • List-style notation is always an alternative. a :: b :: c :: HNil is undeniably harder to type than (a, b, c, ), but it's also especially clear what is going on because the terminator is explicit.

Is there a discussion of syntactic issues regarding HLists somewhere that could be pulled into or referenced from this SIP?

@som-snytt
Contributor
som-snytt commented Aug 12, 2016 edited

Has anyone asked on StackOverflow yet, "What are all the uses of trailing comma in Scala?"

I do think the non-specific objection, that the proposed syntax is superfluous or adds cognitive complexity and is one more thing to lint, doesn't adequately accommodate the folks who say it would make their lives easier. The goal should be to enable those users. The compromise, if there is one, needn't meet universal acclaim.

@floating-cat
floating-cat commented Aug 13, 2016 edited

As a supplement to eed3si9n:
Dart allows trailing commas in parameter/argument lists (which allowed recently), list/map literals and enum class declarations.
You can find more details here.

@odersky
Contributor
odersky commented Aug 13, 2016

Sent from my iPhone

On 12.08.2016, at 19:33, Ichoran notifications@github.com wrote:

@odersky - The HList issue didn't make it into the description of this SIP, so I'm not quite sure what the implications for HList syntax are.

If HList replaces TupleN, then it can take the same syntactic sugar (with no trailing comma). The long form is always available (like the long form of TupleN) when disambiguation is needed e.g. with single-element HLists.

My point is that the long form is necessary to support Tuple1. And it gives a much cleaner expansion. Try to write expansion rules from (...) to HCons/HNil with trailing commas and without.
If HList does not replace TupleN, then the comma cannot be extended to all comma-separated lists because that's TupleN sugar already, and the two would be ambiguous if TupleN could have a trailing comma.

No, HList should definitely replace TupleN.

If the issue is to disambiguate Tuples/HLists from parameter lists, then the trailing comma is probably overly-subtle as a way to denote the difference, and in any case the travesty you mention remains.

List-style notation is always an alternative. a :: b :: c :: HNil is undeniably harder to type than (a, b, c, ), but it's also especially clear what is going on because the terminator is explicit.

Is there a discussion of syntactic issues regarding HLists somewhere that could be pulled into or referenced from this SIP?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@fanf
fanf commented Aug 13, 2016

Just to be sure: nothing in the proposal implies that a trailing comma has a semantic meaning? If it is proposed that a trailing comma has semantic, I'm even more against the proposal. Scala has already way too much irregularities, subtleties, and traps to add such a big one.

And if sometimes, a trailing comma is mandatory and sometimes not, it is just an enormous cognitive weight, even more for new users. And a tar pit for linters.

@odersky
Contributor
odersky commented Aug 13, 2016

Here are the expansion rules with trailing commas:

  1. "," is right-associative:

    (a, b, c) = (a, (b, c))

  2. Unit is implied:

    (... , ) = (..., ())

Then take (a, b) as syntax for HCons and () / Unit as syntax for HNil.

Sent from my iPhone

On 12.08.2016, at 19:33, Ichoran notifications@github.com wrote:

@odersky - The HList issue didn't make it into the description of this SIP, so I'm not quite sure what the implications for HList syntax are.

If HList replaces TupleN, then it can take the same syntactic sugar (with no trailing comma). The long form is always available (like the long form of TupleN) when disambiguation is needed e.g. with single-element HLists.

If HList does not replace TupleN, then the comma cannot be extended to all comma-separated lists because that's TupleN sugar already, and the two would be ambiguous if TupleN could have a trailing comma.

If the issue is to disambiguate Tuples/HLists from parameter lists, then the trailing comma is probably overly-subtle as a way to denote the difference, and in any case the travesty you mention remains.

List-style notation is always an alternative. a :: b :: c :: HNil is undeniably harder to type than (a, b, c, ), but it's also especially clear what is going on because the terminator is explicit.

Is there a discussion of syntactic issues regarding HLists somewhere that could be pulled into or referenced from this SIP?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@eed3si9n
Member
eed3si9n commented Aug 14, 2016 edited

summary of the language survey

  • 15 or so languages (K&R C, C++11, Java, C#, ECMAScript 5, Ruby, Python, Go, Rust, PHP, Hack, Swift, Clojure, F#, and Haskell) that I looked at all provide some form of literal for List-like collection creation. Of 15, only 2 do not allow whitespace-like trailing commas in the list/array initializer: PHP and Haskell.
  • In comparison, Scala's innovation in this area is that it does not provide syntactic device for collections, but unifies it with the method invocation to the constructor apply. To make a list, we say List(1, 2, 3). This notation leaves room for libraries to improve collections. Thus, supporting trailing comma in collection creation implies supporting them in method invocation.
  • Tuple/HList literals are provided by 8 (C++11, C#, ECMAScript 5, Python, Rust, Swift, F#, and Haskell) out of 15.
    • C++11 and C# do not have special literal for tuples. Thus, they do not have Tuple1 issue or trailing comma support.
    • ECMAScript 5's heterogenous array uses square brackets, and allows trailing comma.
    • Python and Rust use parenthesis and add semantic distinction to (1,) from (1). These are the only languages that would associate semantics to an existence of comma.
    • F# and Swift use parenthesis, and they do not allow trailing comma or provide syntactic sugar for Tuple1.
    • Haskell uses parenthesis, and they do not allow trailing comma or provide syntactic sugar for Tuple1. In addition, there's TupleSection extension to treat (1,) as (1, _).

Tuple.apply

Here's a suggestion around Tuples/HList. Let's use Tuple.apply. If in the future tuples are unified with HList, and thereby normalized to be just one of the collections, I think the Scala way to go is demote its ML/Haskell style syntactic sugar, and promote Tuple(1, 2, 3).

This avoids joining Python club of associating semantics to trailing comma in an already too subtle use of parenthesis. With or without SIP-27 I am against distinguishing (1,) vs (1) in Scala.

@soc
Member
soc commented Aug 14, 2016 edited

@som-snytt

[...] doesn't adequately accommodate the folks who say it would make their lives easier. The goal should be to enable those users.

I don't believe that language design works that way or should work that way.

While there are often additions in the beginning that are largely beneficial to everyone, like traits or generics, you hit the zone of diminishing returns really fast, where each addition adds only a tiny bit of "goodness" to the language, increasingly outweighed by the growing amount of features of the language.

Everyone has their two dozen pet features they would like to see in the language, but adding all of them won't make the language better, it will make the language unusable.

The trailing commas are in my opinion far beyond "diminishing returns" and already in the territory of "making the language worse by adding another feature". The more features you already have, the "better" needs a new feature to be to favor inclusion. I believe that SIP-commas brings nothing to the table in that regard. It's largely a workaround for the symptoms of bad tooling. We should fix the origin of the issues instead.

There will always be things that are more inconvenient to achieve than in other languages, but trying to patch up every instance of this is just playing whack-a-mole. There will always be the "really last thing, promised" that could benefit from some more syntactical sugar.

Language design is largely about what you leave out, not what you put in.

@xuwei-k
Contributor
xuwei-k commented Aug 18, 2016

in crystal-lang crystal-lang/crystal#1780

@dwijnand
Member
dwijnand commented Sep 4, 2016

@Ichoran

  1. As read now, comma means "there is another one coming". With this SIP, comma means, "I dunno if there's another one coming or not, but there might be". This is bad for several reasons: it is strictly less informative, it does not follow the conventions of Western languages like English,

By restricting trailing commas to multi-line, counting elements is now line-based, rather than comma-seeking based.

  1. Commas gain weird special cases. Although the size of the grammar doesn't really change, the complexity of the grammar does as the pattern is not always something like {Item ',' More} | Item. This directly impacts the cognitive burden on people learning the language.

I think the concept of a trailing comma is fairly common enough that the "cognitive burden" is negligible.

  1. It encourages more verbose code not less, making code generally messier not cleaner. Since it is optional, it will likely be used both ways, raising the cognitive burden of reading others' code and writing in a consistent style on multiple projects.

You're right, it does add another way. This is already part of the drawback/trade-off section of the SIP.

  1. The problem that it addresses just doesn't seem very serious to me. I move lists of things around all the time. It takes a few extra seconds to fix up the separators. Yes, it's a bit irritating. I wish my editor would handle it for me, along with all sorts of other things like understanding when I'm opening a new pair of braces and when I'm just replacing an opening brace that I accidentally deleted.

I'm sure there are other problems, I'm just trying to provide a better story for one of them, one which I think is a very common burden for all.

@dwijnand
Member
dwijnand commented Sep 4, 2016

@DavidDudson

If this proposal were to go ahead it would at least be good to have a compiler warning for multiple trailing commas/semi-colons eg (a,,) That way at least the recommendation from scalas language point of view is at most 1 trailing comma

It is not the intent of this SIP to allow for multiple trailing commas - just one, single trailing comma, so the last element isn't any different than any of the previous ones, and can be move or removed without burden.

@dwijnand
Member
dwijnand commented Sep 4, 2016

@retronym

IntelliJ users can move elements within a comma separated list with the "Code -> Move element {left, right}" (or the corresponding shortcut)

Good tip, thank you. When I'm using IntelliJ and when I need to move elements I've started using this technique in the interim.

@dwijnand
Member
dwijnand commented Sep 4, 2016

@eed3si9n, thank you for the wonderful survey on other languages.

@dwijnand
Member
dwijnand commented Sep 4, 2016 edited

@soc

I still think that this SIP should be rejected, because:

It introduces another pointless way to write exactly the same thing. We spent roughly the last 5 years of getting rid of these variations throughout the language. This will undo all the progress we have made in the last half decade.

I disagree strongly that it's pointless, and I invite you to look at the positive feedback the original Scala PR and this SIP has attracted in order to acknowledge that a lot of people also disagree with you.

I also personally think it's unnecessary hyperbole to say that this "will undo all the progress we have made in the last half decade".

I care about your point of view and your feedback, but you make it difficult for me to take it seriously when you disregard obvious evidence and exagerate like that. Continuing to mock the effort put behind this SIP and importance some people give to this SIP by calling it "SIP-commas" doesn't help you either.

This is the job of the editor/IDE. Just like we shouldn't have to worsen our code-style (don't align rows!!!) to get nice diffs in git, we shouldn't have to worsen our syntax to make editing easier.

It's a digression, but know that aligning rows actually doesn't get nice diffs in git, but it does improve the legibility of code, in my opinion.

Instead of pushing the burden of managing how to add, remove or move elements in a comma-separated list to each and every editing tool out there, I'm proposing that scalac learns how to manage trailing commas instead.

Code is read a lot more often than it is written. People reading the code shouldn't have to suffer from the syntax which only exists to make editing slightly nicer.

I believe that the trade-off between how much people suffer to read and ignore a trailing comma and how much people suffer to micro-managing the lack of a trailing comma when editing code is heavily in favour of the latter. It's the duty of the committee to decide and vote if they agree with me on this.

What is the style guide supposed to tell users?

If you're asking my personal opinion, I would say it should say that trailing commas on multi-lines are accepted, for the conveniences specified in this SIP.

@dwijnand
Member
dwijnand commented Sep 4, 2016

@Ichoran

Can we get a comparison of the pros/cons of allowing this only for multi-line expressions, and a comparison of how bad this is compared to forgetting the commas in the first place (which I not infrequently do when I'm writing dependencies into SBT)?

In particular, the rule would be that instead of whatever ~ ')' you could have whatever ~ (',' ~ newline ~ ')' | ')') to use FastParse-inspired syntax. This seems like it would retain all the listed advantages without exposing some of the disadvantages like making it harder to quickly judge the arity.

I think that it wouldn't help with single-line lists, but that wasn't mentioned, and when moving things on single lines you have to be all fiddly with making sure you grab the right comma anyway, so it's less of a big deal than lines where you're grabbing lines anyway, except the commas don't tear away properly when you do it.

I agree with you, which is why I'm proposing we restrict this change to only multi-line comma-separated elements.

@dwijnand
Member
dwijnand commented Sep 4, 2016

@som-snytt

I like the simplifying restriction that it apply only for multiline exprs.

As do I.

@dwijnand
Member
dwijnand commented Sep 4, 2016

@fanf

You all are aware that most of the pros can be achieved just by adopting an other breaking line convention:

List(
  a
, b
, c
)

Quoting the pros, we have all of them for free (safe for the first line, but arguably it changes much less often) :

avoids problems with last parameter/argument

  • eases commenting out
  • eases swapping
  • avoids merge conflicts
  • less reliance on tool support or manual labor

I am aware, but I reject it as a workaround for 3 reasons.

First, as you mention, it moves the problem to the first element, which only maybe might not be a problem.

Secondly, in the case where a change must happen near the first element (either by adding one or more elements before the first element, or by remove the first element, either entirely or by commenting out) it makes the change a lot harder, because it requires micro-managing the leading comma, replacing it in situ with a space.

Thirdly, it's a large departure from the "normal" style - in contrast I'm only proposing an optional trailing comma.

One final note, which isn't an objective reason but my own personal taste, I can't stand that style.

You might be interesting to know that @olafurpg has added an experimental poorMansTrailingCommasInConfigStyle to his tool scalafmt (shipped with version 0.3.1): olafurpg/scalafmt#412

@dwijnand
Member
dwijnand commented Sep 4, 2016

@fanf

On the other hand, if trailing comma are added, I woukd like the support of heading comma to also fully support the style presented.

I'm personally not interesting in supporting that, but if you feel strongly enough about it, I invite you to consider creating a SIP for it.

@dwijnand
Member
dwijnand commented Sep 4, 2016

@eed3si9n

I have a pro and a con to add.

VCS authorship attribution

As many have noted, we spend more time reading source than writing the code. In a distributed development, I often read the source code using Github by using t shortcut to search, and then using git blame view, and tracking the history of when/how a bug may have been added.
One of the rationale that's often mentioned is diff for code reviews, but I think having correct commit history (because with trailing comma you don't have to fix the last item) would have longer lasting effect for the code base.

That's a good point, thank you, I'll add it to the SIP.

Cross building hindrance

One negative point I can think about, assuming this doesn't get back ported to 2.10 and 2.11 is that it diverges the shape of the source and prevents cross building that may have been possible otherwise.

While I agree that it's a con that must be managed and considered, it's also not unique to this SIP.

@dwijnand
Member
dwijnand commented Sep 4, 2016

@odersky

Personally, the only issue I care about is a consistent syntax for HLists.

Please see my grammar breakdown, where I address the concern about tuples/HLists and Tuple1.

@dwijnand
Member
dwijnand commented Sep 4, 2016

@fanf

And if sometimes, a trailing comma is mandatory and sometimes not, it is just an enormous cognitive weight, even more for new users. And a tar pit for linters.

No, the proposal doesn't mandate trailing commas.

@dwijnand
Member
dwijnand commented Sep 4, 2016

@xeno-by

1. Questions for the SIP committee:

  1. What does the commitee think if trailing commas were only allowed for multi-line comma-separated elements?

This would mean that this would be allowed:

class Foo(
  bippy: String,
  dingo: Int,
)

but this would not:

class Foo(bippy: String, dingo: Int, )

Then there is the question of how much of the Scala grammar should support trailing commas.

In, what I consider to be, decreasing order of importance:

Important to support:

  1. Should trailing commas be supported for ArgumentExprs?

  2. Should trailing commas be supported for Params and ClassParams?

Less important to support:

  1. Should trailing commas be supported for SimpleExpr1? And if yes should (1,) not compile or compile to a Tuple1?

  2. Should trailing commas be supported for TypeArgs, TypeParamClause and FunTypeParamClause?

  3. Should trailing commas be supported for SimpleType and FunctionArgTypes?

  4. Should trailing commas be supported for SimplePattern?

  5. Should trailing commas be supported for ImportSelectors?

Not important to support, except for consistency reasons:

  1. Should trailing commas be supported for Import?

  2. Should trailing commas be supported for Bindings?

  3. Should trailing commas be supported for ids, ValDcl, VarDcl, VarDef and PatDef?

2. Compare to the HList discussion in Dotty

To my limited understanding of lampepfl/dotty#964, I don't see any conflict between my proposal and Dotty's proposal.

3. Salvaging non-controversial parts

My initial proposal was restricted to a small subset of (what I considered) non-controversial parts of the grammar, then the SIP committee requested to explore all the cases of the grammar that have comma-separated lists, which I did. Personally I'm happy to go either way on this.

4. Two ways of doing the same thing

From very early in this SIP it has been acknowledged that this is a drawback/trade-off. Perhaps restricting trailing commas to multi-line reduces the gravity of this drawback.

@dwijnand
Member
dwijnand commented Sep 4, 2016

@jvican While the details of this SIP are still being worked out, specifically given the number of questions I have for the committee, I would like to hold off from committing even more time changing (and re-changing) the specifics in the actual SIP document until the situation is clearer.

You have my word though that even in the event that the SIP is rejected I would update the SIP document accordingly.

@Ichoran
Ichoran commented Sep 4, 2016

Personally, I have far fewer issues with trailing commas only when newline is allowed. There is still the concern about whether there is a missing item, but unlike the inline case where a missing entry is likely and the editing help you get is minimal, with separate lines, a missing entry is unlikely and the editing help you get is maximal. Definitely shifts the risk/reward ratio.

I remain concerned about making the specification more complex, but if it's isolated to multiple lines and phrased like semicolon inference but backwards, it feels a lot tidier.

And I would make it apply always, no exceptions. If you can have something separated by commas and it can go on separate lines, you can always have a trailing comma and it's cool.

@dwijnand
Member
dwijnand commented Sep 4, 2016

And I would make it apply always, no exceptions. If you can have something separated by commas and it can go on separate lines, you can always have a trailing comma and it's cool.

I personally could live with that, but I'm not sure if everyone would.

Also it would have to be a gradual change though, particularly given there are libraries that cross-build to multiple versions of Scala.

@Ichoran
Ichoran commented Sep 4, 2016

I can't comfortably live with anything other than a completely consistent rule. I have too many other things to pay attention to.

@jvican
Member
jvican commented Sep 5, 2016

@dwijnand I'm glad to see that you have so many interesting questions for the Committee! I'll ask them in our next meeting, that coincidentally takes place at Scala World, on the 13th of September (that's next week).

Thanks for your commitment, Dale.

@sjrd sjrd and 3 others commented on an outdated diff Sep 12, 2016
sips/pending/_posts/2016-06-25-trailing-commas.md
+* SimplePattern via Patterns
+* TypeParamClause, FunTypeParamClause
+* ImportExp
+* PatDef
+
+## Implementation
+
+The implementation is a simple change to the parser, allowing for a trailing comma, for the groups detailed above, and has been proposed in [scala/scala#5245][].
+
+## Drawbacks/Trade-offs
+
+The drawback, or trade-off, to this change is that it adds another way in which it is possible to do something in Scala. But it is the opinion of this SIP that the pragmatic advantage of being able to have trailing commas is worth this drawback.
+
+Given that this is a change in syntax, another drawback is that it requires changing the existing tools, such as those that parse Scala: intellij-scala, scalariform, scala.meta and scalaparse.
+
+Another drawback is that for projects that cross build to previous versions of Scala they would have to take into account that this feature wouldn't be available for those versions (assuming this feature isn't backported).
@sjrd
sjrd Sep 12, 2016 Member

How is this any worse than for any other change to Scala? Even simple additions to the standard library are subject to this.

@dwijnand
dwijnand Sep 12, 2016 Member

Indeed

I thought it's fair to mention it.

@sjrd
sjrd Sep 12, 2016 Member

IMO this is noise. I was trying to figure out what made this SIP special about this. IMO this paragraph should be removed.

@fanf
fanf Sep 12, 2016

It leads to source incompatibility at the language level. It implies a major version bump, not only a minor one (as far as I understand Scala policy regarding that, for ex. https://twitter.com/viktorklang/status/735772498780819457)

@sjrd
sjrd Sep 12, 2016 Member

It is not forward source compatible (most things aren't), but it is backwards source compatible. So an existing codebase will always compile with a new compiler. This only requires a minor version bump.

FTR, adding a method to the standard library is less compatible than that. It can always break backward source compatibility of some codebases, if they pimped a method of the same name via implicits: the new method would suddenly take over instead of the implicit pimp. If the semantics are different, this breaks code.

This change is much more compatible than adding a method, as it can never break existing codebases.

@eed3si9n
eed3si9n Sep 12, 2016 Member

@sjrd I specifically mentioned Cross Build Hinderance (#533 (comment)) as a drawback.

It's a drawback worth considering for this SIP in particular because the added benefits are not as feature-based, and closer to the pro/con 50:50 point than, let's say SIP-14 Futures and Promises. Sure adding Future to the standard library would prevent cross building, but it does more things.

Another reason to think about this specifically for this SIP is that the surface area that might affect this SIP is potentially all Scala sources, which I don't think can be said of just adding a method to the standard library. Closer analogy might be SIP-18 Modularizing Language Features which might have added import language.implicitConversions etc many existing source. The potential surface area becomes relevant especially given the rise of auto formatting of Scala source using Scalariform or Scalafmt.

@sjrd
Member
sjrd commented Sep 12, 2016

The anywhere-in-grammar but only-for-separate-lines variant seems very nice to me. Clearly an improvement wrt the initial proposal, IMO :) As @Ichoran put it, it definitely improves the benefit/cost ratio.

@dwijnand dwijnand Remove Cross building hinderance from drawbacks b2ab043
@xeno-by
Member
xeno-by commented Sep 23, 2016

Hello everyone!

I am here to officially announce that SIP-27 Trailing Commas has been accepted by the SIP committee during the meeting on 20 Sep 2016.

According to the formal rules of the process (http://docs.scala-lang.org/sips/sip-submission.html), the committee proposes that compiler maintainers include this new language feature in a future release of the Scala compiler.

Moreover, I would like to ask @dwijnand to update the specification and implementation according to our discussions. Concretely, the committee has voted in favor of a proposal that allows trailing commas everywhere in the grammar given that the commas are immediately followed by a newline. @dwijnand: Please change the SIP and the prototype implementation to reflect that.

This ends the SIP process for the trailing commas proposal. I would like to thank @dwijnand for the thorough work on this language proposal and everyone for the insightful discussion. It is a pleasure to collaborate on building the future of the language together.

@jvican jvican merged commit eae494c into scala:master Sep 23, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@dwijnand dwijnand deleted the unknown repository branch Sep 23, 2016
@the-Arioch

With regard to "ease commenting out first/last element" maybe what could solve it would be generalizing of Unit as a terminator. It would be consistent with lists ending with ::Nil

Example

import
dir1.dir2.dir3.file4 ,
dir9.dir8.dir7.file6 ,
dirA.dirB.dirC.fileD ,
dirZ.dirY.dirX.fileW ,
();

Here you go - add, remove, (un)comment, reorder - the explicit stub-terminator would prevent compilation error.

@sjrd
Member
sjrd commented Sep 24, 2016

@the-Arioch That doesn't generalize beyond imports. You can't do the same for actual parameters or for bindings, since an additional , () there is already valid and has another meaning.

@the-Arioch

You can use sone other terminator as well, like lists using Nil or HNil

@sjrd
Member
sjrd commented Sep 24, 2016

No, Nil and HNil suffer from the same issue, including for imports because they're just identifiers. You would need to find some piece of syntax that is currently never valid in any of the contexts where comma-separated lists exist.

The above proposal, which has been approved, uses a bare newline as this unique piece of syntax.

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