elm-lang / elm-plans

no longer in use, just want to keep discussions around
BSD 3-Clause "New" or "Revised" License
29 stars 1 forks source link

Proposal: Allow Trailing Commas #2

Closed rtfeldman closed 9 years ago

rtfeldman commented 9 years ago

This is a proposal based on maxhoffman's comment. I had thought trailing commas were off the table, but evidently I was mistaken!

Proposed Change

1. Allow trailing commas in any comma-delimited term.

For example, currently the list literal [1, 2, 3] (with interspersed commas) is allowed, but [1, 2, 3,] (with trailing comma) is not allowed. This change would allow either, and would consider them equivalent.

This would be a non-breaking change to the compiler, as it would strictly permit additional syntax. It would currently apply to:

  1. list literals
  2. record literals
  3. record type aliases
  4. tuples
  5. module exports
  6. the exposing portion of module imports.

2. In the official style guide, recommend consistent ending commas for multiline terms.

Consistent Ending Commas Style (now preferred)

type alias Circle =
    {
        x : Float,
        y : Float,
        radius : Float,
    }

Interspersed Leading Commas Style (no longer preferred)

type alias Circle =
    { x : Float
    , y : Float
    , radius : Float
    }

Examples

The 2014 style guide proposed by @laszlopandy was written with the assumption that trailing commas were allowed. It provides several examples of using consistent ending commas in multiline terms.

The only change I would make to those multiline examples would be to recommend lining up the opening brace with the closing brace, instead of having the opening brace on the previous line. This would be consistent with the official style guide's recommendation of prioritizing consistency and letting things line up naturally over conserving vertical space.

For example, here is the "Partially empty lines" snippet updated with the latest style recommendations:

let
    urls =
        [
            "http://elm-lang.org",
            "http://example.com",
        ]

    imageProps =
        {
            url: "http://example.com/image.png",
            width: 100,
            height: 50,
        }

Motivation

Consider these two ways to express the same idea, copied from the first section above.

Consistent Ending Commas Style (now preferred)

type alias Circle =
    {
        x : Float,
        y : Float,
        radius : Float,
    }

Interspersed Leading Commas Style (no longer preferred)

type alias Circle =
    { x : Float
    , y : Float
    , radius : Float
    }

Compared to the proposed Consistent Ending Commas styles, the Interspersed Leading Commas style has these drawbacks:

  1. It is harder to scan. Instead of your eye being able to jump to the first letter on the line, it must jump to the comma (or brace) and then parse past it.
  2. Any change that involves the first position (e.g. adding something at the top, or reordering it) is more time-consuming in a text editor because it has a different leading character than the other lines. This is an especially noticeable pain point when using multiline lists in elm-html DSLs, as much of building web app interfaces involves rearranging elements.
  3. It is unfamiliar to most programmers. The following style guides use ending commas exclusively:
    1. Guido van Rossum's style guide for Python (consistent ending)
    2. thoughtbot's Ruby style guide (consistent ending)
    3. Google's Java style guide (consistent or interspersed endings permitted, but not leading)
    4. jQuery's style guide (interspersed ending; old JS did not allow trailing commas)
    5. Mozilla's JavaScript style guide (interspersed ending; old JS did not allow trailing commas)

Without this proposal, the only way to do ending commas is interspersed:

Interspersed Ending Commas Style (currently allowed, not recommended)

type alias Circle =
    {
        x : Float,
        y : Float,
        radius : Float
    }

Compared to the proposed Consistent Ending Commas styles, the Interspersed Ending Commas style has these drawbacks:

  1. If you add a new entry to the end, you have to remember to add a comma to the preceding line, turning the addition you wanted to make into an addition plus an edit to an unrelated piece of code.
  2. Because of the previous point, your VCS diffs become noisier when using this style.
  3. When you forget a comma at the end of a line, the error message you get will not directly point you in the right direction...and because it's such a tiny visual difference, it's difficult to spot. I recently had a coworker spend several minutes being bitten by this in a SQL query.

