Disable coloring in build output when `--console=plain` is used by vinod-tahelyani · Pull Request #3087 · GoogleContainerTools/jib

vinod-tahelyani · 2021-02-26T13:50:02Z

codecov · 2021-02-26T13:55:06Z

Codecov Report

Merging #3087 (8b533b6) into master (73a6395) will increase coverage by 0.06%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master    #3087      +/-   ##
============================================
+ Coverage     71.09%   71.15%   +0.06%     
- Complexity     2307     2316       +9     
============================================
  Files           278      278              
  Lines          9755     9790      +35     
  Branches        989      991       +2     
============================================
+ Hits           6935     6966      +31     
+ Misses         2480     2478       -2     
- Partials        340      346       +6

Impacted Files	Coverage Δ	Complexity Δ
...jib/plugins/common/logging/PlainConsoleLogger.java	`90.90% <100.00%> (+0.90%)`	`4.00 <1.00> (ø)`
...loud/tools/jib/gradle/GradleProjectProperties.java	`65.21% <0.00%> (-0.65%)`	`31.00% <0.00%> (-2.00%)`
...ogle/cloud/tools/jib/api/JavaContainerBuilder.java	`82.17% <0.00%> (-0.50%)`	`53.00% <0.00%> (-1.00%)`
...om/google/cloud/tools/jib/gradle/BuildTarTask.java	`4.47% <0.00%> (-0.29%)`	`2.00% <0.00%> (ø%)`
.../google/cloud/tools/jib/gradle/BuildImageTask.java	`5.00% <0.00%> (-0.27%)`	`2.00% <0.00%> (ø%)`
...google/cloud/tools/jib/gradle/BuildDockerTask.java	`4.68% <0.00%> (-0.24%)`	`2.00% <0.00%> (ø%)`
...com/google/cloud/tools/jib/maven/BuildTarMojo.java	`0.00% <0.00%> (ø)`	`0.00% <0.00%> (ø%)`
...m/google/cloud/tools/jib/maven/BuildImageMojo.java	`0.00% <0.00%> (ø)`	`0.00% <0.00%> (ø%)`
.../google/cloud/tools/jib/maven/BuildDockerMojo.java	`0.00% <0.00%> (ø)`	`0.00% <0.00%> (ø%)`
...d/tools/jib/cli/jar/StandardPackagedProcessor.java	`100.00% <0.00%> (ø)`	`5.00% <0.00%> (+1.00%)`
... and 17 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 73a6395...8b533b6. Read the comment docs.

vinod-tahelyani · 2021-02-26T13:59:49Z

@mpeddada1 Can u pls review this?

chanseokoh · 2021-02-26T15:06:14Z

@vinod-tahelyani please follow the instructions above to sign the CLA so that we can accept your contribution.

vinod-tahelyani · 2021-02-27T07:08:55Z

@googlebot I signed it!

vinod-tahelyani · 2021-02-27T07:20:24Z

@googlebot I fixed it.

mpeddada1 · 2021-03-01T21:24:53Z

@vinod-tahelyani, thanks for creating the PR! The change looks good. Can you add a test for it in PlainConsoleLoggerTest?

vinod-tahelyani · 2021-03-07T13:27:22Z

@mpeddada1 we are expected to remove \u001B[31;1m kind as well right, in PlainConsoleLogger?

mpeddada1

@vinod-tahelyani Just added a few comments:)

