mbien commented on code in PR #4396:
URL: https://github.com/apache/netbeans/pull/4396#discussion_r923365370


##########
ide/extexecution.base/src/org/netbeans/api/extexecution/base/BaseExecutionService.java:
##########
@@ -140,6 +140,43 @@ public static BaseExecutionService newService(@NonNull 
Callable<? extends Proces
         return new BaseExecutionService(processCreator, descriptor);
     }
 
+    /**
+     * Infers the output encoding from the relevant system properties, if 
those should all be <code>null</code>
+     * then this will fallback to <code>Charset.defaultCharset()</code>
+     * 
+     * Since JDK 18 and JEP400 Console.charset() is used for the console's 
encoding instead of <code>Charset.defaultCharset()</code>. 
+     * Console.charset() is exposed via stdout.encoding/sun.stdout.encoding.
+     * If ran with JDK<=16 these properties should be null and the old default 
of <code>Charset.defaultCharset()</code> will be used to match pre JEP400 
behavior.
+     * 
+     * The checking order for the encoding is stdout.encoding, 
sun.stdout.encoding, native.encoding, <code>Charset.defaultCharset()</code>
+     * 
+     * @return inferred encoding as Charset
+     */
+    private static Charset getInputOutputEncoding(){
+        String[] encodingSystemProperties = new String[]{"stdout.encoding, 
sun.stdout.encoding, native.encoding"};
+
+        Charset preferredCharset = null;
+        for(String encodingProperty : encodingSystemProperties){
+            String encodingPropertyValue = 
System.getProperty(encodingProperty);
+            if(encodingPropertyValue == null){
+                continue;
+            }
+
+            try {
+                preferredCharset = Charset.forName(encodingPropertyValue);
+            } catch (Exception ex) {

Review Comment:
   nitpick: would recommend changing this to `IllegalArgumentException`, since 
this is the common supertype of all exceptions `forName` throws.



##########
ide/extexecution.base/src/org/netbeans/api/extexecution/base/BaseExecutionService.java:
##########
@@ -140,6 +140,43 @@ public static BaseExecutionService newService(@NonNull 
Callable<? extends Proces
         return new BaseExecutionService(processCreator, descriptor);
     }
 
+    /**
+     * Infers the output encoding from the relevant system properties, if 
those should all be <code>null</code>
+     * then this will fallback to <code>Charset.defaultCharset()</code>
+     * 
+     * Since JDK 18 and JEP400 Console.charset() is used for the console's 
encoding instead of <code>Charset.defaultCharset()</code>. 
+     * Console.charset() is exposed via stdout.encoding/sun.stdout.encoding.
+     * If ran with JDK<=16 these properties should be null and the old default 
of <code>Charset.defaultCharset()</code> will be used to match pre JEP400 
behavior.
+     * 
+     * The checking order for the encoding is stdout.encoding, 
sun.stdout.encoding, native.encoding, <code>Charset.defaultCharset()</code>
+     * 
+     * @return inferred encoding as Charset
+     */
+    private static Charset getInputOutputEncoding(){

Review Comment:
   please mention that this method is duplicated in `CommandLineOutputHandler`, 
e.g via `@see`.



##########
java/maven/src/org/netbeans/modules/maven/execute/CommandLineOutputHandler.java:
##########
@@ -828,29 +828,38 @@ public ExecutionEventObject.Tree getExecutionTree() {
         
     }    
 
-    private static Charset getNativeCharset() {
+    private static Charset getPreferredCharset() {
         // The CommandLineOutputHandler used the default charset to convert
         // output from command line invocations to strings. That encoding is
         // derived from the system file.encoding. From JDK 18 onwards its
         // default value changed to UTF-8.
-        // JDK 18+ exposes the native encoding as the new system property
+        // JDK 17+ exposes the native encoding as the new system property
         // native.encoding, prior versions don't have that property and will
         // report NULL for it.
-        // The algorithm is simple: If native.encoding is set, it will be used
-        // else the old default will be queried via Charset#defaultCharset.
-        String nativeEncoding = System.getProperty("native.encoding");
-        Charset nativeCharset = null;
-        if (nativeEncoding != null) {
+        // To account for the behavior of JEP400 the following order is used 
to determine the encoding:
+        // stdout.encoding, sun.stdout.encoding, native.encoding, 
Charset.defaultCharset()
+        String[] encodingSystemProperties = new String[]{"stdout.encoding, 
sun.stdout.encoding, native.encoding"};
+
+        Charset preferredCharset = null;
+        for(String encodingProperty : encodingSystemProperties){
+            String encodingPropertyValue = 
System.getProperty(encodingProperty);
+            if(encodingPropertyValue == null){
+                continue;
+            }
+
             try {
-                nativeCharset = Charset.forName(nativeEncoding);
+                preferredCharset = Charset.forName(encodingPropertyValue);
             } catch (Exception ex) {

Review Comment:
   same here



##########
java/maven/src/org/netbeans/modules/maven/execute/CommandLineOutputHandler.java:
##########
@@ -828,29 +828,38 @@ public ExecutionEventObject.Tree getExecutionTree() {
         
     }    
 
-    private static Charset getNativeCharset() {
+    private static Charset getPreferredCharset() {

Review Comment:
   please mention that this method is a copy of BaseExecutionService in a 
comment.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists

Reply via email to