Compared to this proposal, using significant whitespace instead of commas has these drawbacks:

  1. It is personally unpopular among a significant portion of Elm users. Sample comments: 1, 2, 3, 4, 5
  2. It is a breaking change that would take considerably more effort to implement, whereas this is a very minor compiler change that is entirely backwards-compatible.
  3. It requires additional breaking changes to function application in order to prevent parsing ambiguities.

Since this is a nonbreaking change of trivial complexity, the only significant drawbacks are stylistic:

  1. The HaskellWiki style guide recommends interspersed leading commas, and Elm's syntax is very similar to Haskell's.
  2. In English, lists use interspersed commas, meaning any non-interspersed style can feel "weird."
  3. Multiple Elm users have voiced a preference against optional syntax (e.g. 1, 2), and this proposal is for an optional trailing comma.
Apanatshka commented 9 years ago

:+1: When summarised like this, it's clear that this is the best way to go :) Thanks for writing this up. With a little extra effort you could also enforce this, by not allowing trailing commas inline and requiring trailing commas if the ending bracket is not on that line. I think that wouldn't be weird, and it would eliminate the optional stuff. It would however be a breaking change.

rtfeldman commented 9 years ago

I'd definitely appreciate that feature, but not sure what @evancz thinks?

(That said, I honestly don't have strong feelings about mandatory vs. optional.)

laszlopandy commented 9 years ago

+1. Said much better than I could. Thanks!

texastoland commented 9 years ago

:+1: @Apanatshka Sounds good but could take it or leave it.

maxhoffmann commented 9 years ago

Thanks @rtfeldman for writing the detailed proposal. As JS got optional trailing commas now I’d be happy to see them in Elm as well. :+1:

texastoland commented 9 years ago

What would this look like for records of functions?

rtfeldman commented 9 years ago

You mean with lambdas? Presumably the same as it currently does, although maybe you'd wrap them in parens if need be.

texastoland commented 9 years ago

They can be written as plain functions too (arguments before equals vs after) but yeah. When I was trying to fake typeclasses this morning I noticed it's pretty horrible. Prefixing commas for functions and postfix for values sounds worse than status quo. And parens just raise another formatting issue. Should I put together a gist?

rtfeldman commented 9 years ago

Please do!

On Mon, Jul 13, 2015 at 12:09 PM Texas Toland notifications@github.com wrote:

They can be written as plain functions too (arguments before equals vs after) but yeah. When I was trying to fake typeclasses this morning I noticed it's pretty horrible. Since we know they're coming down the pipe someday it seems worth accounting for. Prefixing commas for functions and postfix for values sounds worse than status quo. And parens just raise another formatting issue. Should I put together a gist?

— Reply to this email directly or view it on GitHub https://github.com/elm-lang/elm-plans/issues/2#issuecomment-121025351.

mgold commented 9 years ago

Link 404s (or is private)

Apanatshka commented 9 years ago

@dnalot next time use the github shortcut key y to expand the url to canonical form before copying it :)

texastoland commented 9 years ago

Sorry guys finally an actual example!

texastoland commented 9 years ago

Thanks @Apanatshka that's exactly what happened to the previous one.

mgold commented 9 years ago

Ooh, neat trick!

rtfeldman commented 9 years ago

Ah, I see.

Honestly in those cases I'd just move those functions up to the toplevel (or a let binding if I needed stuff in local scope) and then do { model = 0, update = update, view = view }.

This is more concise than that by 1 line of code, but I don't think it's any easier to read.

texastoland commented 9 years ago

The gist is an example not a use case. The use case is aggregating collection functions. That's still WIP or I'd post it. Regardless of use case this proposal needs to accomodate functions in records. Parens or breaking each one out is untennable for a lot if short functions.

rtfeldman commented 9 years ago

In that case I'd put the comma where it would go in JS if I were writing function() { ... }, inline, except without the } because lambdas aren't surrounded by braces in Elm: https://gist.github.com/rtfeldman/5eeed45b7c4fd8ef83d1

texastoland commented 9 years ago

