RFR: 8189102: All tools should support -?, -h and --help

[Lindenmaier, Goetz]

Hi Kumar, 

I'm already looking at the issues from your other mail.
Detailed comments will follow.

I'll also check and fix the new tests you report. 
I think we don't run the langtool tests, i.e. I know
we didn't run them before the repos were
merged.  Obviously we should add them to our testing :)

Thanks for further testing and best regards,
  Goetz.


> -----Original Message-----
> From: Kumar Srinivasan [mailto:kumar.x.srinivasan at oracle.com]
> Sent: Freitag, 1. Dezember 2017 15:42
> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
> Cc: core-libs-dev at openjdk.java.net; 'compiler-dev at openjdk.java.net'
> <compiler-dev at openjdk.java.net>; serviceability-dev (serviceability-
> dev at openjdk.java.net) <serviceability-dev at openjdk.java.net>
> Subject: Re: RFR: 8189102: All tools should support -?, -h and --help
> 
> Hi,
> 
> Besides my private comments to you, there are 2-3 internal tests
> which fail.
> 
> Have you run all the langtools tests ? There are 4 Windows tests
> that fail with langtools:
> 
> jdk/javadoc/doclet/testHelpOption/TestHelpOption.java
> jdk/javadoc/tool/CheckResourceKeys.java
> jdk/javadoc/tool/ToolProviderTest.java
> tools/javap/InvalidOptions.java
> tools/jdeps/MultiReleaseJar.java
> 
> This changeset needs to be vetted thoroughly using internal build and test
> systems.
> Any Oracle engineer willing to sponsor this ?
> 
> Kumar
> 
> 
> On 11/28/2017 3:16 AM, Lindenmaier, Goetz wrote:
> 
> 
> 	Hi,
> 
> 	I uploaded a new webrev:
> 	http://cr.openjdk.java.net/~goetz/wr17/8189102-
> helpMessage/webrev.04/
> 
> 	This includes the changes
> 	  - to jshell requested by Robert
> 	  - to the test as requested by Kumar.
> 
> 	See also incremental patch and the test output including all the
> 	help messages referenced in the webrev.
> 
> 	Best regards,
> 	  Goetz.
> 
> 
> 		-----Original Message-----
> 		From: Kumar Srinivasan
> [mailto:kumar.x.srinivasan at oracle.com]
> 		Sent: Montag, 27. November 2017 17:43
> 		To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
> <mailto:goetz.lindenmaier at sap.com>
> 		Cc: core-libs-dev at openjdk.java.net <mailto:core-libs-
> dev at openjdk.java.net> ; 'compiler-dev at openjdk.java.net
> <mailto:compiler-dev at openjdk.java.net> '
> 		<compiler-dev at openjdk.java.net> <mailto:compiler-
> dev at openjdk.java.net> ; serviceability-dev (serviceability-
> 		dev at openjdk.java.net <mailto:dev at openjdk.java.net> )
> <serviceability-dev at openjdk.java.net> <mailto:serviceability-
> dev at openjdk.java.net>
> 		Subject: Re: RFR: 8189102: All tools should support -?, -h and -
> -help
> 
> 		Hi,
> 
> 		I looked at some of the components I maintain, and they look
> good.
> 
> 		Wrt. the test:
> 
> 		1. The local variables/fields don't comply with the coding
> conventions,
> 		we have been
> 		      following in the JDK.
> 
> 		2.  Hmm, someone parked a .ini file in bin directory, later
> removed,
> 		      perhaps on windows simply check for .exe ? Should a
> check exist
> 		      to verify if file has executable permissions
> ?"File.canExecute".
> 
> 		  171     // Returns true if the file is not a tool.
> 		  172     static boolean notATool(String file) {
> 		  173         file = file.toLowerCase();
> 		  174         if (file.endsWith(".dll") ||
> 		  175             file.endsWith(".map") ||
> 		  176             file.endsWith(".pdb") ||
> 		  177             file.equals("server")) {  // server subdir on
> windows.
> 		  178             return true;
> 		  179         }
> 		  180         return false;
> 		  181     }
> 
> 
> 		3.  Typo in comment
> 
> 		201     // Check whether 'flag' appears in 'line' as a word of
> itself. I must not
> 
> 		s/I must/It must/
> 
> 
> 		4. There is a method to check for isEnglishLocale in
> TestHelper, perhaps
> 		use it
> 		to have the test bale out in non english locales as early as
> possible ?
> 
> 		5.  Has this been tested on all platforms ? do you need help
> testing it ?
> 
> 		Kumar
> 
> 
> 		On 11/17/2017 3:23 AM, Lindenmaier, Goetz wrote:
> 
> 			Hi,
> 
> 			please review this change. I also filed a CSR for this:
> 			http://cr.openjdk.java.net/~goetz/wr17/8189102-
> 
> 		helpMessage/webrev.02/
> 
> 			Bug: https://bugs.openjdk.java.net/browse/JDK-
> 8189102
> 			CSR: https://bugs.openjdk.java.net/browse/JDK-
> 8191477
> 
> 			See the webrev for a detailed description of the
> changes.
> 
> 			If required, I'll make break-out changes to be
> reviewed separately.
> 
> 			I had posted a RFR before, but improved the change
> to
> 			give a more complete overview of currently
> supported flags
> 			for the CSR:
> 			http://mail.openjdk.java.net/pipermail/hotspot-
> dev/2017-
> 
> 		October/028615.html
> 
> 
> 			Best regards,
> 			   Goetz.
> 
> 
> 
>