checkstyle / contribution

some useful sources that should not stay in main repo but it is good to host them
GNU Lesser General Public License v2.1
49 stars 130 forks source link

patch-diff-report-tool: should preserve stylesheet.css from jxr site #424

Open romani opened 4 years ago

romani commented 4 years ago

We do read sources generated by JXR. unfortunately tool does not copy it to diff folder, so sources become plain text.

customization of css:

          <plugin>
            <groupId>org.apache.maven.plugins</groupId>
            <artifactId>maven-jxr-plugin</artifactId>
            <version>${jxr-plugin.version}</version>
            <!-- plugin do not have ability to define followinng by system property -->
            <configuration>
                <stylesheet>${project.basedir}/jxr.css</stylesheet>
              <excludes>
                <exclude>**/.ci-temp/**/*</exclude>
              </excludes>
            </configuration>
          </plugin>

custom css, should use default + our customization for higlihting of target (anchored) line.

/* content of https://github.com/apache/maven-jxr/blob/master/maven-jxr-plugin/src/main/resources/stylesheet.css -- BEGIN */

body {
    background-color: #fff;
    font-family: Arial, Helvetica, sans-serif;
}

a:link {
    color: #00f;
}
a:visited {
    color: #00a;
}

a:active, a:hover {
    color: #f30 !important;
}

ul, li {
    list-style-type:none;
    margin:0;
    padding:0;
}

table td {
    padding: 3px;
    border: 1px solid #000;
}
table {
    width:100%;
    border: 1px solid #000;
    border-collapse: collapse;
}

div.overview {
    background-color:#ddd;
    padding: 4px 4px 4px 0;
}
div.overview li, div.framenoframe li {
    display: inline;
}
div.framenoframe {
    text-align: center;
    font-size: x-small;
}
div.framenoframe li {
    margin: 0 3px 0 3px;
}
div.overview li {
    margin:3px 3px 0 3px;
    padding: 4px;
}
li.selected {
    background-color:#888;
    color: #fff;
    font-weight: bold;
}

table.summary {
    margin-bottom: 20px;
}
table.summary td, table.summary th {
    font-weight: bold;
    text-align: left;
    padding: 3px;
}
table.summary th {
    background-color:#036;
    color: #fff;
}
table.summary td {
    background-color:#eee;
    border: 1px solid black;
}

em {
    color: #A00;
}
em.comment {
    color: #390;
}
.string {
    color: #009;
}

#overview {
    padding:2px;
}

hr {
    height: 1px;
    color: #000;
}

/* JXR style sheet */
.jxr_comment
{
    color: #390;
}

.jxr_javadoccomment
{
    color: #A00;
}

.jxr_string
{
    color: #009;
}

.jxr_keyword
{
    color: #000;
}
/* content of https://github.com/apache/maven-jxr/blob/master/maven-jxr-plugin/src/main/resources/stylesheet.css -- END */
/* BELOW ARE OUR CUSTOMIZATION */
:target {
  background-color: #b2b2e1;
}

to make it look like: image

rnveach commented 4 years ago

so sources become plain text.

jxr and maven-checkstyle-plugin inside checkstyle-tester should be removed as they just amount to nothing. Jxr just limits us to Java files and we can't run regression on non-java files. See https://github.com/checkstyle/contribution/issues/273 .

jxr generation of Java file for final report is done inside https://github.com/checkstyle/contribution/blob/master/patch-diff-report-tool/src/main/java/com/github/checkstyle/site/XrefGenerator.java#L84 and https://github.com/checkstyle/contribution/blob/master/patch-diff-report-tool/src/main/java/com/github/checkstyle/site/XrefGenerator.java#L142-L144 and that is where CSS would have to be injected. The only thing the tool lets you do is inject a footer.

