trailing commas in multi-line method arguments
alanhamlett opened this issue · 7 comments
Which is the preferred style?
# 1
return I18n.t('Guest Profile Page: A note for the Host saying the potential Guest has '\
'not cancelled a reservation since joining Airbnb.',
:default => '%{guest_name} hasn’t canceled a reservation since joining '\
'Airbnb',
:guest_name => @user.smart_name)
or
# 2
return I18n.t('Guest Profile Page: A note for the Host saying the potential Guest has '\
'not cancelled a reservation since joining Airbnb.',
:default => '%{guest_name} hasn’t canceled a reservation since joining '\
'Airbnb',
:guest_name => @user.smart_name,
)
or
# 3
return I18n.t(
'Guest Profile Page: A note for the Host saying the potential Guest has '\
'not cancelled a reservation since joining Airbnb.',
:default => '%{guest_name} hasn’t canceled a reservation since joining '\
'Airbnb',
:guest_name => @user.smart_name,
)
or
# 4
return I18n.t(
'Guest Profile Page: A note for the Host saying the potential Guest has '\
'not cancelled a reservation since joining Airbnb.',
:default => '%{guest_name} hasn’t canceled a reservation since joining '\
'Airbnb',
:guest_name => @user.smart_name
)
We currently don't have a rule for trailing commas in multi-line-method calls.
# good
return I18n.t('Guest Profile Page: A note for the Host saying the potential Guest has '\
'not cancelled a reservation since joining Airbnb.',
:default => '%{guest_name} hasn’t canceled a reservation since joining '\
'Airbnb',
:guest_name => @user.smart_name)
# bad
return I18n.t('Guest Profile Page: A note for the Host saying the potential Guest has '\
'not cancelled a reservation since joining Airbnb.',
:default => '%{guest_name} hasn’t canceled a reservation since joining '\
'Airbnb',
:guest_name => @user.smart_name,
)
# good
return I18n.t(
'Guest Profile Page: A note for the Host saying the potential Guest has '\
'not cancelled a reservation since joining Airbnb.',
:default => '%{guest_name} hasn’t canceled a reservation since joining '\
'Airbnb',
:guest_name => @user.smart_name,
)
# Also good
return I18n.t(
'Guest Profile Page: A note for the Host saying the potential Guest has '\
'not cancelled a reservation since joining Airbnb.',
:default => '%{guest_name} hasn’t canceled a reservation since joining '\
'Airbnb',
:guest_name => @user.smart_name
)
Regarding the first argument, I think it's fine to not enforce one style here. It depends, e.g.
foo(argument1,
argument2)
looks totally fine, but if it's more like
really_long_name_including_multiple_namespaces(
argument1,
argument2
)
then that form is probably better.
I think the rationale that should apply is, when adding a new "last" item to a list (where the current last item can have an optional trailing comma), use whichever form results in the smallest git diff.
In @romanfuchs' examples, the former would not need one; the latter would.
If this rationale makes sense to others, then I think that we should mandate it.
I like the smallest diff rule, and personally prefer 1 and 3 over the other examples.
One thing to keep in mind is that in method definitions, a trailing comma is invalid ruby syntax.
Overall I don't have a strong preference on the trailing comma in method calls though.
Prior to trailing function commas being valid JS, the JS style guide's approach was simply "everywhere a trailing comma is allowed, where adding it keeps diffs small - use one" - using the same rationale here would cover invalid syntax.
Using a trailing comma without parentheses has to be forbidden, though. It will (and has) cause errors.