Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(18)

Issue 314650043: ICU support for UWP

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 month, 1 week ago by jefgen.msft
Modified:
1 month ago
Reviewers:
yoshito_umaoka, jefgen.msft, Shawn.Steele, shawnste, yoshito
CC:
aandrejs_microsoft.com
Base URL:
svn+ssh://source.icu-project.org/repos/icu/branches/jefgen/shawnste/uwp/
Visibility:
Public.

Description

ICU support for UWP

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+3281 lines, -213 lines) Patch
M source/allinone/allinone.sln View 2 chunks +110 lines, -22 lines 0 comments Download
A source/common/common_uwp.vcxproj View 1 chunk +1041 lines, -0 lines 0 comments Download
M source/common/locmap.h View 1 chunk +2 lines, -1 line 0 comments Download
M source/common/locmap.cpp View 37 chunks +227 lines, -43 lines 1 comment Download
M source/common/normlzr.cpp View 1 chunk +6 lines, -0 lines 0 comments Download
M source/common/putil.cpp View 11 chunks +232 lines, -16 lines 0 comments Download
M source/common/putilimp.h View 2 chunks +6 lines, -0 lines 0 comments Download
M source/common/ucln_imp.h View 1 chunk +2 lines, -0 lines 0 comments Download
M source/common/udata.cpp View 6 chunks +16 lines, -1 line 0 comments Download
M source/common/uloc.cpp View 1 chunk +14 lines, -0 lines 0 comments Download
M source/common/uloc_tag.cpp View 1 chunk +1 line, -1 line 0 comments Download
M source/common/umapfile.cpp View 4 chunks +31 lines, -1 line 0 comments Download
M source/common/umutex.h View 2 chunks +4 lines, -0 lines 0 comments Download
M source/common/unicode/chariter.h View 1 chunk +4 lines, -0 lines 0 comments Download
M source/common/unicode/platform.h View 1 chunk +12 lines, -0 lines 0 comments Download
M source/common/unicode/uchriter.h View 1 chunk +4 lines, -0 lines 0 comments Download
M source/common/unicode/uloc.h View 1 chunk +3 lines, -0 lines 1 comment Download
M source/common/unicode/uvernum.h View 1 chunk +5 lines, -0 lines 0 comments Download
M source/common/wintz.h View 2 chunks +4 lines, -2 lines 0 comments Download
M source/common/wintz.cpp View 3 chunks +5 lines, -2 lines 0 comments Download
M source/data/makedata.mak View 21 chunks +82 lines, -49 lines 0 comments Download
M source/data/makedata.vcxproj View 5 chunks +9 lines, -9 lines 0 comments Download
A source/data/makedata_uwp.vcxproj View 1 chunk +137 lines, -0 lines 0 comments Download
A source/i18n/i18n_uwp.vcxproj View 1 chunk +1020 lines, -0 lines 0 comments Download
M source/i18n/windtfmt.h View 2 chunks +3 lines, -2 lines 0 comments Download
M source/i18n/windtfmt.cpp View 7 chunks +99 lines, -10 lines 0 comments Download
M source/i18n/winnmfmt.h View 2 chunks +2 lines, -1 line 0 comments Download
M source/i18n/winnmfmt.cpp View 11 chunks +133 lines, -36 lines 0 comments Download
M source/i18n/wintzimpl.cpp View 1 chunk +2 lines, -0 lines 0 comments Download
M source/test/intltest/winutil.cpp View 2 chunks +2 lines, -0 lines 0 comments Download
M source/tools/pkgdata/pkgdata.cpp View 5 chunks +45 lines, -13 lines 0 comments Download
M source/tools/toolutil/pkg_genc.cpp View 2 chunks +18 lines, -4 lines 0 comments Download

Messages

Total messages: 5
jefgen.msft
1 month, 1 week ago (2017-03-18 00:55:05 UTC) #1
jefgen.msft
Testing out adding a comment on Rietveld. https://codereview.appspot.com/314650043/diff/1/source/common/unicode/uloc.h File source/common/unicode/uloc.h (right): https://codereview.appspot.com/314650043/diff/1/source/common/unicode/uloc.h#newcode544 source/common/unicode/uloc.h:544: * I ...
1 month, 1 week ago (2017-03-18 01:18:32 UTC) #2
yoshito
Reviewed all changes. I think you may use int32_t instead of int for a few ...
1 month ago (2017-03-21 21:07:13 UTC) #3
Shawn.Steele_microsoft.com
Thanks! I believe I entered a ticket to update locmap to our latest LCID mappings. ...
1 month ago (2017-03-21 21:28:12 UTC) #4
jefgen.msft
1 month ago (2017-03-21 21:50:30 UTC) #5
Indeed -- Thank you Yoshito for taking a look!

Aside, this is the ticket for updating the LCIDs:
http://bugs.icu-project.org/trac/ticket/12709
And there is also this ticket for removing/limiting any internal (to ICU)
usage of LCIDs: http://bugs.icu-project.org/trac/ticket/12712

Thanks again Yoshito! :)


On Tue, Mar 21, 2017 at 2:28 PM, Shawn Steele <Shawn.Steele@microsoft.com>
wrote:

> Thanks!
>
> I believe I entered a ticket to update locmap to our latest LCID
> mappings.  Hopefully we won't need it for Windows, but it would still be
> necessary for compatibility on other platforms - the catch is that we're
> trying not to add LCIDs for new locales, so the behavior is probably pretty
> stuck at this point.  People that really want full support should figure
> out how to move away from LCIDs.  (Something we can probably all agree on).
>
> Thanks for taking a look,
>
> Shawn
>
> -----Original Message-----
> From: y.umaoka@gmail.com [mailto:y.umaoka@gmail.com]
> Sent: Tuesday, March 21, 2017 2:07 PM
> To: jefgen.msft@gmail.com; Shawn Steele <Shawn.Steele@microsoft.com>;
> yoshito_umaoka@us.ibm.com
> Cc: Axel Andrejs <aandrejs@microsoft.com>; reply@codereview-hr.
> appspotmail.com
> Subject: Re: ICU support for UWP (issue 314650043 by jefgen.msft@gmail.com
> )
>
> Reviewed all changes. I think you may use int32_t instead of int for a few
> places for consistency. Otherwise, looks fine to me.
>
> In the past, we have been patching locmap.cpp when a new release of
> Windows came out.. Now it looks we don't need to keep it updated for ICU
> running on Windows (but we may still need the data updated for someone who
> wants to convert locale-LCID on other platforms).
>
> Thanks!
>
>
> https://codereview.appspot.com/314650043/diff/1/source/common/locmap.cpp
> File source/common/locmap.cpp (right):
>
> https://codereview.appspot.com/314650043/diff/1/source/
> common/locmap.cpp#newcode1119
> source/common/locmap.cpp:1119: int i = 0; int32_t for consistency
>
> https://codereview.appspot.com/314650043/
>
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 80a51fa-tainted