http://rveach.no-ip.org/checkstyle/regression/reports/262/checkstyle/xref/src/it/resources/com/google/checkstyle/test/chapter4formatting/rule4842fallthrough/InputFallThrough.java.html An example diff file does show that it is by default looking for a certain stylesheet.

<link type="text/css" rel="stylesheet" href="../../../../../../stylesheet.css" />

As I am not seeing this stylesheet anywhere, you could just add code to plop this file into the right directory and possibly all Java files would use it.

Going this route, however, means our custom TextTransform at https://github.com/checkstyle/contribution/blob/3d26913507904af127bb57ef90af48f5505b1eb8/patch-diff-report-tool/src/main/java/com/github/checkstyle/site/TextTransform.java#L220 will also need to be modified to use the same stylesheet file as I don't believe it takes into account the directory structure like the Java one.

rnveach commented 1 year ago

customization of css: <configuration> <stylesheet>${project.basedir}/jxr.css</stylesheet>

https://github.com/apache/maven-jxr/blob/6e0098709e4b5e392909638aabfed911b4b2f9a1/maven-jxr-plugin/src/main/java/org/apache/maven/plugin/jxr/AbstractJxrReport.java#L266 https://github.com/apache/maven-jxr/blob/6e0098709e4b5e392909638aabfed911b4b2f9a1/maven-jxr-plugin/src/main/java/org/apache/maven/plugin/jxr/AbstractJxrReport.java#L335 All this does is take the stylesheet we give it, and copies it to the required location.

It is not really a solution as what we need is for the utility to allow us to add more ../s or swap them with a global CSS URL in the HTML.

Example: http://rveach.no-ip.org/checkstyle/regression/320/openjdk7/xref/test/java/lang/annotation/package-info.java.html Needs the stylesheet placed into http://rveach.no-ip.org/checkstyle/regression/320/openjdk7/xref/test/java/stylesheet.css . As we can see, the folder is deeper than a global stylesheet would be for everything.

https://github.com/apache/maven-jxr/blob/15d6b3908b6340e127650dbc0a9fc2007d75beea/maven-jxr/src/main/java/org/apache/maven/jxr/JavaCodeTransform.java#L149 https://github.com/apache/maven-jxr/blob/15d6b3908b6340e127650dbc0a9fc2007d75beea/maven-jxr/src/main/java/org/apache/maven/jxr/JavaCodeTransform.java#L349-L351 The ../s added to the HTML for the stylesheet are the same count as the periods in the package file. There is no way to override the behavior as the methods are private. We cannot pass in any more parameters to the method to change the behavior.

It also looks to me, that this would even fully break apart when we do shorten file names for Windows users as it is still basing logic on package name and not the save location.

Our choices seem to be: 1) Create our own JavaCodeTransform, duplicating theirs like we had to do for the TextTransform. 2) Update the HTML file manually after it is created to swap the CSS. 3) Put multiple stylesheets in all the locations they need to be in. Since this is based on the package and some projects have test classes with non-standard packages, we could be adding many. 4) Submit a request and hope they change it and do a release (4 months since the last release, 1 month since the last code change) 5) Drop this issue.

@romani ping

romani commented 1 year ago

I do not want to depend on maven plugin. I am ok with any approach to make line of code to be more easy to see. '2' is good, '1' is also good. Lets use what is more easy to implement, most likely "2", but if you feel to do "1" is also ok.

nrmancuso commented 1 year ago

@rnveach would (2) help us to migrate from maven plugin?

rnveach commented 1 year ago

I assume we are not referring to maven-checkstyle-plugin, as that has no bearing here for patch-diff-report.

If we are referring to the JXR plugin which generates the Java HTML file for us, then to get away from it we have to drop the dependency completely. This means option 1 is the only solution as all others rely on the JXR plugin to do the work for us.

However, to completely drop JXR requires us to duplicate many core files. I don't know how many there will be or how much there will be but it does NOT seem like this is the easiest solution.

romani commented 1 year ago

Let's use easy solution for now