You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ant.apache.org by "dreis2211 (via GitHub)" <gi...@apache.org> on 2023/02/24 18:27:08 UTC
[GitHub] [ant] dreis2211 opened a new pull request, #199: Use verbose log level for loading stylesheets
dreis2211 opened a new pull request, #199:
URL: https://github.com/apache/ant/pull/199
Hi,
I've noticed that working with `checkstyle` via Gradle (probably using it directly will do the same) we always see the very verbose output of the stylesheet being loaded. Imho this is not a good fit for the current `INFO` level.
```
[ant:xslt] Loading stylesheet <xsl:stylesheet xmlns:xsl="http://www.w3.org/1999/XSL/Transform" version="1.0">
<!--
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.
-->
<xsl:output method="html" indent="yes"/>
<xsl:decimal-format decimal-separator="." grouping-separator="," />
<xsl:key name="files" match="file" use="@name" />
<!-- Checkstyle XML Style Sheet by Stephane Bailliez <sb...@apache.org> -->
<!-- Part of the Checkstyle distribution found at http://checkstyle.sourceforge.net -->
<!-- UsageContext (generates checkstyle_report.html): -->
<!-- <checkstyle failonviolation="false" config="${check.config}"> -->
<!-- <fileset dir="${src.dir}" includes="**/*.java"/> -->
<!-- <formatter type="xml" toFile="${doc.dir}/checkstyle_report.xml"/> -->
<!-- </checkstyle> -->
<!-- <style basedir="${doc.dir}" destdir="${doc.dir}" -->
<!-- includes="checkstyle_report.xml" -->
<!-- style="${doc.dir}/checkstyle-noframes-sorted.xsl"/> -->
<xsl:template match="checkstyle">
<html>
<head>
<style type="text/css">
.bannercell {
border: 0px;
padding: 0px;
}
body {
margin-left: 10;
margin-right: 10;
font:normal 80% arial,helvetica,sanserif;
background-color:#FFFFFF;
color:#000000;
}
.a td {
background: #efefef;
}
.b td {
background: #fff;
}
th, td {
text-align: left;
vertical-align: top;
}
th {
font-weight:bold;
background: #ccc;
color: black;
}
table, th, td {
font-size:100%;
border: none
}
table.log tr td, tr th {
}
h2 {
font-weight:bold;
font-size:140%;
margin-bottom: 5;
}
h3 {
font-size:100%;
font-weight:bold;
background: #525D76;
color: white;
text-decoration: none;
padding: 5px;
margin-right: 2px;
margin-left: 2px;
margin-bottom: 0;
}
</style>
</head>
<body>
<a name="top"></a>
<!-- jakarta logo -->
<table border="0" cellpadding="0" cellspacing="0" width="100%">
<tr>
<td class="bannercell" rowspan="2">
<!--a href="http://jakarta.apache.org/">
<img src="http://jakarta.apache.org/images/jakarta-logo.gif" alt="http://jakarta.apache.org" align="left" border="0"/>
</a-->
</td>
<td class="text-align:right"><h2>CheckStyle Audit</h2></td>
</tr>
<tr>
<td class="text-align:right">Designed for use with <a href='http://checkstyle.sourceforge.net/'>CheckStyle</a> and <a href='http://jakarta.apache.org'>Ant</a>.</td>
</tr>
</table>
<hr size="1"/>
<!-- Summary part -->
<xsl:apply-templates select="." mode="summary"/>
<hr size="1" width="100%" align="left"/>
<!-- Package List part -->
<xsl:apply-templates select="." mode="filelist"/>
<hr size="1" width="100%" align="left"/>
<!-- For each package create its part -->
<xsl:apply-templates select="file[@name and generate-id(.) = generate-id(key('files', @name))]" />
<hr size="1" width="100%" align="left"/>
</body>
</html>
</xsl:template>
<xsl:template match="checkstyle" mode="filelist">
<h3>Files</h3>
<table class="log" border="0" cellpadding="5" cellspacing="2" width="100%">
<tr>
<th>Name</th>
<th>Errors</th>
</tr>
<xsl:for-each select="file[@name and generate-id(.) = generate-id(key('files', @name))]">
<xsl:sort data-type="number" order="descending" select="count(key('files', @name)/error)"/>
<xsl:variable name="errorCount" select="count(error)"/>
<tr>
<xsl:call-template name="alternated-row"/>
<td><a href="#f-{@name}"><xsl:value-of select="@name"/></a></td>
<td><xsl:value-of select="$errorCount"/></td>
</tr>
</xsl:for-each>
</table>
</xsl:template>
<xsl:template match="file">
<a name="f-{@name}"></a>
<h3>File <xsl:value-of select="@name"/></h3>
<table class="log" border="0" cellpadding="5" cellspacing="2" width="100%">
<tr>
<th>Error Description</th>
<th>Line</th>
</tr>
<xsl:for-each select="key('files', @name)/error">
<xsl:sort data-type="number" order="ascending" select="@line"/>
<tr>
<xsl:call-template name="alternated-row"/>
<td><xsl:value-of select="@message"/></td>
<td><xsl:value-of select="@line"/></td>
</tr>
</xsl:for-each>
</table>
<a href="#top">Back to top</a>
</xsl:template>
<xsl:template match="checkstyle" mode="summary">
<h3>Summary</h3>
<xsl:variable name="fileCount" select="count(file[@name and generate-id(.) = generate-id(key('files', @name))])"/>
<xsl:variable name="errorCount" select="count(file/error)"/>
<table class="log" border="0" cellpadding="5" cellspacing="2" width="100%">
<tr>
<th>Files</th>
<th>Errors</th>
</tr>
<tr>
<xsl:call-template name="alternated-row"/>
<td><xsl:value-of select="$fileCount"/></td>
<td><xsl:value-of select="$errorCount"/></td>
</tr>
</table>
</xsl:template>
<xsl:template name="alternated-row">
<xsl:attribute name="class">
<xsl:if test="position() mod 2 = 1">a</xsl:if>
<xsl:if test="position() mod 2 = 0">b</xsl:if>
</xsl:attribute>
</xsl:template>
</xsl:stylesheet>
```
As you can see this is really long and verbose.
I wasn't sure what the difference between debug and verbose is in the context of Ant, but going one level down seemed to be the defensive choice.
Cheers,
Christoph
--
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: dev-unsubscribe@ant.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] [ant] dreis2211 commented on a diff in pull request #199: Use verbose log level for loading stylesheets
Posted by "dreis2211 (via GitHub)" <gi...@apache.org>.
dreis2211 commented on code in PR #199:
URL: https://github.com/apache/ant/pull/199#discussion_r1118662584
##########
src/main/org/apache/tools/ant/taskdefs/XSLTProcess.java:
##########
@@ -1258,7 +1258,7 @@ protected void configureLiaison(final Resource stylesheet) throws BuildException
stylesheetLoaded = true;
try {
- log("Loading stylesheet " + stylesheet, Project.MSG_INFO);
+ log("Loading stylesheet " + stylesheet, Project.MSG_VERBOSE);
Review Comment:
Adjusted as suggested @jaikiran
--
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: dev-unsubscribe@ant.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] [ant] bodewig commented on pull request #199: Use verbose log level for loading stylesheets
Posted by "bodewig (via GitHub)" <gi...@apache.org>.
bodewig commented on PR #199:
URL: https://github.com/apache/ant/pull/199#issuecomment-1446269521
I'll create a bugzilla issue to scan for Resource#toString uses. This change here is fine as it is and we don't need to clutter it with more discussion.
Thank you for catching this @dreis2211
--
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: dev-unsubscribe@ant.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] [ant] jaikiran commented on pull request #199: Use verbose log level for loading stylesheets
Posted by "jaikiran (via GitHub)" <gi...@apache.org>.
jaikiran commented on PR #199:
URL: https://github.com/apache/ant/pull/199#issuecomment-1446256401
Hello Stefan,
> Looks as if StringResource printed its contents, but that seems wrong to me as there may be more places where code expects toString to not reveal the contents.
Agreed. The `StringResource` is overriding the toString() to print the contents and that looked wrong to me too. However, it has an explicit `@since` on it so it looked to me that this might have been done intentionally for some reason. Like you say, there might be additional places we need to check where we log the `Resource.toString()` in a message.
--
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: dev-unsubscribe@ant.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] [ant] bodewig commented on pull request #199: Use verbose log level for loading stylesheets
Posted by "bodewig (via GitHub)" <gi...@apache.org>.
bodewig commented on PR #199:
URL: https://github.com/apache/ant/pull/199#issuecomment-1446249553
I wonder what kind of `Resource` this is as `Resource#toString` is expected to only print the resource's name - and this is what the code here relied on. Looks as if `StringResource` printed its contents, but that seems wrong to me as there may be more places where code expects `toString` to not reveal the contents.
Having said that, using `name` rather than `toString` here (and potentially more places) is the right thing to do in any case.
--
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: dev-unsubscribe@ant.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] [ant] asfgit closed pull request #199: Use verbose log level for loading stylesheets
Posted by "asfgit (via GitHub)" <gi...@apache.org>.
asfgit closed pull request #199: Use verbose log level for loading stylesheets
URL: https://github.com/apache/ant/pull/199
--
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: dev-unsubscribe@ant.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] [ant] bodewig commented on pull request #199: Use verbose log level for loading stylesheets
Posted by "bodewig (via GitHub)" <gi...@apache.org>.
bodewig commented on PR #199:
URL: https://github.com/apache/ant/pull/199#issuecomment-1446276764
https://bz.apache.org/bugzilla/show_bug.cgi?id=66496
--
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: dev-unsubscribe@ant.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] [ant] jaikiran commented on a diff in pull request #199: Use verbose log level for loading stylesheets
Posted by "jaikiran (via GitHub)" <gi...@apache.org>.
jaikiran commented on code in PR #199:
URL: https://github.com/apache/ant/pull/199#discussion_r1118656632
##########
src/main/org/apache/tools/ant/taskdefs/XSLTProcess.java:
##########
@@ -1258,7 +1258,7 @@ protected void configureLiaison(final Resource stylesheet) throws BuildException
stylesheetLoaded = true;
try {
- log("Loading stylesheet " + stylesheet, Project.MSG_INFO);
+ log("Loading stylesheet " + stylesheet, Project.MSG_VERBOSE);
Review Comment:
Thank you Christoph for this change. I'm surprised it was logging the contents of the file. I think what we should probably do here is keep it at `INFO` level but change the log message to just print the name of the resource :
```
log("Loading stylesheet " + stylesheet.getName(), Project.MSG_INFO);
```
--
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: dev-unsubscribe@ant.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
[GitHub] [ant] jaikiran commented on pull request #199: Use verbose log level for loading stylesheets
Posted by "jaikiran (via GitHub)" <gi...@apache.org>.
jaikiran commented on PR #199:
URL: https://github.com/apache/ant/pull/199#issuecomment-1446237933
Thank you for these changes. Looks good to me. I'll merge this shortly. Since this is your first contribution to the Ant project, I'll be adding `Christoph Dreis` to our contributors list https://github.com/apache/ant/blob/master/CONTRIBUTORS. If you prefer a different first name, last name, feel free to let us know.
--
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: dev-unsubscribe@ant.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org