I'll add my reviewer-bit (let's give more good people reviewer bits!).
Looks good!
(but also demonstrates deep existing problems with substring change)
Not a defect with this change, but it looks to me like those
assert(false) will trigger if e.g. a number is specified that is
greater than Integer.MAX_VALUE.
2632 } catch (NumberFormatException x) {
2633 assert(false);
On Mon, Nov 17, 2014 at 4:29 AM, Aleksey Shipilev
<aleksey.shipi...@oracle.com> wrote:
> On 11/17/2014 03:23 PM, Claes Redestad wrote:
>>
>> On 2014-11-17 12:54, Aleksey Shipilev wrote:
>>>> Perhaps rewriting to something like this would make the code
>>>> cleaner:
>>>>
>>>> index(s, m.start(1), m.end(1));
>>>> flags(s, m.start(2), m.end(2));
>>>> width(s, m.start(3), m.end(3));
>>>> precision(s, m.start(4), m.end(4));
>>>> int tTStart = m.start(5);
>>>> if (tTStart >= 0) {
>>>> dt = true;
>>>> if (s.charAt(tTStart) == 'T') {
>>>> f.add(Flags.UPPERCASE);
>>>> }
>>>> }
>>>> conversion(s.charAt(m.start(6)));
>>> Yes please.
>>>
>>>> (I noticed that getting and checking tTEnd is basically redundant,
>>>> since the formatSpecifier regex guarantees either one char or
>>>> nothing is matched for that group)
>>> That makes sense.
>>
>> New webrev including the above cleanup:
>>
>> http://cr.openjdk.java.net/~redestad/8065070/webrev.01
>
> Good, thank you!
>
> -Aleksey.
>
>
