michael-o commented on code in PR #625: URL: https://github.com/apache/maven-doxia-sitetools/pull/625#discussion_r2971565915
########## doxia-site-renderer/src/test/java/org/apache/maven/doxia/siterenderer/RenderingContextTest.java: ########## @@ -1,76 +0,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. - */ -package org.apache.maven.doxia.siterenderer; - -import java.io.File; - -import org.codehaus.plexus.testing.PlexusTest; -import org.junit.jupiter.api.Test; - -import static org.codehaus.plexus.testing.PlexusExtension.getBasedir; -import static org.junit.jupiter.api.Assertions.assertEquals; - -/** - * @author <a href="mailto:[email protected]";>olamy</a> - * @since 20 oct. 07 - */ -@PlexusTest -class RenderingContextTest { - - /** - * Test getRelativePath() with various file paths. - * - * @throws java.lang.Exception if any. - */ - @Test - void filePathWithDot() throws Exception { - File baseDir = new File(getBasedir() + File.separatorChar + "test" + File.separatorChar + "resources"); - - String document = "file.with.dot.in.name.xml"; - DocumentRenderingContext docRenderingContext = - new DocumentRenderingContext(baseDir, "test", document, "", "xml", false); - assertEquals("file.with.dot.in.name.html", docRenderingContext.getOutputPath()); - assertEquals(".", docRenderingContext.getRelativePath()); - - document = "file.with.dot.in.name"; - docRenderingContext = new DocumentRenderingContext(baseDir, document, "generator"); // not Doxia source - assertEquals("file.with.dot.in.name.html", docRenderingContext.getOutputPath()); - assertEquals(".", docRenderingContext.getRelativePath()); - - document = "index.xml.vm"; - docRenderingContext = new DocumentRenderingContext(baseDir, "test", document, "", "xml", false); - assertEquals("index.html", docRenderingContext.getOutputPath()); - assertEquals(".", docRenderingContext.getRelativePath()); - - document = "download.apt.vm"; - docRenderingContext = new DocumentRenderingContext(baseDir, "test", document, "", "apt", false); - assertEquals("download.html", docRenderingContext.getOutputPath()); - assertEquals(".", docRenderingContext.getRelativePath()); - - document = "path/file.apt"; - docRenderingContext = new DocumentRenderingContext(baseDir, "test", document, "", "apt", false); - assertEquals("path/file.html", docRenderingContext.getOutputPath()); - assertEquals("..", docRenderingContext.getRelativePath()); - - document = "path/file"; - docRenderingContext = new DocumentRenderingContext(baseDir, document, "generator"); // not Doxia source - assertEquals("path/file.html", docRenderingContext.getOutputPath()); - assertEquals("..", docRenderingContext.getRelativePath()); - } -} Review Comment: This should have been a "git mv" instead of deletion and addtion, no? ########## doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/SiteRenderingContext.java: ########## @@ -73,6 +76,37 @@ public boolean isEditable() { public boolean isSkipDuplicates() { return skipDuplicates; } + + /** + * Add an alternative source directory to this site directory. + * This will implicitly turn it into an editable site directory. Multiple source directories can be used, the first one containing a file with a given name will be used for that file, Review Comment: If you say the first one containing, that is completely random with a hash set. Please explain. ########## doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/SiteRenderingContext.java: ########## @@ -44,15 +46,16 @@ public static class SiteDirectory { private File path; private boolean editable; private boolean skipDuplicates; + private final Collection<File> editableSourceDirectories = new HashSet<>(); public SiteDirectory(File path, boolean editable) { this(path, editable, false); } /** * - * @param path - * @param editable + * @param path absolute path to the site directory containing doxia sources files, expected to have a Doxia Site layout, i.e. one directory per Doxia parser module Review Comment: You say absolute path, but we don't enforce it...? ########## doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DocumentRenderingContext.java: ########## @@ -32,26 +35,59 @@ * @since 1.5 (was since 1.1 in o.a.m.d.sink.render) */ public class DocumentRenderingContext { - private final File basedir; - private final String basedirRelativePath; + /** absolute path to the source base directory (not null, pseudo value when not a Doxia source), this is parser format specific. Must start with {@link #siteRootDirectory}. */ + private final File basedir; + /** {@link #basedir} relative path to the document's source including {@link #extension}. May be {@code null} if not rendered from a Doxia source */ private final String inputPath; + /** same as {@link #inputPath} but with extension replaced with {@code .html}, this is parser format specific */ private final String outputPath; + /** the Doxia module parser id associated to this document, may be null if document not rendered from a Doxia source. */ private final String parserId; - private final String relativePath; - + /** the source document filename extension, may be null if document not rendered from a Doxia source. */ private final String extension; private Map<String, String> attributes; - private final boolean editable; + /** + * The absolute paths of directories which may contain the original editable source. + * If empty document is not editable. + */ + private final Collection<File> sourceDirectories; + + /** The project's build directory, may be {@code null} rendered from a Doxia source) */ + private final File rootDirectory; + + /** The site's root directory (never null), must be within below {@link #rootDirectory}, may be {@code null} rendered from a Doxia source */ Review Comment: Contradiction. It first says never null, then it maybe null ########## doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DocumentRenderingContext.java: ########## @@ -189,12 +315,13 @@ public String getParserId() { } /** - * Get the relative path to site root. + * Get the relative path of the parent folder to site root. Review Comment: parent directory ########## doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/SiteRenderingContext.java: ########## @@ -73,6 +76,37 @@ public boolean isEditable() { public boolean isSkipDuplicates() { return skipDuplicates; } + + /** + * Add an alternative source directory to this site directory. + * This will implicitly turn it into an editable site directory. Multiple source directories can be used, the first one containing a file with a given name will be used for that file, + * the others will be ignored. This allows to have a main source directory and an optional overlay directory with custom files. + * Only necessary to call if the source directory is different from the site directory, otherwise the site directory will be used as source directory. + * @param sourceDirectory + * @since 2.1 + */ + public void addAlternativeEditableSourceDirectory(File sourceDirectory) { + editableSourceDirectories.add(sourceDirectory); + editable = true; + } + + /** + * Get the source directories for this site directory. If no alternative source directory has been added via {@link #addAlternativeEditableSourceDirectory(File)} + * the site directory itself ({@link #getPath()}) will be returned as the only source directory. + * If the site directory is not editable, an empty collection will be returned. + * @return the source directories for this site directory + * @since 2.1 + */ + public Collection<File> getEditableSourceDirectories() { + if (!editable) { + return Collections.emptyList(); + } + if (editableSourceDirectories.isEmpty()) { + return Collections.singleton(path); + } else { + return editableSourceDirectories; Review Comment: This should return only a view, no? ########## doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DocumentRenderingContext.java: ########## @@ -32,26 +35,59 @@ * @since 1.5 (was since 1.1 in o.a.m.d.sink.render) */ public class DocumentRenderingContext { - private final File basedir; - private final String basedirRelativePath; + /** absolute path to the source base directory (not null, pseudo value when not a Doxia source), this is parser format specific. Must start with {@link #siteRootDirectory}. */ + private final File basedir; + /** {@link #basedir} relative path to the document's source including {@link #extension}. May be {@code null} if not rendered from a Doxia source */ private final String inputPath; + /** same as {@link #inputPath} but with extension replaced with {@code .html}, this is parser format specific */ private final String outputPath; + /** the Doxia module parser id associated to this document, may be null if document not rendered from a Doxia source. */ private final String parserId; - private final String relativePath; - + /** the source document filename extension, may be null if document not rendered from a Doxia source. */ private final String extension; private Map<String, String> attributes; - private final boolean editable; + /** + * The absolute paths of directories which may contain the original editable source. + * If empty document is not editable. + */ + private final Collection<File> sourceDirectories; + + /** The project's build directory, may be {@code null} rendered from a Doxia source) */ + private final File rootDirectory; + + /** The site's root directory (never null), must be within below {@link #rootDirectory}, may be {@code null} rendered from a Doxia source */ + private final File siteRootDirectory; + /** optional descriptive text of the plugin which generated the output (usually Maven coordinates). Only set when document is not based on a Doxia source. */ private final String generator; + static File stripSuffixFromPath(File file, String suffix) { + File relevantFile = file; + if (suffix == null || suffix.isEmpty()) { + return relevantFile; + } + File currentSuffixPart = new File(suffix); + // compare elements from end, suffix should be a suffix of file + do { + if (currentSuffixPart.getName().equals(relevantFile.getName())) { + relevantFile = relevantFile.getParentFile(); + if (relevantFile == null) { + throw new IllegalArgumentException("Suffix " + suffix + " has more elements than file " + file); + } + } else { + throw new IllegalArgumentException("Suffix " + suffix + " is not a suffix of file " + file); Review Comment: Why fail if we could return the original one without modifying it? You don't fail in like 73 either. ########## doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java: ########## @@ -228,32 +227,27 @@ private List<String> filterExtensionIgnoreCase(List<String> fileNames, String ex * Populates the files map with {@link DocumentRenderer}s per output name in parameter {@code files} for all files in the moduleBasedir matching the module extensions, * taking care of duplicates if needed. * - * @param rootDir + * @param siteRootDirectory + * @param siteDirectory * @param moduleBasedir * @param module * @param excludes * @param files - * @param editable - * @param skipDuplicates * @throws IOException * @throws RendererException */ private void addModuleFiles( - File rootDir, + File siteRootDirectory, + SiteDirectory siteDirectory, Review Comment: If we have a site root directory, why is path the absolute for site directory? ########## doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/SiteRenderingContext.java: ########## @@ -44,15 +46,16 @@ public static class SiteDirectory { private File path; private boolean editable; private boolean skipDuplicates; + private final Collection<File> editableSourceDirectories = new HashSet<>(); public SiteDirectory(File path, boolean editable) { this(path, editable, false); } /** * - * @param path - * @param editable + * @param path absolute path to the site directory containing doxia sources files, expected to have a Doxia Site layout, i.e. one directory per Doxia parser module Review Comment: Doxia source files -- 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]
