Please review -XshowSettings a java launcher option.

[Xueming Shen]

(1)Setting.java#43-#47 "assignment" need to be updated as well.
(2)LauncherHelper.java#189-190
      While it's unlikely we are going to have a line.separator with 
byte value
      negative, I would prefer still do b & 0xff before passing in for 
formatting,
      just in case:-)  And maybe it'd be better to simply let the 
j.u.Formatter do
      the formatting job for you?

       ostream.printf("0x%02X ", b & 0xff);

(3) prettyPrintLocales() needs a "println() at the end?

(4) OK, it's nit, but it's Friday night:-) so...
      a)any reason to use ":" after "default locale" and "=" after 
"available locales"?
      b)"lastline" is really "lastentry", and have to check it inside 
loop is really annoying,

          final int last = locales.length - 1;
          int i = 0;
          while (i < last) {
              ostream.print(locales[i++]);
              // print columns of 8
              if (i % 8 == 0) {
                  ostream.println(",");
                  ostream.print(INDENT + INDENT);
              } else {
                  ostream.print(", ");
              }
          }
          ostream.println(locales[i]);

(5)it's really really a nit, but I have to say:-) why "pretty...", try 
to hint only the "Locales"
and "Values" are prettily printed?

On 11/12/2010 12:17, Kumar Srinivasan wrote:
> Thanks for all the reviews!.
>
> Here are the fixes in this version:
> http://cr.openjdk.java.net/~ksrini/6452854/webrev.01
>
> 1. Fixed the representation of numbers and scaling using BigDecimal.
> 2. Argument processing checks for -XshowSettings and -XshowSetting:opt
>     (added a test to catch the former, added some comments in java.c)
> 3. Replaced String* with PrintStream.print*, it makes the logic easier
>     to read, so I decided to go with this.
> 4. Removed extraneous path.separators.
> 5. line.separator will be printed as ASCII symbols CR and LF
> 6. If the helper cannot determine the value that line will not be 
> displayed.
> 7. In the case of the Max memory not gotten from the launcher 
> (Estimated) is printed.
> 8. Removed the option hints -X* and so on in the labels.
> 9. Renamed Ergonomic Class -> Ergonomic Machine Class.
>
> Thanks
> Kumar
>
>
>
>
>> On 11/11/2010 16:42, Kumar Srinivasan wrote:
>>>> line 176, 188, 190-191, 195, and other lines in printPrintLocales 
>>>> and printLocale methods:
>>>> - the assignment to the buf and out variable to itself (returned 
>>>> from StringBuffer.append() method) is not necessary.
>>>
>>>
>>> Yes fixed, I missed these.
>>
>> The "intention" of returning the StringBuffer itself for those 
>> append() methods is that
>> you can then write the code like
>>
>>
>> private static void printLocale(PrintStream ostream) {
>>    ostream.println(new StringBuilder("\n" + LOCALE_SETTINGS + "\n")
>>                        .append(INDENT)
>>                       .append("default locale: ")
>>                       .append(Locale.getDefault().getDisplayLanguage())
>>                       .append(prettyPrintLocales())
>>                       .toString());
>> }
>>
>>
>> One more nit is that you might want to do something special for
>>
>>   line.separator =
>>
>> to make it "readable"
>>
>> -Sherman
>>
>