Conversation
|
Could you include some docs in the PR? It's unclear to me how a user of my service would specify their language and locality preference order (such as pt_BR, pt, en), and how those would be used at runtime. It's also unclear how this will work for non-LTR languages, or for mixing LTR and non-LTR info. |
| }) ~~ /'Your Name'/; | ||
|
|
||
| is get-response(route { | ||
| # XXX We currently fuzzy-match `en` and `en-XX`, should we really? |
There was a problem hiding this comment.
Is their precedent for doing this elsewehre?
| # TODO q sort | ||
| # TODO move this to a request method | ||
| $header.split(',')>>.trim.map(*.split(';')[0].trim) |
There was a problem hiding this comment.
Or at least Cro::HTTP::Request should have something like quality-header that does this
There was a problem hiding this comment.
Well, are other headers parsed this way?
There was a problem hiding this comment.
I believe all the Accept* headers can use this.
| # XXX We currently fuzzy-match `en` and `en-XX`, should we really? | ||
| load-translation-file('main', 't/resources/main.po', :language<en en-GB en-US>); | ||
| load-translation-file('main', 't/resources/main-fr.po', :language<fr fr-FR fr-CH>); | ||
| _-prefix 'main'; |
There was a problem hiding this comment.
Hm, this looks delightfully odd. I'm tempted to suggest translation-prefix, but then it's maybe less clear that it's about the _ function...perhaps it's fine.
There was a problem hiding this comment.
Definitely To Be Nitpicked™.
| } | ||
|
|
||
| #| Defines the prefix to use for translations using this form | ||
| method i18n-prefix(--> Str) { Str:U } |
There was a problem hiding this comment.
I wonder if this shoulda been done as a trait on the form class for consistency with it being an attribute trait on the fields.
There was a problem hiding this comment.
I considered it as well, I can change it.
Altai-man
left a comment
There was a problem hiding this comment.
The POFile module is used, but is not specified in dependencies? Also new module files are introduced, but not added to META6.json, this will lead to test failures if tested with zef test ., which is what we must pass.
|
|
||
| #| Load a translation file and store it with a given prefix and a given (set of) language | ||
| sub load-translation-file(Str:D $prefix, $file, :language(:languages(@languages))) is export { | ||
| my $pofile = POFile.load($file); |
There was a problem hiding this comment.
This probably wants some defensive CATCH.
| } | ||
|
|
||
| sub match-language(Str @languages, Str $accept --> Int) { | ||
| if +@languages && $accept.defined { |
There was a problem hiding this comment.
Is taking the length here really necessary? Empty positionals are boolified as False.
| try { request.annotations<language> } // Str | ||
| } | ||
|
|
||
| # TODO move this to Request |
There was a problem hiding this comment.
Either do or remove the TODO?
No description provided.