Which is what Evan has been doing. But a lot of vertical whitespace hurts readability for me. Ten functions would be ten more lines of nearly pure whitesapce. My point is trailing commas raise one concern. I agree with your points about readability, code maintenance, and familiarity with almost every other language. I agree with Evan's point that commas vs newlines make it conspicuous you're inside a data structure. Optional leading comma might alleviate it but be even less conventional than newlines. I don't think my issue weakens this proposal. Just whatever we decide should end up in the style guide and be minimally painful compared to status quo.

Apanatshka commented 9 years ago

@dnalot I think the style guide would say to use , on it's own line between functions. You can then still opt for not following the style guide and using leading comma's instead, if this proposal goes through with the optional syntax.

What I find most interesting about this example is that enforcing that last trailing comma in this case would really suck. So my suggestion to make this non-optional would either get more complicated (special case for function as the last element of a multi-line list defininition), or you could just ignore it :)

rtfeldman commented 9 years ago

That's a good point...in light of that, I think I'd lean towards just going with the original proposal as it is currently described (they're strictly optional).

Apanatshka commented 9 years ago

I'm leaning towards that too, otherwise the enforced syntax would become too complicated.

mgold commented 9 years ago

I've said this on another proposal already today... but while I favor non-optional in theory, and am linked to on that in the proposal, optional seems to work out the best in this case. Though, for those who want to use the interspersed form, I would say that being consistent in a codebase in more important than any one form.

You're definitely right about being easier to scan - it's an eerie nonverbal experience, paying attention to how your eye moves, but it's real.

texastoland commented 9 years ago

Agreed. For context (not arguing its acceptance) I'm checking out the impact of allowing rank-2 in a specific case without higher kinds. I wanted to see if it can facilitate code reuse without generic containers. But you'd end up with a long record of short functions. I can't think of any other use case for that. Just wanted to cover our bases!

texastoland commented 9 years ago

I've been playing with commas on their own line. Actually does look superb for my use case.

evancz commented 9 years ago

I am closing this repo down. This idea goes back to Laszlo's initial style guide. I remember it, and will continue to remember it. We may be doing a few syntactic changes/simplifications in 0.16 so I will revisit this myself when the time comes. I will also reach out to groups specifically to gather more information on this when I need it.

likethesky commented 8 years ago

Just to save someone else the time to test this themselves: 0.16.0 does not implement (that is, allow) trailing spaces in comma delimited terms. So you're either stuck leaving the last one off, or sticking with the "pre comma" style for now, until some later version, presumably.

sporto commented 8 years ago

+1 for this

boxed commented 8 years ago

Trailing commas is pretty great. The argument that it's clearer with the comma at the start of the line seems bogus to me... how often would you miss a comma and the type checker DIDN'T find that?

passiomatic commented 7 years ago

+1 for allowing a trailing comma. Coming from Python and JavaScript-land I would expect Elm to allow a trailing comma, especially in records literals -- this is where typically Elm give me a compilation error since it isn't so unusual to add and remove fields during a development/refactoring.

colinta commented 7 years ago

Boy I hate to +1 an issue or feature request, I really do - but since I am brand new to elm, and I love Elm already (I am a vocal dissenter to JavaScript and all the chaos that it embraces), I guess I'm excusing myself on this one, since it's a language-syntax feature, not something I could (reasonably) implement in a fork or even a pull-request.

Not having trailing commas was an early sad surprise - trailing commas are the easiest way to support reordering literal List items (no special first or last item). I hope elm adds them.

nickshanks commented 6 years ago

If you are going to include these as points against Interspersed Ending Commas Style:

Compared to the proposed Consistent Ending Commas styles, the Interspersed Ending Commas style has these drawbacks:

If you add a new entry to the end, you have to remember to add a comma to the preceding line, turning the addition you wanted to make into an addition plus an edit to an unrelated piece of code. Because of the previous point, your VCS diffs become noisier when using this style.

Then you should list the corresponding points as arguments against Interspersed Leading Commas Style too:

"If you add, remove or re-arrange an entry at the front, you have to remember to add a comma to the following line, turning the addition you wanted to make into an addition plus an edit to an unrelated piece of code. Because of the previous point, your VCS diffs become noisier when using this style."

Only Consistent Ending Commas Style is immune to unrelated edits both ends of the list (as long as the opening and closing braces are on a different lines to the list items).