I18n support

[tstrachota]

• Heading in HelpBuilder extracted to a function to make overriding easier.
• Translation table for overriding messages. Usage is:

Clamp.messages = {
  :too_many_arguments => "..."
}

[tstrachota]

@mdub ping?

[mmoll]

what's the status here?

[mdub]

The status is that I've been a crap maintainer, and haven't been able to find/make time to even review @tstrachota's PR and provide feedback. Apologies all around, particularly to Tomas.

It looks like the PR delegates to a message method to generate most messages. Why is the usage_heading different?

It would help me if someone could provide a example use-case, e.g. a sample set of translated messages. @tstrachota?

Would it make sense for the translations to be maintained in the Clamp codebase? For instance, a user could

require 'clamp'
require 'clamp/i18n/pl'

to enable Polish messages.

[tstrachota]

@mdub thanks for looking at the PR again. Example usage is here:
https://github.com/theforeman/hammer-cli/blob/master/lib/hammer_cli/clamp.rb
We already distribute Clamp with my patch in rpm and deb packages.

I made usage_heading different because I expected people to override whole Help::Builder in case they want to change output there. But it's a good idea to provide more convenient way for changing just the language. Please see my second commit which fixes that.

Tests are currently failing but that's happening also in the master. RSpec changed it's interface with new releases and there's no version restriction in the Gemfile. Fixing tests was easy, so I went that way: #49

Regarding the translations being maintained on the Clamp's side, I intentionally avoided that for following reasons:

• users can use i18n library of their choice (there will probably be more strings to translate in the application that uses Clamp that in the Clamp itself)
• no need to add another dependency
• maintaining translations would mean more work for Clamp's developers and we can easily let that on users that build on the top of Clamp and usually maintain their translations anyway

[jordansissel]

I like the overal idea of this patch, and I think it could be structured in a way that implies additional language support could be contributed to the project. I like that you want to allow consumers of clamp to provide their own translations in cases where Clamp has no translation (or a poor translation), but I think there's value in providing some kind of locale setting and having Clamp.messages tie the locale together to look up a certain translations.

[jordansissel]

I saw travis failed on this PR. I confirm the same when I run this PR locally, but most of the failures are unrelated.

I rebased against master, and the spec suite passes.

There aren't any tests for this PR. I'm willing to build on this patch and write tests for it as well as adding support for selecting locale and documenting how to add custom locale/translations.

Thoughts?

[jordansissel]

As a curiosity, is there any reason to avoid using the I18n ruby library for this?

[tstrachota]

@jordansissel rebased

As for your question: I tried to describe it in #43 (comment) Locale selection is also something that can happen outside clamp not locking users into some specific solution.

[pitr-ch]

@mdub Hi, would you consider giving @tstrachota permission to maintain and release this gem? It seems to me like a good option to keep this great lib moving forward. Clamp become key part of hammer-cli and distributing patched rpms and debs does not seem like a good solution. Anyway just a thought.

[mdub]

@pitr-ch Thanks for your suggestion, but (despite having dragged my feet on this PR) I'm not quite ready to hand over the reins yet.

I'm (finally) giving this a proper look now.

I like the simple "format" string stuff. Needs some tests around #format_string, though.

@tstrachota, the "hammer-cli" link you posted just duplicates the default translations, doesn't it? I was hoping for an example of translations into a different language.

Extracting the #usage_heading method seems unnecessary, as it (mostly) just delegates to a translation. Do you have a use-case (with example) of overriding it?

[tstrachota]

@mdub we have translations into about 10 languages. See https://github.com/theforeman/hammer-cli/tree/master/locale

Regarding the #usage_heading there's no need to extract it now. I can remove the method if you don't find it useful.

[pitr-ch]

@mdub just to clarify: I did not meant to hand it over but just allowing @tstrachota to be a co-maintainer to help out when you are too busy. Anyway thanks for the gem!

[mdub]

So, I've finally made time to look at this properly.

@tstrachota I've refactored it a bit, on a branch. Please let me know what you think, and in particular, whether anything I've done will break your usage in hammer-cli.

• https://github.com/mdub/clamp/compare/messages

We still need some tests before merging it in.

[tstrachota]

@mdub thanks for looking into it! Your changes look good. I like the extraction into a module.
I can have a look at adding some tests.

[mdub]

Excellent!

[tstrachota]

@mdub I added tests and some basic docs. Feel free to tune it.

[tstrachota]

Thanks, @mdub !!

[mmoll]

thanks! I guess a new release will be out soon? :)

[mdub]

@mmoll @tstrachota Indeed ... just released as "1.0.0"!