Letters "Q", "q", "M", "L", "d", "D", "F", "h", "H", "k", "K", "m", "s" and no doubt others use NORMAL via
appendValue(field); Changing these to use NOT_NEGATIVE would be a big change and doesn't seem justified. For "D", "DD" and "DDD" these seem to be the best balance (as discussed up-thread): * D 1 appendValue(ChronoField.DAY_OF_YEAR) * DD 2 appendValue(ChronoField.DAY_OF_YEAR, 2, 3, SignStyle.NOT_NEGATIVE) * DDD 3 appendValue(ChronoField.DAY_OF_YEAR, 3) ie. we use NOT_NEGATIVE when we are making changes to a field and that is the best option, but otherwise we stick with NORMAL for single letter patterns like "D". Stephen On 3 May 2016 at 15:22, Roger Riggs <roger.ri...@oracle.com> wrote: > Hi Nadeesh, > > > On 5/3/2016 3:24 AM, nadeesh tv wrote: > > Hi Roger, > Please see the answers inline > > On 5/3/2016 2:43 AM, Roger Riggs wrote: > > Hi Nadeesh, > > src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java > > line 1835: the appendValue(field) method has a sign-style of NORMAL; should > that be NOT_NEGATIVE? > > It's 'NOT_NEGATIVE' only. May be a browser caching issue. Can you please > check it once again. > > My question was about the existing case for "D" that uses > appendValue(field). > That is a shorthand for: > appendValue(new NumberPrinterParser(field, 1, 19, SignStyle.NORMAL)); > > test/java/time/tck/java/time/format/TCKDateTimeFormatterBuilder.java: > > What is being tested in test_dayOfYearFieldPattern? > It does not check that the value parsed matches the input. > > These test cases added initially. Now it's become almost redundant due to > 'dayOfYearFieldValues' . I could remove it. > > If they are redundant, they should be removed. > > > test/java/time/test/java/time/format/TestDateTimeFormatterBuilder.java > > 686, 688: should these cases show with sign-style NOT_NEGATIVE also? > > Suspecting browser caching here also. > > I'm referring to the test case for "D", which seems like it should show up > as NOT_NEGATIVE. > > > Updated the webrev by removing some unnecessary spaces > http://cr.openjdk.java.net/~ntv/8079628/webrev.03/ > > > Thanks, Roger >
