arnetheduck / nph

An opinionated code formatter for Nim
Other
77 stars 12 forks source link

trailing comma in collections #57

Closed Archargelod closed 4 months ago

Archargelod commented 4 months ago

Please, consider not removing a trailing comma for multiline collections. Nim simply ignores it. Having a comma at the end of every element makes it easier to add, move and sort elements (e.g. with Vim).

Example:

var arr = [
  "zbc123looooooooooong line",
  "abc123looooooooooong line",
  "abd123looooooooooong line",
  "bbd123looooooooooong line",
]

Formatted with nph:

var arr = [
  "zbc123looooooooooong line",
  "abc123looooooooooong line",
  "abd123looooooooooong line",
  "bbd123looooooooooong line"
]

Now If you have to copy or move the last line or add elements after it - you can often forget to put a comma. Is there a downside to having a trailing comma? I don't see any.

arnetheduck commented 4 months ago

Here's the actual formatting of that example (with nph-0.4.1):

var arr = [
  "zbc123looooooooooong line", "abc123looooooooooong line", "abd123looooooooooong line",
  "bbd123looooooooooong line"
]

This is a simple list, so the short form is used - the question then becomes, should trailing comma be used for short-form lists, and when?

Archargelod commented 4 months ago

@arnetheduck, sorry I didn't test this particular example, here's one that should format into the long form on v0.4.1:

let arr = [
  "string over 40 characters long___________",
  "string over 40 characters long___________",
]

One line collections doesn't benefit from trailing comma much. It also looks a bit out of place:

let shortList = ["foo", "bar", "baz",]

Multi-line short form, I believe, should be consistent with the long form:

var arr = [
  "zbc123looooooooooong line", "abc123looooooooooong line", "abd123looooooooooong line",
  "bbd123looooooooooong line",
]
arnetheduck commented 4 months ago

Your first example is actually a multi-line short form given that all entries are simple (even if they all happen to be long such that only one fits per line).

For long form, it's actually a no-brainer, but the extra comma does end up looking a bit out of place for multi-line short forms, ie:

var arr = functionacall(
  "zbc123looooooooooong line", "abc123looooooooooong line", "abd123looooooooooong line",
  "bbd123looooooooooong line",
)

let x = a in {
  "zbc123looooooooooong line", "abc123looooooooooong line", "abd123looooooooooong line",
  "bbd123looooooooooong line",
}
arnetheduck commented 4 months ago

Need to think a bit more about the short form - reopening for now but feel free to close as well.

Archargelod commented 4 months ago

For reference, I tried this feature in other opinionated formatters, mentioned in the Readme:

prettier:

black:

clang-format:

arnetheduck commented 4 months ago

prettier

prettier does the "short-list" trick for numbers (not for strings / identifiers though).

They do add a trailing in this case.