Formatting: don't dedent comma between nonbreaking call arguments

[helixbass]

This:

tokens.push(
  tokenStore.getLastToken node, if hasSemi then 2 else 1 # from
,
  tokenStore.getLastToken node, if hasSemi then 1 else 0 # "foo"
)

shouldn't be printing the dedented comma ie should be:

tokens.push(
  tokenStore.getLastToken node, if hasSemi then 2 else 1 # from
  tokenStore.getLastToken node, if hasSemi then 1 else 0 # "foo"
)

[GeoffreyBooth]

Really? How would we know that these are two arguments without the comma?

I'm not a fan of implicit commas for function arguments, there's an issue discussing the topic in the CoffeeScript repo.

[helixbass]

That "dedented comma" style is intended for visually separating function arguments that otherwise might not parse correctly eg:

a(
  implicitObject1: yes
,
  implicitObject2: yes
)

vs (parses differently):

a(
  implicitObject1: yes
  implicitObject2: yes
)

or that might look weird eg:

a(
  if b
    c
,
  if d
    e
  else
    f
)

vs (hard to visually parse the different arguments):

a(
  if b
    c
  if d
    e
  else
    f
)

How would we know that these are two arguments without the comma?

The fact that they're consecutive argument lines that start at the same indentation should indicate two separate arguments

I'm not a fan of implicit commas for function arguments

So generally then you'd want to turn on the option comma: all or comma: nonTrailing (vs the default comma: none)

With those settings you get commas for function arguments, eg with comma: nonTrailing you'd get:

tokens.push(
  tokenStore.getLastToken(node, hasSemi),
  tokenStore.getLastToken node, hasSemi
)

For the snippet I used for this issue, you should get this with comma: nonTrailing:

tokens.push(
  tokenStore.getLastToken(node, if hasSemi then 2 else 1), # from
  tokenStore.getLastToken node, if hasSemi then 1 else 0 # "foo"
)

But the bugginess in this issue (whether using comma: none or comma: nonTrailing/comma: all) is that this "dedented comma mode" is kicking in that should only kick in when the function arguments "break" into multiline indented structures eg this is correct:

tokens.push(
  tokenStore.getLastToken node,
    if hasSemi and someMuchMuchMuchMuchMuchMuchMuchMuchLongerCondition
      2
    else
      1
,
  tokenStore.getLastToken node,
    if hasSemi and someMuchMuchMuchMuchMuchMuchMuchMuchLongerCondition
      1
    else
      0
)

Also if you don't like implicit commas for function arguments but do like them elsewhere (eg for array elements, nonimplicit object properties, function params) then it would probably make sense for me to refine the comma option so that you could specify different comma preferences for different syntax constructs (eg comma: ['none', {functionArguments: 'nonTrailing'}])

[GeoffreyBooth]

I can't seem to find it now, but there was a discussion during the CoffeeScript 2 effort about allowing one-per-line function arguments, e.g.:

fn
  arg1
  arg2
  arg3

Which currently compiles into an implicit object if any of those arguments have colons. So we didn't end up supporting that. There was pretty strong opposition, if I remember correctly, because people didn't like how close to ambiguous it is (you have to read fairly closely to see if it's an implicit object or multiple arguments).

[helixbass]

Ok yup I agree with not allowing that as new syntax

From what I'm aware the only time currently that you can omit call parens with indented args is with an initial implicit object first arg eg:

  fn
    a: 1
    b: 2

The Prettier plugin does understand that

people didn't like how close to ambiguous it is (you have to read fairly closely to see if it's an implicit object or multiple arguments)

This is along the lines of my thinking with when to force a dedented comma between args, to help with readability when it's not a simple "one argument per line" argument list, eg it will do this:

a(
  b
,
  c: 1
  d: 2
,
  e
)

rather than the less readable:

a(
  b
  c: 1
  d: 2
  e
)

[GeoffreyBooth]

Yeah. @jashkenas have a difference of opinion when it comes to arrays of objects. I prefer this style:

arr = [
  a: 1
  b: 2
,
  a: 3
  b: 4
]

While he prefers:

arr = [
  {
    a: 1
    b: 2
  }
  {
    a: 3
    b: 4
  }
]

Not exactly this issue, but related 😄. There are people who really dislike the deindented comma just floating out there, because it’s a tiny thing that’s easily overlooked; whereas others hate unnecessary braces. We had a back and forth about this in a PR a long time ago in the CoffeeScript repo. I think both opinions are valid. (I know, Prettier is supposed to be opinionated. . . .)

[jashkenas]

