-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[core] New report format html-report-v2.xslt to provide html with datatable and chart features #4125
Conversation
Generated by 🚫 Danger |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR!
The screenshot looks impressive, especially the charts are a good idea.
For the available report formats, we have them all mentioned in the documentation (see https://pmd.github.io/latest/pmd_userdocs_report_formats.html). Please add your new format there as well (in this PR).
The name could be simply "html-report-v2.xslt" - since it should be clear that this report is for PMD anyway...
In the documentation we can also describe, that using this report format, various javascript/css sources are loaded from the internets... depending on the environment this might not be allowed and/or possible. So, this report format won't work offline.
There we can also describe the additional features (like pdf export, excel export). Although I'm undecided, whether to include all these features, as it smells like doing too much and a very indirect solution (in order to create a excel file, you first need to create a html report...).
Maybe you can spend some time and look at #4132 (which I've just created) and share your thoughts. It would be interesting to know the different use cases (e.g. why we need a PDF export).
pmd-core/etc/xslt/pmd-report-v2.xslt
Outdated
<html> | ||
<head> | ||
<title>PMD Report </title> | ||
<link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/bootstrap@5.2.0/dist/css/bootstrap.min.css" integrity="sha384-gH2yIJqKdNHPEq0n4Mqa/HGKIhSkIHeL5AyhkYV8i59U5AR6csBvApHHNl/vI1Bx" crossorigin="anonymous"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How should we deal with updates of these libraries? E.g. bootstrap 5.2.1 is now available.
I think, whatever version we use here, it will be outdated very soon. Is this a problem we need to address?
Hi Andreas,
Thanks for the feedback. Let me work on them and keep you updated...
…On Sat, Sep 24, 2022 at 9:13 AM Andreas Dangel ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In pmd-core/etc/xslt/pmd-report-v2.xslt
<#4125 (comment)>:
> + <xsl:variable name="filename" ***@***.***" />
+ <xsl:variable name="slno" select="position()" />
+ <xsl:for-each select="scan:violation">
+ <xsl:variable name="pos" select="position()" />
+ <xsl:variable name="currPos" select="position()" />
+ <tr>
+ <td><xsl:value-of select="$slno"/></td>
+ <td><xsl:value-of select="position()"/></td>
+ <td><xsl:value-of select="$filename"/> </td>
+ <td><xsl:value-of ***@***.***"/></td>
+ <td><xsl:value-of ***@***.***"/></td>
+ <xsl:variable name="priority" ***@***.***" />
+ <script> total++; </script>
+ <xsl:choose>
+ <xsl:when test="$priority = 1">
+ <script> p1++; </script>
One more thing: If we have a pmd report with many violations, then we
would add these script-tags for each violation. This unnecessarily blows up
the html output. It should be possible to use jquery to calculate the
statistics afterwards from the table data in the document. That's by the
way how the datatable gets hold of the data for paging etc. - we can do the
same thing.
—
Reply to this email directly, view it on GitHub
<#4125 (review)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABNP3OVH5LTFOPQCHGAKDWLV735BBANCNFSM6AAAAAAQK5JQO4>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Hi @mohan-chinnappan-n, how is it going with this? Do you think we could try schedule it for the next release (in a month)? Please let us know if you need any help |
Sure. We can schedule it for the next release..
…On Tue, Oct 25, 2022 at 3:13 PM Clément Fournier ***@***.***> wrote:
Hi @mohan-chinnappan-n <https://github.com/mohan-chinnappan-n>, how is it
going with this? Do you think we could try schedule it for the next release
(in a month)? Please let us know if you need any help
Cheers
—
Reply to this email directly, view it on GitHub
<#4125 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABNP3OWE3XLVES7GTL54O2TWFAWODANCNFSM6AAAAAAQK5JQO4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Hi Clement,
Planning to finish this by Nov-06-2022.
you have a slack channel, we can communicate.
I have a few ideas like adding new features to PMD.... we can discuss
On Tue, Oct 25, 2022 at 3:20 PM Mohan Chinnappan <
***@***.***> wrote:
… Sure. We can schedule it for the next release..
On Tue, Oct 25, 2022 at 3:13 PM Clément Fournier ***@***.***>
wrote:
> Hi @mohan-chinnappan-n <https://github.com/mohan-chinnappan-n>, how is
> it going with this? Do you think we could try schedule it for the next
> release (in a month)? Please let us know if you need any help
> Cheers
>
> —
> Reply to this email directly, view it on GitHub
> <#4125 (comment)>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ABNP3OWE3XLVES7GTL54O2TWFAWODANCNFSM6AAAAAAQK5JQO4>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>
|
Hi @mohan-chinnappan-n , any news on this? Can we help? |
XLST for creating HTML reports
Created new html-report-v2.xslt based on the comments and feedback |
Hi Andreas,
- Changed the name to pmd-core/etc/xslt/html-report-v2.xslt
- Updated with your comments/feedback.
Hope you can merge it.
I will update the docs as well as after you merge it.
…On Fri, Nov 18, 2022 at 9:45 AM Andreas Dangel ***@***.***> wrote:
Hi @mohan-chinnappan-n <https://github.com/mohan-chinnappan-n> , any news
on this? Can we help?
—
Reply to this email directly, view it on GitHub
<#4125 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABNP3OWL5WSVSDHYDGJGSFDWI6JANANCNFSM6AAAAAAQK5JQO4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update!
- Please remove the file "pmd-report-v2.xslt", as you have renamed it. We only need "html-report-v2.xslt".
- Also please add the documentation directly in this PR. This increases the probability that we have the documentation ready when we merge this PR.
pmd-core/src/main/java/net/sourceforge/pmd/cli/PMDCommandLineInterface.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is ready now
thanks!
…On Tue, Jan 24, 2023 at 4:21 AM Andreas Dangel ***@***.***> wrote:
***@***.**** approved this pull request.
This is ready now
—
Reply to this email directly, view it on GitHub
<#4125 (review)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABNP3OXYAFYCNNYYH56X7KDWT6NIFANCNFSM6AAAAAAQK5JQO4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
[core] New report format html-report-v2.xslt to provide html with datatable and chart features pmd#4125
Describe the PR
Added pmd-report-v2.xslt to provide html with
datatable (search, column sorting and pagination)
chart features
Screenshot
Also updated CLI help with a line for this xslt
Related issues
Ready?
./mvnw clean verify
passes (checked automatically by github actions)