Re: [13] RFR 8217254, 8217721: CompactNumberFormat​() constructor does not comply with spec and format​() method spec for IAEx is not complaint

2019-03-07 Thread Nishit Jain
Thanks Naoto, Updated: http://cr.openjdk.java.net/~nishjain/8217254_8217721/webrev.01/ Regards, Nishit Jain On 06-03-2019 23:24, naoto.s...@oracle.com wrote: Hi Nishit, Just one comment on j.t.CompactNumberFormat.java. At line 425, Null check can be done at the top of the method

[13] RFR 8217254, 8217721: CompactNumberFormat​() constructor does not comply with spec and format​() method spec for IAEx is not complaint

2019-03-06 Thread Nishit Jain
() method was not compliant with the specification Fix: Updated the constructor and format method to throw exception as per the specification Regards, Nishit Jain

[13] RFR 8209175: Handle 'B' character introduced in CLDR 33 JDK update for Burmese (my) locale

2019-02-22 Thread Nishit Jain
e resolved with am/pm strings. This is based on the LDML specification given for 'B' character in which it falls back to 'a' character if it is not supported by any locale. Regards, Nishit Jain

Re: [13] RFR: CONFIG level logging statements printed in CLDRCalendarDataProviderImpl.java even when default log Level is INFO

2019-02-20 Thread Nishit Jain
Hi Naoto, Thanks for the explanation. Change looks fine to me. Regards, Nishit Jain On 19-02-2019 22:51, Naoto Sato wrote: Hi Nishit, The reason is that "US" is the only required locale in the JDK (cf. Locale.getAvailableLocales(). In fact, initially I supplied "001" w

Re: [13] RFR: CONFIG level logging statements printed in CLDRCalendarDataProviderImpl.java even when default log Level is INFO

2019-02-19 Thread Nishit Jain
"? Can't we just return "1" instead of setting the region to "US"? Regards, Nishit Jain On 16-02-2019 04:25, Naoto Sato wrote: Hello, Please review the fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8218960 The proposed changeset is located at: http

Re: RFR(XS):JDK-8219228: java/util/Base64/TestEncodingDecodingLength.java failing on 8GB test machine.

2019-02-19 Thread Nishit Jain
with error = "Not enough space". Since it is a memory intensive test, I think it is better and safe to increase "os.maxMemory" check Change looks good to me, I am not an openJDK reviewer though. Regards, Nishit Jain On 18-02-2019 18:19, Zeller, Arno wrote: Hello! I found tha

Re: [13] RFR 8217969, 8218265: Base64.Decoder.decode methods ... and java/util/Base64/TestEncodingDecodingLength.java failing

2019-02-05 Thread Nishit Jain
Thanks Roger, Naoto for reviewingthe changes Updated the webrev - Used ByteBuffer.wrap(inputBytes) instead of ByteBuffer.allocate(size) - Added Decoder.decode(ByteBuffer)method test. (line 58) http://cr.openjdk.java.net/~nishjain/8217969_8218265/webrev.01/ Regards, Nishit Jain On 05-02-2019 00

[13] RFR 8217969, 8218265: Base64.Decoder.decode methods ... and java/util/Base64/TestEncodingDecodingLength.java failing

2019-02-04 Thread Nishit Jain
as used only to test the overflow condition in Decoder.decode() which is no longer there. Regards, Nishit Jain

Re: [13] RFR: 8217609: New era placeholder not recognized by java.text.SimpleDateFormat

2019-01-29 Thread Nishit Jain
Hi Naoto, Look good to me. Regards, Nishit Jain On 26-01-2019 01:29, naoto.s...@oracle.com wrote: Hello, Please review the fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8217609 The proposed changeset is located at: http://cr.openjdk.java.net/~naoto/8217609/webrev.00

Re: [13] RFR: 8210583: Base64.Encoder incorrectly throws NegativeArraySizeException

2019-01-21 Thread Nishit Jain
Benchmark   Mode  Cnt Score  Error  Units MyBenchmark.testMethod    ss   50  2200.663 ± 1826.348  ms/op Regards, Nishit Jain On 19-01-2019 06:09, Joe Darcy wrote: How long does the regression test take to run on machines that have enough memory? Thanks, -Joe On 1/18/2019 3:38 PM

Re: [13] RFR: 8210583: Base64.Encoder incorrectly throws NegativeArraySizeException

2019-01-21 Thread Nishit Jain
Thanks Roger, Updated. http://cr.openjdk.java.net/~nishjain/8210583/webrev.05/ Regards, Nishit Jain On 18-01-2019 21:13, Roger Riggs wrote: Hi Nishit, Looks good, with a minor fix. ok, the rationale for MAX_VALUE-2 make sense. TestEncodingDecodingLength: Line 61 and 68,   The error message

Re: [13] RFR: 8210583: Base64.Encoder incorrectly throws NegativeArraySizeException

2019-01-18 Thread Nishit Jain
low Integer.MAX_VALUE, so to test the added OOME condition need to use Integer.MAX_VALUE - 2. Updated the webrev with rest of the suggestions http://cr.openjdk.java.net/~nishjain/8210583/webrev.04/ Regards, Nishit Jain On 18-01-2019 02:44, Naoto Sato wrote: I agree with 'withOutputParam' comment. It does s

Re: [13] RFR: 8216969: ParseException thrown for certain months with russian locale

2019-01-18 Thread Nishit Jain
Looks Good. Regards, Nishit Jain On 17-01-2019 22:07, Naoto Sato wrote: Hi Nishit, Thanks. Updated: http://cr.openjdk.java.net/~naoto/8216969/webrev.01/ Naoto On 1/17/19 2:57 AM, Nishit Jain wrote: Hi Naoto, Looks good to me. Just a small suggestion. - To improve readability, can we

Re: [13] RFR: 8216969: ParseException thrown for certain months with russian locale

2019-01-17 Thread Nishit Jain
Hi Naoto, Looks good to me. Just a small suggestion. - To improve readability, can we declare "standalone mask" (0x8000) as a static field and use that at all the places? Regards, Nishit Jain On 17-01-2019 05:50, naoto.s...@oracle.com wrote: Hi, Please review the fix to the follo

Re: Compact Number Formatting and Fraction Digits

2019-01-17 Thread Nishit Jain
to introduce an API which if set, truncates trailing fractional zeros while formatting output. This may need some thought process on its feasibility. Regards, Nishit Jain On 17-01-2019 14:37, Gunnar Morling wrote: Hi, I took a look at the compact number formatting recently added in JDK 12

[13] RFR: 8210583: Base64.Encoder incorrectly throws NegativeArraySizeException

2019-01-16 Thread Nishit Jain
. There is an unrelated change in encodeToString(byte[]) where a string instance is created using JavaLangAccess.newStringNoRepl(byte[], ISO_8859_1)instead of string constructor, to save memory. Regards, Nishit Jain

Re: [12] RFR: 8215303: Allowing additional currency code points from later Unicode updates

2019-01-04 Thread Nishit Jain
Changes looks fine to me. Regards, Nishit Jain On 03-01-2019 22:26, Naoto Sato wrote: Hello, Please review the fix to the following issue (and its approved CSR): https://bugs.openjdk.java.net/browse/JDK-8215303 https://bugs.openjdk.java.net/browse/JDK-8215305 The proposed changeset

[12] RFR JDK-8214935: Upgrade IANA LSR data

2018-12-07 Thread Nishit Jain
://bugs.openjdk.java.net/browse/JDK-8214935 Webrev: http://cr.openjdk.java.net/~nishjain/8214935/webrev.00/ Regards, Nishit Jain

Re: RFR 8177552: Compact Number Formatting support

2018-12-04 Thread Nishit Jain
Thanks Roger, Updated Webrev: http://cr.openjdk.java.net/~nishjain/8177552/webrevs/webrev.06/ Regards, Nishit Jain On 04-12-2018 00:58, Roger Riggs wrote: Hi Nishit, Thanks for the updates and cleanup. CompactNumberFormat.java: - 827: To locate a single character use

Re: RFR 8177552: Compact Number Formatting support

2018-12-03 Thread Nishit Jain
* the prefix/suffix is longer than the suffix already compared? Yes, I think this can be done. Updated. 1721:  IntelliJ observes that if (gotNeg) is redundant since 1708 rules out them being both true or both false. Updated Regards, Nishit Jain Thanks, Roger On 11/28/18 3:46 AM, Nis

[12] RFR 8213294: Upgrade IANA LSR data

2018-11-29 Thread Nishit Jain
Hi, Please review the fix for 8213294, which upgrades the IANA LSR data to the latest available version. Bug: https://bugs.openjdk.java.net/browse/JDK-8213294 Webrev: http://cr.openjdk.java.net/~nishjain/8213294/webrev.00 Regards, Nishit Jain

Re: RFR 8177552: Compact Number Formatting support

2018-11-28 Thread Nishit Jain
Thanks Naoto, Updated. http://cr.openjdk.java.net/~nishjain/8177552/webrevs/webrev.04/ Regards, Nishit Jain On 27-11-2018 20:55, naoto.s...@oracle.com wrote: Hi Nishit, On 11/26/18 11:11 PM, Nishit Jain wrote: Hi Naoto, On 26-11-2018 21:01, naoto.s...@oracle.com wrote: Hi Nishit, On 11

Re: RFR 8177552: Compact Number Formatting support

2018-11-26 Thread Nishit Jain
Hi Naoto, On 26-11-2018 21:01, naoto.s...@oracle.com wrote: Hi Nishit, On 11/26/18 12:41 AM, Nishit Jain wrote: Hi Naoto, To add to my previous mail comment, the DecimalFormat spec also says that /*"DecimalFormat can be instructed to format and parse scientific notation onl

Re: RFR 8177552: Compact Number Formatting support

2018-11-26 Thread Nishit Jain
m aware of, unless you are pointing to the issue like JDK-8211161, which we know is not an issue. Regards, Nishit Jain On 23-11-2018 15:55, Nishit Jain wrote: Hi Naoto, > I think DecimalFormat and CNF should behave the same, ie. 'E' should be treated as the exponent without a quote. Personally I do

Re: RFR 8177552: Compact Number Formatting support

2018-11-25 Thread Nishit Jain
Hi Roger, Please find my comments belowand check the updated webrev. http://cr.openjdk.java.net/~nishjain/8177552/webrevs/webrev.02/ Regards, Nishit Jain On 22-11-2018 00:04, Roger Riggs wrote: Hi Nishit, Comments on the tests: - The tests looks to be quite complete. - Have the locale

Re: RFR 8177552: Compact Number Formatting support

2018-11-23 Thread Nishit Jain
rs are needed it should be done by DecimalFormat scientific instance *not *with the general number instance.So, I don't think that we should allow parsing of exponential numbers.Comments welcome. Regards, Nishit Jain On 22-11-2018 02:02, naoto.s...@oracle.com wrote: Hi Nishit, On 11/21/18 12:5

Re: RFR 8177552: Compact Number Formatting support

2018-11-21 Thread Nishit Jain
duplication. - Also updated it with other changes as suggested in the comments Regards, Nishit Jain On 20-11-2018 00:33, naoto.s...@oracle.com wrote: Hi Nishit, On 11/18/18 10:29 PM, Nishit Jain wrote: Hi Naoto, Please check my comments inline. On 17-11-2018 04:52, naoto.s...@oracle.com wrote: Hi

Re: RFR 8177552: Compact Number Formatting support

2018-11-20 Thread Nishit Jain
be used to to get the customized CNF instance. Regards, Nishit Jain On 19-11-2018 19:42, Stephen Colebourne wrote: I'm not a big fan of having a class named `Style` as it is commonly used in business logic. (Yes, its an inner class, but I still think the potential for annoyance is high

Re: RFR 8177552: Compact Number Formatting support

2018-11-18 Thread Nishit Jain
assCastException may be thrown. The type is checked in the superclass equals method getClass() != obj.getClass(), so I think there is no need to check the type here. Regards, Nishit Jain Naoto On 11/16/18 9:54 AM, Nishit Jain wrote: Hi, Please review this non trivial feature addition to NumberFo

RFR 8177552: Compact Number Formatting support

2018-11-16 Thread Nishit Jain
va.net/browse/JDK-8188147 Request to please help review the the change. Regards, Nishit Jain

Re: [12]RFR 8210443: Migrate Locale matching tests to JDK Repo.

2018-09-24 Thread Nishit Jain
+1 On 24-09-2018 23:04, Naoto Sato wrote: Looks good to me. Naoto On 9/24/18 5:57 AM, Dora Zhou wrote: Hello, Please help review the fix for migrating GS-auto Locale matching tests to JDK repository. Please refer to the bug description for details. Bug:

[12] RFR 8208560: ChoiceFormat class has unused constants needs cleanup

2018-07-31 Thread Nishit Jain
access, but they are not getting used elsewhere in the package. Removed them from ChoiceFormat class. Regards, Nishit Jain

[12] RFR 8021322: [Fmt-Ch] Implementation of ChoiceFormat math methods should delegate to java.lang.Math methods

2018-07-26 Thread Nishit Jain
nextDouble(double), previousDouble(double), nextDouble(double, boolean) APIs implementation to delegate the execution to the equivalent Math APIs Regards, Nishit Jain

[12] RFR: 8193444: SimpleDateFormat throws ArrayIndexOutOfBoundsException when format contains long sequences of unicode characters

2018-07-12 Thread Nishit Jain
as 8-bit. Fix: The handling of long_length (> 254) for non-ASCII unicode characters is missing in the compilation phase of the format pattern, added that in the fix. Regards, Nishit Jain

Re: RFR JDK-8204938: Add a test case to automatically check the updated LSR data

2018-06-21 Thread Nishit Jain
Thanks Naoto, Roger for the review. Made the suggested changes and pushed to the repo. Regards, Nishit Jain On 21-06-2018 00:23, Roger Riggs wrote: Hi Nishit, Looks ok My only comment would be to put the relative path to the language-subtag-registry.txt in a private static final constant

RFR JDK-8204938: Add a test case to automatically check the updated LSR data

2018-06-20 Thread Nishit Jain
not be updated, it automatically cross checks the updated lsr data. Regards, Nishit Jain

RFR JDK-8203872: Upgrading JDK with latest available LSR data from IANA

2018-06-04 Thread Nishit Jain
Hi, Please review the fix for JDK-8203872. Bug: https://bugs.openjdk.java.net/browse/JDK-8203872 Webrev: http://cr.openjdk.java.net/~nishjain/8203872/webrev.01/ Fix: Updated the LSR data to the version 2017-08-15 Regards, Nishit Jain

[11] RFR JDK-8196399, JDK-8199672: Formatting a decimal using locale-specific grouping..., ClassCastException is thrown...

2018-03-20 Thread Nishit Jain
ping separator position identification is skipped. 8199672: Check if the instance returned is DecimalFormat; else, use DecimalFormat constructor to obtain the instance. Regards, Nishit Jain

Re: [11] RFR JDK-8060094: java/util/Formatter/Basic.java failed in tr locale

2018-02-23 Thread Nishit Jain
Thanks Naoto, Please check the updated webrev http://cr.openjdk.java.net/~nishjain/8060094/webrev.04/ Edits made: In FormatLocale.java, clarified the exception messages about the locale used and removed an unused import. Regards, Nishit Jain On 23-02-2018 00:56, Naoto Sato wrote: Hi Nishit

[11] RFR JDK-8060094: java/util/Formatter/Basic.java failed in tr locale

2018-02-22 Thread Nishit Jain
when specified during instance creation or during format() call. The default locale was getting used irrespective of whether a Locale is specified in the API. Fix: Modified the APIs to use the locale specified. Regards, Nishit Jain

[11] RFR JDK-8190904: Incorrect currency instance returned by java.util.Currency.getInstance()

2018-02-16 Thread Nishit Jain
minor unit. These are ignored as inconsistent entries. Also, If there is any ISO 4217 currency data with same currency code as the currency entry given in the properties file, then the existing Currency data should be updated with the given currency values. Regards, Nishit Jain

[10]RFR 8190278: ClassCastException is thrown by java.util.Scanner when a NumberFormatProvider is used.

2017-12-11 Thread Nishit Jain
, if not,      then use the DecimalFormat constructor way to create its object, where SPI provider implementation is ignored. Regards, Nishit Jain

Re: [10] RFR 8187551: MessageFormat.setFormat(int, Format) AIOOBE not thrown when documented

2017-12-03 Thread Nishit Jain
Thanks Roger, Updated the webrevto add the new test case in MessageRegression.java http://cr.openjdk.java.net/~nishjain/8187551/webrev.03/ Regards, Nishit Jain On 01-12-2017 20:40, Roger Riggs wrote: Hi Nishit, Please add the new test to test/jdk/java/text/Format/MessageFormat

[10] RFR 8187551: MessageFormat.setFormat(int, Format) AIOOBE not thrown when documented

2017-12-01 Thread Nishit Jain
for filing the issue and suggesting the fix. Regards, Nishit Jain

Re: [10] RFR 6354947: [Fmt-*] DecimalFormat ignores FieldPosition settings on input, contrary to Javadocs

2017-11-29 Thread Nishit Jain
per javadoc or the javadoc will be changed. > Please change the issue summary (on both the CSR and the issue) to reflect the true cause. Done Regards, Nishit Jain On 29-11-2017 20:35, Roger Riggs wrote: Hi Nishit, The webrev and updates to the javadoc look fine. However, the summary of the

Re: [10] RFR 8191404: Upgrading JDK with latest available LSR data from IANA.

2017-11-22 Thread Nishit Jain
eparate issue. I would like to bring one point to notice that, I have not added all the new tags (which are added in this version) in the test case, just few of them are added to test that LocaleEquivalentMaps are properly updated. Hope that is fine. Regards, Nishit Jain On 22-11-2017 04:47,

[10] RFR 6354947: [Fmt-*] DecimalFormat ignores FieldPosition settings on input, contrary to Javadocs

2017-11-16 Thread Nishit Jain
APIs specification. Regards, Nishit Jain

[10] RFR 8191404: Upgrading JDK with latest available LSR data from IANA.

2017-11-16 Thread Nishit Jain
Hi, Please review the fix for JDK-8191404 Bug: https://bugs.openjdk.java.net/browse/JDK-8191404 Webrev: http://cr.openjdk.java.net/~nishjain/8191404/webrev.00/ Fix: Updated the LSR data with the version: 2017-08-15 Regards, Nishit Jain

Re: [10] RFR 8180469: Wrong short form text for supplemental Japanese era

2017-08-31 Thread Nishit Jain
Looks good to me. I am not a reviewer, though. Regards, Nishit Jain On 31-08-2017 04:25, Naoto Sato wrote: Hi, Please review the fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8180469 The proposed changeset is located at: http://cr.openjdk.java.net/~naoto/8180469

Re: [10] RFR JDK-8186713: Document default rounding mode in NumberFormat

2017-08-29 Thread Nishit Jain
Thanks Brian, Naoto for the review. The below updated patch is pushed to the repository. http://cr.openjdk.java.net/~nishjain/8186713/webrev.02/ Regards, Nishit Jain On 29-08-2017 04:56, Brian Burkhalter wrote: Hi Nishit, I suggest these changes in NumberFormat.java: 184:delete the line 186

Re: [10] RFR JDK-8186713: Document default rounding mode in NumberFormat

2017-08-28 Thread Nishit Jain
this is fine. Also updated the webrev with other suggested changes. http://cr.openjdk.java.net/~nishjain/8186713/webrev.01/ Regards, Nishit Jain On 25-08-2017 00:55, Brian Burkhalter wrote: A few minor comments: L178-195: Is a list necessary here? L186: I don’t think “Rounding: " is n

[10] RFR JDK-8186713: Document default rounding mode in NumberFormat

2017-08-24 Thread Nishit Jain
used while formatting the numbers. Also, in the specification of java.util.Formatter, changed the reference of the deprecated BigDecimal.ROUND_HALF_UP,with java.math.RoundingMode.HALF_UP. Regards, Nishit Jain