slachiewicz commented on code in PR #244:
URL:
https://github.com/apache/maven-reporting-impl/pull/244#discussion_r3741056524
##########
src/main/java/org/apache/maven/reporting/AbstractMavenReport.java:
##########
@@ -302,6 +327,97 @@ private void reportToSite() throws MojoExecutionException {
}
}
+ /**
+ * A sink for one subpage of a multipage report, remembering where it is
meant to be written to.
+ */
+ private static class MultiPageSubSink extends SiteRendererSink {
+ private final File outputDirectory;
+
+ private final String outputName;
+
+ MultiPageSubSink(File outputDirectory, String outputName,
DocumentRenderingContext docRenderingContext) {
+ super(docRenderingContext);
+ this.outputDirectory = outputDirectory;
+ this.outputName = outputName;
+ }
+
+ String getOutputName() {
+ return outputName;
+ }
+
+ File getOutputDirectory() {
+ return outputDirectory;
+ }
+ }
+
+ /**
+ * The sink factory handed to {@link #generate(Sink, SinkFactory,
Locale)}, mirroring what Maven Site Plugin
+ * provides so that a multipage report behaves the same when its goal is
invoked directly.
+ */
+ private static class MultiPageSinkFactory implements SinkFactory {
+ /**
+ * The report that is (maybe) generating multiple pages
+ */
+ private final MavenReport report;
+
+ /**
+ * The main DocumentRenderingContext, which is the base for the
DocumentRenderingContext of subpages
+ */
+ private final DocumentRenderingContext docRenderingContext;
+
+ /**
+ * List of sinks (subpages) associated to this report
+ */
+ private final List<MultiPageSubSink> sinks = new ArrayList<>();
+
+ MultiPageSinkFactory(MavenReport report, DocumentRenderingContext
docRenderingContext) {
+ this.report = report;
+ this.docRenderingContext = docRenderingContext;
+ }
+
+ @Override
+ public Sink createSink(File outputDirectory, String outputName) {
+ // Create a new document rendering context, similar to the main
one, but with a different output name
+ String document = PathTool.getRelativeFilePath(
+ report.getReportOutputDirectory().getPath(), new
File(outputDirectory, outputName).getPath());
+ // Remove .html suffix since we know that we are in Site Renderer
context
+ document = document.substring(0, document.lastIndexOf('.'));
+
Review Comment:
The claim is right — `lastIndexOf` returns -1 for a name with no dot and
`substring(0, -1)` throws.
I have left it, deliberately. That line is copied verbatim from
`MultiPageSinkFactory` in maven-site-plugin's `ReportDocumentRenderer`, and the
point of this PR is that a multipage report behaves the same whether its goal
is invoked directly or through the site. Making only this copy lenient would
mean a report using an extension-less name works standalone and throws under
`mvn site`, which is a worse failure mode than both rejecting it consistently:
it would only show up in one of the two paths.
The implied contract is that the name ends in `.html` — the site plugin's
own comment on that line says "Remove .html suffix since we know that we are in
Site Renderer context" — so an extension-less name is invalid input either way.
Happy to fix it in both places in a follow-up if a maintainer wants the
diagnostic improved; it should not be fixed in just one.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
