[GitHub] [maven-shared-utils] guylabs commented on a change in pull request #93: Make uninstall resilient to Jansi errors

[GitBox <gi...@apache.org>]

guylabs commented on a change in pull request #93:
URL: https://github.com/apache/maven-shared-utils/pull/93#discussion_r665333009



##########
File path: src/main/java/org/apache/maven/shared/utils/logging/MessageUtils.java
##########
@@ -98,7 +98,14 @@ private static void doSystemUninstall()
     {
         if ( JANSI )
         {
-            AnsiConsole.systemUninstall();
+            try

Review comment:
       This was already proposed here https://issues.apache.org/jira/browse/MNG-7161 but it seemed that the consensus was to go with that approach, which makes `MessageUtils` less dependent on changes on Jansi which caused this error. Fixing the root cause will fix _this_ specific use case, but there may be others which will break too, and as the `systemUninstall` is called as a last step before Maven finishes the build we can assume that it's safe to swallow these errors. I created https://github.com/fusesource/jansi/issues/214 to fix the root cause.
   
   IMO we should not add it to the `systemInstall` as there you'd like to be aware when we can't install the color support and get the exception during runtime, as if we would swallow that error too, it will be harder to debug the root cause without any exception thrown. If you prefer to have the same handling, then I can add this.




-- 
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: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org