[
https://issues.apache.org/jira/browse/MNG-7486?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17543463#comment-17543463
]
ASF GitHub Bot commented on MNG-7486:
-------------------------------------
mthmulders commented on code in PR #746:
URL: https://github.com/apache/maven/pull/746#discussion_r884137008
##########
maven-core/src/main/java/org/apache/maven/internal/MultilineMessageHelper.java:
##########
@@ -0,0 +1,91 @@
+package org.apache.maven.internal;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Helper class to format multiline messages to the console
+ */
+public class MultilineMessageHelper
+{
+
+ private static final int DEFAULT_MAX_SIZE = 65;
+ private static final char BOX_CHAR = '*';
+
+ public static String separatorLine()
+ {
+ StringBuilder sb = new StringBuilder( DEFAULT_MAX_SIZE );
+ repeat( sb, '*', DEFAULT_MAX_SIZE );
+ return sb.toString();
+ }
+
+ public static List<String> format( String... lines )
+ {
+ int size = DEFAULT_MAX_SIZE;
+ int rem = size - 4; // 4 chars = 2 box_char + 2 spaces
+ List<String> result = new ArrayList<>();
+ StringBuilder sb = new StringBuilder( size );
+ // first line
+ sb.setLength( 0 );
+ repeat( sb, BOX_CHAR, size );
+ result.add( sb.toString() );
+ // lines
+ for ( String line : lines )
+ {
+ sb.setLength( 0 );
+ String[] words = line.split( " " );
+ for ( String word : words )
+ {
+ if ( sb.length() >= rem - word.length() - ( sb.length() > 0 ?
1 : 0 ) )
Review Comment:
What in the hypothetical case a word is longer than the line length? I guess
it would never get printed?
##########
maven-core/src/main/java/org/apache/maven/lifecycle/internal/builder/BuilderCommon.java:
##########
@@ -141,35 +142,36 @@ public MavenExecutionPlan resolveBuildPlan( MavenSession
session, MavenProject p
final Set<Plugin> unsafePlugins =
executionPlan.getNonThreadSafePlugins();
if ( !unsafePlugins.isEmpty() )
{
- logger.warn(
"*****************************************************************" );
- logger.warn( "* Your build is requesting parallel execution,
but project *" );
- logger.warn( "* contains the following plugin(s) that have
goals not marked *" );
- logger.warn( "* as @threadSafe to support parallel building.
*" );
- logger.warn( "* While this /may/ work fine, please look for
plugin updates *" );
- logger.warn( "* and/or request plugins be made thread-safe.
*" );
- logger.warn( "* If reporting an issue, report it against the
plugin in *" );
- logger.warn( "* question, not against maven-core
*" );
- logger.warn(
"*****************************************************************" );
+ for ( String s : MultilineMessageHelper.format(
+ "Your build is requesting parallel execution, but this
project contains the following "
+ + "plugin(s) that have goals not marked as
@threadSafe to support parallel execution.",
+ "While this /may/ work fine, please look for plugin
updates and/or "
+ + "request plugins be made thread-safe.",
+ "If reporting an issue, report it against the plugin
in question, not against Apache Maven." ) )
+ {
+ logger.warn( s );
+ }
if ( logger.isDebugEnabled() )
{
final Set<MojoDescriptor> unsafeGoals =
executionPlan.getNonThreadSafeMojos();
- logger.warn( "The following goals are not marked
@threadSafe in " + project.getName() + ":" );
+ logger.warn( "The following goals are not marked as
@threadSafe in " + project.getName() + ":" );
Review Comment:
Same as above.
##########
maven-core/src/main/java/org/apache/maven/lifecycle/internal/builder/BuilderCommon.java:
##########
@@ -141,35 +142,36 @@ public MavenExecutionPlan resolveBuildPlan( MavenSession
session, MavenProject p
final Set<Plugin> unsafePlugins =
executionPlan.getNonThreadSafePlugins();
if ( !unsafePlugins.isEmpty() )
{
- logger.warn(
"*****************************************************************" );
- logger.warn( "* Your build is requesting parallel execution,
but project *" );
- logger.warn( "* contains the following plugin(s) that have
goals not marked *" );
- logger.warn( "* as @threadSafe to support parallel building.
*" );
- logger.warn( "* While this /may/ work fine, please look for
plugin updates *" );
- logger.warn( "* and/or request plugins be made thread-safe.
*" );
- logger.warn( "* If reporting an issue, report it against the
plugin in *" );
- logger.warn( "* question, not against maven-core
*" );
- logger.warn(
"*****************************************************************" );
+ for ( String s : MultilineMessageHelper.format(
+ "Your build is requesting parallel execution, but this
project contains the following "
+ + "plugin(s) that have goals not marked as
@threadSafe to support parallel execution.",
+ "While this /may/ work fine, please look for plugin
updates and/or "
+ + "request plugins be made thread-safe.",
+ "If reporting an issue, report it against the plugin
in question, not against Apache Maven." ) )
+ {
+ logger.warn( s );
+ }
if ( logger.isDebugEnabled() )
{
final Set<MojoDescriptor> unsafeGoals =
executionPlan.getNonThreadSafeMojos();
- logger.warn( "The following goals are not marked
@threadSafe in " + project.getName() + ":" );
+ logger.warn( "The following goals are not marked as
@threadSafe in " + project.getName() + ":" );
for ( MojoDescriptor unsafeGoal : unsafeGoals )
{
- logger.warn( unsafeGoal.getId() );
+ logger.warn( " " + unsafeGoal.getId() );
}
}
else
{
- logger.warn( "The following plugins are not marked
@threadSafe in " + project.getName() + ":" );
+ logger.warn( "The following plugins are not marked as
@threadSafe in " + project.getName() + ":" );
Review Comment:
Same as above.
##########
maven-core/src/main/java/org/apache/maven/internal/MultilineMessageHelper.java:
##########
@@ -0,0 +1,91 @@
+package org.apache.maven.internal;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Helper class to format multiline messages to the console
+ */
+public class MultilineMessageHelper
+{
+
+ private static final int DEFAULT_MAX_SIZE = 65;
+ private static final char BOX_CHAR = '*';
+
+ public static String separatorLine()
+ {
+ StringBuilder sb = new StringBuilder( DEFAULT_MAX_SIZE );
+ repeat( sb, '*', DEFAULT_MAX_SIZE );
+ return sb.toString();
+ }
+
+ public static List<String> format( String... lines )
+ {
+ int size = DEFAULT_MAX_SIZE;
+ int rem = size - 4; // 4 chars = 2 box_char + 2 spaces
Review Comment:
Could we just name this 'remainder'? I don't think we have a limit on
variable names, so better make them clear and descriptive IMO.
##########
maven-core/src/main/java/org/apache/maven/lifecycle/internal/builder/BuilderCommon.java:
##########
@@ -141,35 +142,36 @@ public MavenExecutionPlan resolveBuildPlan( MavenSession
session, MavenProject p
final Set<Plugin> unsafePlugins =
executionPlan.getNonThreadSafePlugins();
if ( !unsafePlugins.isEmpty() )
{
- logger.warn(
"*****************************************************************" );
- logger.warn( "* Your build is requesting parallel execution,
but project *" );
- logger.warn( "* contains the following plugin(s) that have
goals not marked *" );
- logger.warn( "* as @threadSafe to support parallel building.
*" );
- logger.warn( "* While this /may/ work fine, please look for
plugin updates *" );
- logger.warn( "* and/or request plugins be made thread-safe.
*" );
- logger.warn( "* If reporting an issue, report it against the
plugin in *" );
- logger.warn( "* question, not against maven-core
*" );
- logger.warn(
"*****************************************************************" );
+ for ( String s : MultilineMessageHelper.format(
+ "Your build is requesting parallel execution, but this
project contains the following "
+ + "plugin(s) that have goals not marked as
@threadSafe to support parallel execution.",
Review Comment:
Nit: `@threadSafe` suggests the (old) Javadoc notation. Wouldn't it make
more sense to do either `@ThreadSafe` or just "not marked as thread-safe"
nowadays?
##########
maven-core/src/main/java/org/apache/maven/internal/MultilineMessageHelper.java:
##########
@@ -0,0 +1,91 @@
+package org.apache.maven.internal;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Helper class to format multiline messages to the console
+ */
+public class MultilineMessageHelper
+{
+
+ private static final int DEFAULT_MAX_SIZE = 65;
+ private static final char BOX_CHAR = '*';
+
+ public static String separatorLine()
+ {
+ StringBuilder sb = new StringBuilder( DEFAULT_MAX_SIZE );
+ repeat( sb, '*', DEFAULT_MAX_SIZE );
+ return sb.toString();
+ }
+
+ public static List<String> format( String... lines )
+ {
+ int size = DEFAULT_MAX_SIZE;
+ int rem = size - 4; // 4 chars = 2 box_char + 2 spaces
+ List<String> result = new ArrayList<>();
+ StringBuilder sb = new StringBuilder( size );
+ // first line
+ sb.setLength( 0 );
+ repeat( sb, BOX_CHAR, size );
+ result.add( sb.toString() );
+ // lines
+ for ( String line : lines )
+ {
+ sb.setLength( 0 );
+ String[] words = line.split( " " );
Review Comment:
`split` takes a regex param. Would it make sense to split on _all_
whitespace, e.g. `\s+`? Tab characters or multiple consecutive space characters
would not cause more "words" to appear:
```
jshell> "hello world".split(" ");
$1 ==> String[3] { "hello", "", "world" }
jshell> "hello world".split("\s");
$2 ==> String[3] { "hello", "", "world" }
jshell> "hello world".split("\s+");
$3 ==> String[2] { "hello", "world" }
```
##########
maven-core/src/main/java/org/apache/maven/lifecycle/internal/builder/BuilderCommon.java:
##########
@@ -141,35 +142,36 @@ public MavenExecutionPlan resolveBuildPlan( MavenSession
session, MavenProject p
final Set<Plugin> unsafePlugins =
executionPlan.getNonThreadSafePlugins();
if ( !unsafePlugins.isEmpty() )
{
- logger.warn(
"*****************************************************************" );
- logger.warn( "* Your build is requesting parallel execution,
but project *" );
- logger.warn( "* contains the following plugin(s) that have
goals not marked *" );
- logger.warn( "* as @threadSafe to support parallel building.
*" );
- logger.warn( "* While this /may/ work fine, please look for
plugin updates *" );
- logger.warn( "* and/or request plugins be made thread-safe.
*" );
- logger.warn( "* If reporting an issue, report it against the
plugin in *" );
- logger.warn( "* question, not against maven-core
*" );
- logger.warn(
"*****************************************************************" );
+ for ( String s : MultilineMessageHelper.format(
+ "Your build is requesting parallel execution, but this
project contains the following "
+ + "plugin(s) that have goals not marked as
@threadSafe to support parallel execution.",
+ "While this /may/ work fine, please look for plugin
updates and/or "
+ + "request plugins be made thread-safe.",
+ "If reporting an issue, report it against the plugin
in question, not against Apache Maven." ) )
+ {
+ logger.warn( s );
+ }
if ( logger.isDebugEnabled() )
{
final Set<MojoDescriptor> unsafeGoals =
executionPlan.getNonThreadSafeMojos();
- logger.warn( "The following goals are not marked
@threadSafe in " + project.getName() + ":" );
+ logger.warn( "The following goals are not marked as
@threadSafe in " + project.getName() + ":" );
for ( MojoDescriptor unsafeGoal : unsafeGoals )
{
- logger.warn( unsafeGoal.getId() );
+ logger.warn( " " + unsafeGoal.getId() );
}
}
else
{
- logger.warn( "The following plugins are not marked
@threadSafe in " + project.getName() + ":" );
+ logger.warn( "The following plugins are not marked as
@threadSafe in " + project.getName() + ":" );
for ( Plugin unsafePlugin : unsafePlugins )
{
- logger.warn( unsafePlugin.getId() );
+ logger.warn( " " + unsafePlugin.getId() );
}
- logger.warn( "Enable verbose output (-X) to see more
precisely which goals are not marked"
+ logger.warn( "" );
+ logger.warn( "Enable verbose output (-X) to see precisely
which goals are not marked as"
+ " @threadSafe." );
Review Comment:
Same as above.
> Create a multiline message helper for boxed log messages
> --------------------------------------------------------
>
> Key: MNG-7486
> URL: https://issues.apache.org/jira/browse/MNG-7486
> Project: Maven
> Issue Type: New Feature
> Reporter: Michael Osipov
> Assignee: Guillaume Nodet
> Priority: Major
> Fix For: 3.8.6, 3.9.0, 4.0.0-alpha-1, 4.0.0
>
>
> Simplify the way how boxed messages, e.g., for non-threadsafe plugins is
> created.
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
