Updated (13) webrev looks fine (link above is to webrev 12). Stephen
On 21 December 2016 at 13:24, nadeesh tv <nadeesh...@oracle.com> wrote: > Hi Roger & Stephen, > > Please see the updated webrev > http://cr.openjdk.java.net/~ntv/8145633/webrev.13/ > > > On 12/21/2016 3:11 AM, Roger Riggs wrote: > > Hi Nadeesh, > > On 12/20/2016 2:34 PM, nadeesh tv wrote: > > > Hi Roger & Stephen , > Thanks for the comments. > > Please see the updated webrev > http://cr.openjdk.java.net/~ntv/8145633/webrev.12/ > > Changes included : > > 1. Doc changes and cosmetic changes suggested by Roger (except the note > about delay..) > > The comments at 4776-4779 are fine. > The issue came from passing null at line 4809 to the NumberPrinterParser > constructor > that expects the field argument to be non-null, and wanting some explanation > of that disconnect. > > > Changes included: Added the following clarification. > > 4775 * Prints or parses a localized pattern from a localized field. > 4776 * The specific formatter and parameters is not selected until the > 4777 * the field is to be printed or parsed. > 4778 * The locale is needed to select the proper WeekFields from which > 4779 * the field for day-of-week, week-of-month, or week-of-year is > selected. > 4780 * Since Locale is available only during parsing or formatting, the > WeekBasedField > 4781 * will be null during construction. > > Is it OK? > > Thanks and Regards, > Nadeesh > > > Other than that, I'm fine with the changes. > > Roger > > > 2. Changed the behavior of 'e' to parse only 1 digit as suggested by > Stephen. Changed the existing test cases for this. > > Thanks and Regards, > Nadeesh > > On 12/20/2016 10:48 PM, Stephen Colebourne wrote: > > In the test provider_adjacentValuePatterns2(), please add > > {"YYYYwwe", Y, w, c, "20161201", 2016, 12, 1}, > > This should succeed, because a single number is all that is needed to > parse day-of-week. (So, it will need to be removed from the invalid > patterns test). > > Line 1869 will need to change to "count, count, count" to make the tests > pass. > > Otehrwise, looks fine, thanks. > Stephen > > > On 20 December 2016 at 09:55, nadeesh tv <nadeesh...@oracle.com> wrote: > > Hi all, > > BugId: https://bugs.openjdk.java.net/browse/JDK-8145633 > > Issue: Support adjacent value parsing for Localized Patterns > > Webrev: http://cr.openjdk.java.net/~ntv/8145633/webrev.10/ > > Pattern 'c' and 'W' were previously allowed to have 'zero padding' which > was not explicitly mentioned in CLDR > (http://unicode.org/reports/tr35/tr35-dates.html). > To allow 'c' and 'W' to take part in adjacent value parsing ( at the same > time, 2 digits are not required for these patterns), restricted the max > width of these patterns to 1. > > Special thanks to Stephen for the help. > > -- > Thanks and Regards, > Nadeesh TV > > > > > -- > Thanks and Regards, > Nadeesh TV >