@mpeddada1 we are expected to remove \u001B[31;1m kind as well right, in PlainConsoleLogger?

does this remove bright colors?

.../src/test/java/com/google/cloud/tools/jib/plugins/common/logging/PlainConsoleLoggerTest.java

chanseokoh · 2021-03-09T19:49:19Z

...mmon/src/main/java/com/google/cloud/tools/jib/plugins/common/logging/PlainConsoleLogger.java


-    singleThreadedExecutor.execute(() -> messageConsumer.accept(message));
+    // remove the color from the message
+    final String plainMessage = message.replaceAll("\u001B\\[[0-9;]{1,5}m", "");


I think this is too lenient and actually more confusing. For example, [0-9;]{1,5} accepts a string like ;;8;3;. ~~I think there can only be one ;.~~

Oh, actually, it turns out there can be multiple semi-colons.

Several attributes can be set in the same sequence, separated by semicolons.

Therefore, \u001B\\[[0-9;]*m is more accurate. However, I am fine limiting the scope by finding up to 5 characters.

vinod-tahelyani · 2021-03-10T14:15:16Z

@vinod-tahelyani Just added a few comments:)

@mpeddada1 we are expected to remove \u001B[31;1m kind as well right, in PlainConsoleLogger?

does this remove bright colors?

the intention is to remove a combination of (color + bold). example It will remove

…Colors for plain logging

.../src/test/java/com/google/cloud/tools/jib/plugins/common/logging/PlainConsoleLoggerTest.java

mpeddada1

LGTM, thank you for your contribution!

This comment has been minimized.

Sign in to view

google-cla bot added the cla: no label Feb 26, 2021

This comment has been minimized.

Sign in to view

vinod-tahelyani force-pushed the FIX/disable-coloring-on-console-plain branch from 455b68a to 43770e3 Compare February 27, 2021 07:29

This comment has been minimized.

Sign in to view

vinod-tahelyani force-pushed the FIX/disable-coloring-on-console-plain branch from 43770e3 to 82fd82a Compare February 27, 2021 07:47

This comment has been minimized.

Sign in to view

Remove the color from the message in log()

c835d3c

vinod-tahelyani force-pushed the FIX/disable-coloring-on-console-plain branch from 82fd82a to c835d3c Compare February 27, 2021 07:50

google-cla bot added cla: yes and removed cla: no labels Feb 27, 2021

chanseokoh added the kokoro:run label Feb 28, 2021

kokoro-team removed the kokoro:run label Feb 28, 2021

vinod-tahelyani added 2 commits March 7, 2021 18:46

Add ';' in regex for neutralizing bold chars in plain console logger

8fcf1b6

Add test for removing colors and bold chars to PlainConsoleLoggerTest

151b4d1

mpeddada1 added the kokoro:run label Mar 8, 2021

kokoro-team removed the kokoro:run label Mar 8, 2021

mpeddada1 reviewed Mar 9, 2021

View reviewed changes

.../src/test/java/com/google/cloud/tools/jib/plugins/common/logging/PlainConsoleLoggerTest.java Outdated Show resolved Hide resolved

.../src/test/java/com/google/cloud/tools/jib/plugins/common/logging/PlainConsoleLoggerTest.java Outdated Show resolved Hide resolved

chanseokoh reviewed Mar 9, 2021

View reviewed changes

Reverting change in testLog() and creating new test testLog_filterOut…

b7c92bf

…Colors for plain logging

mpeddada1 reviewed Mar 10, 2021

View reviewed changes

.../src/test/java/com/google/cloud/tools/jib/plugins/common/logging/PlainConsoleLoggerTest.java Outdated Show resolved Hide resolved

Formatting PlainConsoleLoggerTest.java

8b533b6

mpeddada1 added the kokoro:run label Mar 11, 2021

kokoro-team removed the kokoro:run label Mar 11, 2021

mpeddada1 approved these changes Mar 11, 2021

View reviewed changes

mpeddada1 merged commit 3a0944f into GoogleContainerTools:master Mar 11, 2021

Conversation

vinod-tahelyani commented Feb 26, 2021

Uh oh!

This comment has been minimized.

codecov bot commented Feb 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

vinod-tahelyani commented Feb 26, 2021

Uh oh!

chanseokoh commented Feb 26, 2021

Uh oh!

vinod-tahelyani commented Feb 27, 2021

Uh oh!

This comment has been minimized.

This comment has been minimized.

vinod-tahelyani commented Feb 27, 2021

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

mpeddada1 commented Mar 1, 2021

Uh oh!

vinod-tahelyani commented Mar 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mpeddada1 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

chanseokoh Mar 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chanseokoh Mar 10, 2021

Choose a reason for hiding this comment

Uh oh!

vinod-tahelyani commented Mar 10, 2021

Uh oh!

Uh oh!

mpeddada1 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov bot commented Feb 26, 2021 •

edited

Loading

vinod-tahelyani commented Mar 7, 2021 •

edited

Loading

chanseokoh Mar 9, 2021 •

edited

Loading