Add comment on kwargs to avoid new people open issues like #500

[alchimere]

I opened the issue #500 because of warnings during testing my app with ruby-2.7 but I just realized nothing can be done in the method itself to fix this.

So my proposal is to add a comment that explain that.

This way other developers that will face the same problem will understand sooner quicker than me

[knapo]

I think you missed an important use case, when other keyword arguments are passed:

#ruby 2.6
I18n.translate(:salutation, throw: true, name: 'Joe')
# ruby 2.7
I18n.translate(:salutation, throw: true, **{ name: 'Joe' })` 

The method was not designed to be used in 2.7 and its 2.7 syntax is confusing.

I believe the idea behind having options as keyword argument, was to make method usage simple and straightforward and in 2.7 we can't really achieve this. If we look on 2.7 only, it's easier to change options to be a hash argument, but we'll brake the current api.

I think the only was to keep it working in 2.7 is having one hash argument, something like:

 def translate(key = nil, options = {})
   throw  = options.delete(:throw) || false
   raise  = options.delete(:raise) || false
   locale = options.delete(:locale) || config.locale
   ...
end

@radar what do you think ?

[alchimere]

@knapo

I think you missed an important use case, when other keyword arguments are passed:

#ruby 2.6
I18n.translate(:salutation, throw: true, name: 'Joe')
# ruby 2.7
I18n.translate(:salutation, throw: true, **{ name: 'Joe' })` 

The method was not designed to be used in 2.7 and its 2.7 syntax is confusing.

I don't understand your comment, I18n.translate(:salutation, throw: true, name: 'Joe') still works using the same way ruby 2.7, you don't have to wrap in a hash before splatting it into kwargs (**{ key: :val } is strictly the same as key: :val without braces)

Concrete example you can copy/paste in different versions to test:

def meth(key = nil, *, throw: false, **options)
  puts({ key: key, throw: throw, options: options }.to_json)
end

meth(:plop, throw: true, foo: 'bar')
# prints {"key":"plop","throw":true,"options":{"foo":"bar"}}
# same result using both ruby 2.6 and 2.7 without any warning

meth(:plop, throw: true, **{foo: 'bar'})
# exactly same behavior as above, still without any warning

meth(:plop, { throw: true, foo: 'bar' })
# same behavior BUT 2.7 prints a warning

So it doesn't require to break the current api but just think to call the double splat when using a hash in a variable (because if you don't use a variable you'll just skip the brackets and keep compatibility with ruby 3 kwargs without any change)

[knapo]

@alchimere ah, sorry, you're right :)

[radar]

Perhaps it is time to drop support for any Ruby version without keyword argument support. I’ll mull it over.

[alchimere]

Perhaps it is time to drop support for any Ruby version without keyword argument support. I’ll mull it over.

I don't understand what you mean. Keyword arguments have been introduced in ruby since 2.0 (2.1 for keyword arguments without default value) and the current version of i18n require ruby 2.3+ so the support for ruby versions without kwargs support is already dropped.

Also, the current code already uses kwargs in it's parameters, so it already doesn't support ruby version without keyword argument support.

I don't think there is anything to change in this library. The point of this MR is to add a comment to help people that will soon migrate to ruby 2.7 to easily understand why exactly they have this warning and what they should do to avoid it.

Given this library is used by a lot projects I hope this might help some of them migrate easier to ruby 2.7.

[radar]

Ok, now that I have packed my holiday brain away... you can ignore the above comment.

I think a comment on the method is fine for the time being.

@knapo Breaking the main API that everyone uses for this library would be a great way to annoy basically the entire world, so let's not do that.

I'll merge this PR now :)