Opened 7 years ago

Closed 7 years ago

#6859 closed (fixed)

Javascript i18n doc errors

Reported by: ramiro Owned by: nobody
Component: Documentation Version: master
Severity: Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Attachments (2)

js-i18n-doc.diff (2.5 KB) - added by ramiro 7 years ago.
js-i18n-doc.2.diff (3.8 KB) - added by ramiro 7 years ago.

Download all attachments as: .zip

Change History (6)

Changed 7 years ago by ramiro

comment:1 follow-up: Changed 7 years ago by julien

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Thanks for that. I've spotted another glitch in the doc...

The generated interpolate function is like that:

function interpolate(fmt, obj, named) {

if (named) {

return fmt.replace(/%\(\w+\)s/g, function(match){return String(obj[match.slice(2,-2)])});

} else {

return fmt.replace(/%s/g, function(match){return String(obj.shift())});

}

}

The 'named' argument should be a boolean indicating whether or not you want to use a dictionary instead of a list of name-less arguments.

However, in the doc, the given example is:
d = {

count: 10

};
s = interpolate(ungettext('this is %(count)s object', 'this are %(count)s objects', d.count), d);

It's missing a 'true' argument, and should instead be as follows:
d = {

count: 10

};
s = interpolate(ungettext('this is %(count)s object', 'this are %(count)s objects', d.count), d, true);

Maybe also adding a mention in the doc like: "In the example above, the argument 'true' indicates that a named interpolation should be considered, instead of a positional one.".

I would have amended your patch but it won't apply entirely (the 'ungettext' are still there after application of the patch). Do you want to amend your patch yourself and post it again?

Cheers!

comment:2 in reply to: ↑ 1 ; follow-up: Changed 7 years ago by ramiro

Replying to julien:

The 'named' argument should be a boolean indicating whether or not you want to use a dictionary instead of a list of name-less arguments.
However, in the doc, the given example is [...] missing a 'true' argument

I'm attaching a new revision of the patch that adds a refactoring of the documentation fot the javascript_catalog view JavaScrpt i18n funtions to take all these suggestions in account. Thanks!.

Also, will close #6355 as a dupe, it's already accepted but this ticket also fixes other problems in the same area of the documentation and has a patch.

Changed 7 years ago by ramiro

comment:3 in reply to: ↑ 2 Changed 7 years ago by anonymous

Thanks, great patch! Just a tiny remark. You mention about the *fmt* argument, and then in the example you use "fmts". I know this has no impact on the code at all, but maybe keeping "fmt" all the way through would prevent some confusion.

Replying to ramiro:

Replying to julien:

The 'named' argument should be a boolean indicating whether or not you want to use a dictionary instead of a list of name-less arguments.
However, in the doc, the given example is [...] missing a 'true' argument

I'm attaching a new revision of the patch that adds a refactoring of the documentation fot the javascript_catalog view JavaScrpt i18n funtions to take all these suggestions in account. Thanks!.

Also, will close #6355 as a dupe, it's already accepted but this ticket also fixes other problems in the same area of the documentation and has a patch.

comment:4 Changed 7 years ago by mtredinnick

  • Resolution set to fixed
  • Status changed from new to closed

(In [7357]) Fixed #6859 -- Greatly cleaned up the section on i18n pluralization in
Javascript. With new example code and everything! Thanks, Ramiro Morales.

Note: See TracTickets for help on using tickets.
Back to Top