> On Feb. 2, 2011, 8:56 a.m., Kent Quirk wrote: > > So I have a couple of issues with this patch: > > > > * I'm worried about the translation impact, how these strings are used and > > where. Should they be localized? Should they not be localized? I find it > > hard to believe that we ONLY need to translate Home and End. > > * The "1" key is duplicated. Are there others? > > * I think the single-letter keys should be placed in a more useful order so > > that we can easily find missing items. Keyboard order (especially since > > keyboards differ) is not good enough. > > * I'm worried about the naming convention; single-letter names (ahem, yes, > > I know who's talking here) can be rather ambiguous. It's probably the > > easiest fix -- anything else may require much more extensive changes -- but > > I'm worried about this. Is this fix robust under localization? > > > > I haven't had time to investigate these issues yet; does someone else have > > the knowledge to discuss? > > > > Vadim ProductEngine wrote: > 1) AFAIU, only those keys need translation that are currrently used in > menu shortcuts (see menu_*.xml). > We don't know in advance what keys we're gonna use in future > shortcuts, so it makes sense to make them all localizable. > 2) Thanks, the "1" is indeed duplicated. Looks like a copy&paste issue. > 3) Ok, the order doesn't matter to me. I can change it if you want. > 4) I borrowed the naming style from existing code (see llkeyboard.cpp). > Moreover, we tried adding "Key_" prefix to key names for the same reasons as > you say, but had to roll that change back because it had broken translation > of older shortcuts (see comments in STORM-362).
I would like to see a revised changeset please, reflecting 2 and 3. I'm happy with your responses to 1 and 4. - Kent ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/108/#review307 ----------------------------------------------------------- On Jan. 19, 2011, 8:30 a.m., Vadim ProductEngine wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://codereview.secondlife.com/r/108/ > ----------------------------------------------------------- > > (Updated Jan. 19, 2011, 8:30 a.m.) > > > Review request for Viewer. > > > Summary > ------- > > Made all keys localizable. > > > This addresses bug STORM-465. > http://jira.secondlife.com/browse/STORM-465 > > > Diffs > ----- > > indra/newview/skins/default/xui/en/strings.xml 38465c40c060 > > Diff: http://codereview.secondlife.com/r/108/diff > > > Testing > ------- > > Tested on Linux. No keys produce the warning for me. > > > Thanks, > > Vadim > >
_______________________________________________ Policies and (un)subscribe information available here: http://wiki.secondlife.com/wiki/OpenSource-Dev Please read the policies before posting to keep unmoderated posting privileges