Not exactly this issue, but related 😄. There are people who really dislike the deindented comma just floating out there, because it’s a tiny thing that’s easily overlooked; whereas others hate unnecessary braces. We had a back and forth about this in a PR a long time ago in the CoffeeScript repo. I think both opinions are valid. (I know, Prettier is supposed to be opinionated. . . .)

Quite right! Just to restate my opinion — I have a strong dislike for the floating dedented comma.

Optional braces, brackets and parens are supposed to be a convenience that can aid readability when used tastefully ... they're not supposed to be enforced or relied upon, and it's alright if there are many syntactical situations where you need to use them to get a clean parse.

[helixbass]

I think both opinions are valid

I agree!

(I know, Prettier is supposed to be opinionated. . . .)

Well yes to an extent. I've been a bit more liberal than the Prettier JS formatter with the stylistic formatting options exposed

In this case @GeoffreyBooth you get your preferred formatting by default:

arr = [
  a: 1
  b: 2
,
  a: 3
  b: 4
]

while you can get the @jashkenas preferred version by using respectExplicit: ['objectBraces'] (ie keep any non-implicit objects from the source as non-implicit) or noImplicit: ['objectBraces'] (ie make all objects non-implicit):

arr = [
  {
    a: 1
    b: 2
  }
  {
    a: 3
    b: 4
  }
]

[helixbass]

Quite right! Just to restate my opinion — I have a strong dislike for the floating dedented comma.

Interesting. @jashkenas what would your preferred formatting be for eg:

f(
  if someCondition
    firstArg1
  else
    firstArg2
  secondArg
)

Basically I'm thinking through what formatting alternatives would be for other places where currently I'm generating a floating dedented comma

It's possible that beyond the respectExplicit/noImplicit ways to avoid the dedented comma in the previous example, I could expose an avoidDedentedComma option that would generally try and avoid using dedented commas as a separator (so would eg force object braces where an implicit object would want to be surrounded by dedented commas)

Optional braces, brackets and parens are supposed to be a convenience that can aid readability when used tastefully ... they're not supposed to be enforced or relied upon, and it's alright if there are many syntactical situations where you need to use them to get a clean parse

As of now the default behavior of the Prettier formatter is to try and strip braces/parens wherever possible (except for a couple situations where readability seems significantly improved with explicit braces/parens), and then it's up to you to "insist" on explicit braces/parens via noImplicit/respectExplicit

It seems shaky to think about other mechanisms for sometimes using explicit braces/parens (where they'd be syntactically optional) since it'd be hard to categorize them I'd think (and/or name them for exposing options to allow people to fine-tune in which specific types of circumstances they do or don't prefer explicit braces/parens)

But if you can describe different types of syntactic patterns where you do/don't prefer explicit braces/parens, that would be quite interesting fodder

[jashkenas]

For your example:

f(
  if someCondition
    firstArg1
  else
    firstArg2
  secondArg
)

I would generally prefer:

firstArg = if someCondition
    firstArg1
  else
    firstArg2

f firstArg, secondArg

... but that's not the sort of thing that Prettier can do to a piece of code.

As to a general rule — I enjoy dropping the first, outermost level of braces/parens/brackets in a given expression ... but once you have more than one nesting of optional braces/parens/brackets, I prefer to include all of the inner ones, to help keep things crystal clear.

For a real example from thirty seconds ago, I initially misread your code snippet while skimming, assuming that secondArg was part of the conditional body.

[GeoffreyBooth]

When I ran the plugin on CoffeeScript’s Cakefile a few months ago (so things might've changed since then) my biggest dislike was it adding parentheses around calls of implicit objects, e.g.:

fn(
  a: 1
  b: 2
)

where the original source was the same but without those parentheses.

I think the CoffeeScript compiler codebase is a good benchmark for what many, if not most, people would consider good style. In the code I’ve written for it, I’ve generally done as @jashkenas describes: leave out parentheses and braces unless it’s at all confusing/ambiguous to a reader, even if it parses fine. That’s a hard thing to translate into rules, for sure, but one way to see how far or close you’re getting is to run the plugin on the CoffeeScript codebase and see what it changes. Maybe there’s a way to unprefer dedented commas without affecting parentheses or braces more generally. (But please don’t take away my dedented commas 😄.)

My personal style is mostly the same as the CoffeeScript codebase except that I use tabs, word-wrapped lines (I let lines run as long as they need to) and I don’t make code line up on subsequent lines (like lining up the word require in a bunch of require statements at the top of a file). I also generally always either indent objects or put braces around them; so fn {a: 1} or the indented version, but never fn a: 1. If I could have a rule to represent my preference, it would be “object with braces when it can fit on one line, else implicit object without braces.”