Issue #13770: Enforce Linux Style EOL for all XML files

[kalpadiptyaroy]

Issue #13770: Enforce Linux Style EOL for all XML files

[kalpadiptyaroy]

Finally fixed the EOL problem in a white collar way.
Just need to fix the Modifed Controller Variable stuff

[ERROR] [checkstyle] [ERROR] ... site\XdocsTemplateSink.java:91:23: Control variable 'id' is modified. [ModifiedControlVariable]

@romani - any suggestions on this Modified Controller Variable thing?

Hold on patience for #13696. We are very close to fixing this EOL issue.

[romani]

Please create javadoc for methods to define from what class you copied implementation and how you modified them.
We will need such details during update of library where such code is located, in case it is changed, we will need to copy it one more time.

If checkstyle violation on copied code we can put comment to suppress violation -cs: ModifiedControlVariable

[kalpadiptyaroy]

Please create javadoc for methods to define from what class you copied implementation and how you modified them. We will need such details during update of library where such code is located, in case it is changed, we will need to copy it one more time.

If checkstyle violation on copied code we can put comment to suppress violation -cs: ModifiedControlVariable

Done.
and tackled ModifiedControllerVariable by compliance.

[romani]

item

[romani]

items:

[romani]

Items:

[romani]

See questions above.
Please reply to each review thread.

[kalpadiptyaroy]

See questions above. Please reply to each review thread.

Yes replied

[romani]

Please address #13774 (comment)

[kalpadiptyaroy]

@romani. I tried to preserve the original write function as follows

@Override
    protected void write(String text) {
        if (!this.tableCaptionXmlWriterStack.isEmpty()
                && this.tableCaptionXmlWriterStack.getLast() != null) {
            this.tableCaptionXmlWriterStack.getLast().writeMarkup(unifyToLinuxEndOfLine(text));
        }
        else if (!this.tableContentWriterStack.isEmpty()
                && this.tableContentWriterStack.getLast() != null) {
            this.tableContentWriterStack.getLast().write(unifyToLinuxEndOfLine(text));
        }
        else {
            writer.write(unifyToLinuxEndOfLine(text));
        }
    }

But this is giving rise to PMD errors as below:

INFO] --- pmd:3.21.0:check (default) @ checkstyle ---
[INFO] PMD version: 6.55.0
[INFO] PMD Failure: com.puppycrawl.tools.checkstyle.site.XdocsTemplateSink:57 Rule:LooseCoupling Priority:3 Avoid using implementation types like 'LinkedList'; use the interface instead.
[INFO] PMD Failure: com.puppycrawl.tools.checkstyle.site.XdocsTemplateSink:63 Rule:LooseCoupling Priority:3 Avoid using implementation types like 'LinkedList'; use the interface instead.
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  22:17 min
[INFO] Finished at: 2023-09-26T23:56:35+05:30

Now, if I try to replace LinkedList with List in the declarations then getLast() func. from this.tableContentWriterStack.getLast() becomes undefined.

Therefore, I feel it's better to put a proper explanation in the javadoc to justify the deviation in overridden implementation.

Just updated the Javadoc in the latest push.

[michael-o]

@nrmancuso @michael-o ping - Kindly share your reviews.

I still don't understand the purpose

[romani]

A way better implementation!!
Thanks a lot.

@kalpadiptyaroy , please test it in another PR to prove that it works.

[romani]

@michael-o, is this implementation ok to you. You required change so we need to get from you approval.

[michael-o]

@michael-o, is this implementation ok to you. You required change so we need to get from you approval.

Logically yes, but what I don't understand why not pass CustomPrintWriter to the real XdocSink instead of derived impl...

[romani]

@michael-o , do you mean to put this custom writer to

checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/site/XdocsTemplateSink.java

Line 38 in 63b9e0b

public class XdocsTemplateSink extends XdocSink {

?

[kalpadiptyaroy]

Logically yes, but what I don't understand why not pass CustomPrintWriter to the real XdocSink instead of derived impl...

XdocSink is a doxia class. So, I can't put the custom writer over there. Hence, I kept it in XdocsTemplateSink.

[kalpadiptyaroy]

@kalpadiptyaroy , please test it in another PR to prove that it works.

I will definitely cherry-pick this in the other PR too. But prior to that. Can you tell me how to resolve these CI failures?

[michael-o]

Logically yes, but what I don't understand why not pass CustomPrintWriter to the real XdocSink instead of derived impl...

XdocSink is a doxia class. So, I can't put the custom writer over there. Hence, I kept it in XdocsTemplateSink.

Why? You can pass your custom writer, isn't that enough?

[nrmancuso]

@kalpadiptyaroy semaphore failures can be ignored, looks like we had some problem downloading dependencies. As for checker - these both seem like reasonable failures, we can implement the required changes.

[romani]

Link check is failing also.
@nrmancuso , should we add such lines to suppress file ?

[nrmancuso]

Link check is failing also. @nrmancuso , should we add such lines to suppress file ?

I restarted link check, let’s make sure it’s stable failure

[nrmancuso]

Final todo:

1. Suppress checker errors: I spent an hour trying to make checker happy on this SHA, to no avail. Please just suppress these errors.
2. Suppress linkcheck.
3. Rebase on latest master

@kalpadiptyaroy let's get this PR across the finish line

[romani]

@nrmancuso , there is some groovy failure after Checker execution, please assist.

[nrmancuso]

@nrmancuso , there is some groovy failure after Checker execution, please assist.

Looks like groovy script has a different idea about how these suppressions should look? @kalpadiptyaroy please try to apply patch exactly as proposed by git diff step in workflow execution. Maybe we have some bug in our groovy script.

[kalpadiptyaroy]

Sure. Will do that.

[kalpadiptyaroy]

@nrmancuso - Done - We can merge now!

[nrmancuso]

Let’s merge this.