From 39820f203eb33168b3cd3a0312396de0e2c36289 Mon Sep 17 00:00:00 2001 From: James Roper Date: Tue, 12 Feb 2019 15:42:20 +1100 Subject: [PATCH 1/3] Improved includes This improves the includes, so rather than the included markdown being parsed at the parent parents render time, it's parsed and then included in the AST straight after the parent page is parsed. One consequence of this is that now the left hand page navigation works, since the headers can be extracted from the included markdown. I think it's also more robust all around. In order to implement this, I had to create a new node, an IncludeNode, which doesn't get parsed (the include is still a DirectiveNode), but rather a post parse processing step converts the include DirectiveNode's into IncludeNode's, by parsing the included file. The IncludeNode then has the included files RootNode in it, along with the included path and file object. Then, a serializer plugin, when it encounters the IncludeNode, creates a new context based on the file that the node came from, and renders the nodes children into that - this ensures that included files can still have things like snips with paths relative to the included file. Doing all this exposed an issue - since we expect included markdown files to include directives that may contain things that look like labels, it would have a problem with #149. So I made the snippet parser be able to turn on or off the filtering out of lines containing labels. The include directive turns this off by default, while the snip and other directives turn it on by default. Since that option now existed, I also added support to all of the above directives to configure it, via a `filterLabel` flag. This provides a work around, if not a fix, to #149. --- .../lightbend/paradox/ParadoxProcessor.scala | 48 ++++++++++++++-- .../paradox/markdown/Directive.scala | 57 ++++++++----------- .../paradox/markdown/IncludeNode.scala | 45 +++++++++++++++ .../lightbend/paradox/markdown/Index.scala | 2 + .../lightbend/paradox/markdown/Snippet.scala | 39 +++++++------ .../lightbend/paradox/markdown/Writer.scala | 5 +- .../paradox/markdown/MarkdownTestkit.scala | 9 ++- tests/src/test/resources/example.conf | 17 ++++++ tests/src/test/resources/headers.md | 2 + tests/src/test/resources/include-code-snip.md | 1 + .../markdown/IncludeDirectiveSpec.scala | 25 ++++++++ .../paradox/markdown/SnipDirectiveSpec.scala | 21 +++++++ .../markdown/SnippetIndentationTest.scala | 2 +- .../lightbend/paradox/markdown/example.scala | 7 +++ 14 files changed, 222 insertions(+), 58 deletions(-) create mode 100644 core/src/main/scala/com/lightbend/paradox/markdown/IncludeNode.scala create mode 100644 tests/src/test/resources/example.conf create mode 100644 tests/src/test/resources/headers.md create mode 100644 tests/src/test/resources/include-code-snip.md diff --git a/core/src/main/scala/com/lightbend/paradox/ParadoxProcessor.scala b/core/src/main/scala/com/lightbend/paradox/ParadoxProcessor.scala index 61f7bfc6..4e4262c5 100644 --- a/core/src/main/scala/com/lightbend/paradox/ParadoxProcessor.scala +++ b/core/src/main/scala/com/lightbend/paradox/ParadoxProcessor.scala @@ -20,11 +20,13 @@ import com.lightbend.paradox.template.PageTemplate import com.lightbend.paradox.markdown._ import com.lightbend.paradox.tree.Tree.{ Forest, Location } import java.io.{ File, FileOutputStream, OutputStreamWriter } +import java.util -import org.pegdown.ast.{ ClassyLinkNode, ExpLinkNode, RootNode } +import org.pegdown.ast._ import org.stringtemplate.v4.STErrorListener import scala.annotation.tailrec +import scala.collection.JavaConverters._ /** * Markdown site processor. @@ -175,20 +177,58 @@ class ParadoxProcessor(reader: Reader = new Reader, writer: Writer = new Writer) * Parse markdown files (with paths) into a forest of linked pages. */ def parsePages(mappings: Seq[(File, String)], convertPath: String => String, properties: Map[String, String]): Forest[Page] = { - Page.forest(parseMarkdown(mappings), convertPath, properties) + Page.forest(parseMarkdown(mappings, properties), convertPath, properties) } /** * Parse markdown files into pegdown AST. */ - def parseMarkdown(mappings: Seq[(File, String)]): Seq[(File, String, RootNode, Map[String, String])] = { + def parseMarkdown(mappings: Seq[(File, String)], properties: Map[String, String]): Seq[(File, String, RootNode, Map[String, String])] = { mappings map { case (file, path) => val frontin = Frontin(file) - (file, normalizePath(path), reader.read(frontin.body), frontin.header) + val root = parseAndProcessMarkdown(file, frontin.body, properties ++ frontin.header) + (file, normalizePath(path), root, frontin.header) } } + def parseAndProcessMarkdown(file: File, markdown: String, properties: Map[String, String]): RootNode = { + val root = reader.read(markdown) + processIncludes(file, root, properties) + } + + private def processIncludes(file: File, root: RootNode, properties: Map[String, String]): RootNode = { + val newRoot = new RootNode + // This is a mutable list, and is expected to be mutated by anything that wishes to add children + val newChildren = newRoot.getChildren + + root.getChildren.asScala.foreach { + case include: DirectiveNode if include.name == "include" => + val labels = include.attributes.values("identifier").asScala + val source = include.source match { + case direct: DirectiveNode.Source.Direct => direct.value + case other => throw IncludeDirective.IncludeSourceException(other) + } + val includeFile = SourceDirective.resolveFile("include", source, file, properties) + val frontin = Frontin(includeFile) + val filterLabels = include.attributes.booleanValue("filterLabels", false) + val (text, snippetLang) = Snippet(includeFile, labels, filterLabels) + // I guess we could support multiple markup languages in future... + if (snippetLang != "md" && snippetLang != "markdown") { + throw IncludeDirective.IncludeFormatException(snippetLang) + } + val includedRoot = parseAndProcessMarkdown(includeFile, text, properties ++ frontin.header) + newChildren.add(IncludeNode(includedRoot, includeFile, source)) + + case other => newChildren.add(other) + } + + newRoot.setReferences(root.getReferences) + newRoot.setAbbreviations(root.getAbbreviations) + + newRoot + } + /** * Normalize path to '/' separator. */ diff --git a/core/src/main/scala/com/lightbend/paradox/markdown/Directive.scala b/core/src/main/scala/com/lightbend/paradox/markdown/Directive.scala index 44caaef2..03f133b0 100644 --- a/core/src/main/scala/com/lightbend/paradox/markdown/Directive.scala +++ b/core/src/main/scala/com/lightbend/paradox/markdown/Directive.scala @@ -20,7 +20,6 @@ import com.lightbend.paradox.tree.Tree.Location import java.io.{ File, FileNotFoundException } import java.util.Optional -import com.lightbend.paradox.tree.Tree import org.pegdown.ast._ import org.pegdown.ast.DirectiveNode.Format._ import org.pegdown.plugins.ToHtmlSerializerPlugin @@ -99,18 +98,7 @@ trait SourceDirective { this: Directive => } protected def resolveFile(propPrefix: String, source: String, page: Page, variables: Map[String, String]): File = - source match { - case s if s startsWith "$" => - val baseKey = s.drop(1).takeWhile(_ != '$') - val base = new File(PropertyUrl(s"$propPrefix.$baseKey.base_dir", variables.get).base.trim) - val effectiveBase = if (base.isAbsolute) base else new File(page.file.getParentFile, base.toString) - new File(effectiveBase, s.drop(baseKey.length + 2)) - case s if s startsWith "/" => - val base = new File(PropertyUrl(SnipDirective.buildBaseDir, variables.get).base.trim) - new File(base, s) - case s => - new File(page.file.getParentFile, s) - } + SourceDirective.resolveFile(propPrefix, source, page.file, variables) private lazy val referenceMap: Map[String, ReferenceNode] = { val tempRoot = new RootNode @@ -124,6 +112,22 @@ trait SourceDirective { this: Directive => } } +object SourceDirective { + def resolveFile(propPrefix: String, source: String, pageFile: File, variables: Map[String, String]): File = + source match { + case s if s startsWith "$" => + val baseKey = s.drop(1).takeWhile(_ != '$') + val base = new File(PropertyUrl(s"$propPrefix.$baseKey.base_dir", variables.get).base.trim) + val effectiveBase = if (base.isAbsolute) base else new File(pageFile.getParentFile, base.toString) + new File(effectiveBase, s.drop(baseKey.length + 2)) + case s if s startsWith "/" => + val base = new File(PropertyUrl(SnipDirective.buildBaseDir, variables.get).base.trim) + new File(base, s) + case s => + new File(pageFile.getParentFile, s) + } +} + // Default directives /** @@ -360,8 +364,9 @@ case class SnipDirective(page: Page, variables: Map[String, String]) try { val labels = node.attributes.values("identifier").asScala val source = resolvedSource(node, page) + val filterLabels = node.attributes.booleanValue("filterLabels", true) val file = resolveFile("snip", source, page, variables) - val (text, snippetLang) = Snippet(file, labels) + val (text, snippetLang) = Snippet(file, labels, filterLabels) val lang = Option(node.attributes.value("type")).getOrElse(snippetLang) val group = Option(node.attributes.value("group")).getOrElse("") val sourceUrl = if (variables.contains(GitHubResolver.baseUrl) && variables.getOrElse(SnipDirective.showGithubLinks, "false") == "true") { @@ -412,7 +417,8 @@ case class FiddleDirective(page: Page, variables: Map[String, String]) val source = resolvedSource(node, page) val file = resolveFile("fiddle", source, page, variables) - val (code, _) = Snippet(file, labels) + val filterLabels = node.attributes.booleanValue("filterLabels", true) + val (code, _) = Snippet(file, labels, filterLabels) printer.println.print(s"""
@@ -697,29 +703,14 @@ object DependencyDirective { case class UndefinedVariable(name: String) extends RuntimeException(s"'$name' is not defined") } -case class IncludeDirective(context: Writer.Context) extends LeafBlockDirective("include") with SourceDirective { - - override def page: Page = context.location.tree.label - override def variables = context.properties +case class IncludeDirective(page: Page, variables: Map[String, String]) extends LeafBlockDirective("include") with SourceDirective { override def render(node: DirectiveNode, visitor: Visitor, printer: Printer): Unit = { - val labels = node.attributes.values("identifier").asScala - val source = resolvedSource(node, page) - val file = resolveFile("include", source, page, context.properties) - val (text, snippetLang) = Snippet(file, labels) - // I guess we could support multiple markup languages in future... - if (snippetLang != "md" && snippetLang != "markdown") { - throw IncludeDirective.IncludeFormatException(snippetLang) - } - val includeNode = context.reader.read(text) - - // This location has no forest around it... which probably means that things like toc and navigation can't - // be rendered inside snippets, which I'm ok with. - val newLocation = Location(Tree.leaf(Page.included(file, source, page, includeNode)), Nil, Nil, Nil) - printer.print(context.writer.writeContent(includeNode, context.copy(location = newLocation))) + throw new IllegalStateException("Include directive should have been handled in markdown preprocessing before render, but wasn't.") } } object IncludeDirective { + case class IncludeSourceException(source: DirectiveNode.Source) extends RuntimeException(s"Only explicit links are supported by the include directive, reference links are not: " + source) case class IncludeFormatException(format: String) extends RuntimeException(s"Don't know how to include '*.$format' content.") } diff --git a/core/src/main/scala/com/lightbend/paradox/markdown/IncludeNode.scala b/core/src/main/scala/com/lightbend/paradox/markdown/IncludeNode.scala new file mode 100644 index 00000000..45426bae --- /dev/null +++ b/core/src/main/scala/com/lightbend/paradox/markdown/IncludeNode.scala @@ -0,0 +1,45 @@ +/* + * Copyright © 2015 - 2017 Lightbend, Inc. + * + * Licensed 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 com.lightbend.paradox.markdown + +import java.io.File +import java.util + +import com.lightbend.paradox.markdown.Writer.Context +import com.lightbend.paradox.tree.Tree +import com.lightbend.paradox.tree.Tree.Location +import org.pegdown.Printer +import org.pegdown.ast.{ AbstractNode, Node, RootNode, Visitor } +import org.pegdown.plugins.ToHtmlSerializerPlugin + +case class IncludeNode(included: RootNode, includedFrom: File, includedFromPath: String) extends AbstractNode { + override def accept(visitor: Visitor): Unit = visitor.visit(this) + override def getChildren: util.List[Node] = included.getChildren +} + +class IncludeNodeSerializer(context: Context) extends ToHtmlSerializerPlugin { + override def visit(node: Node, visitor: Visitor, printer: Printer): Boolean = node match { + case IncludeNode(included, includedFrom, includedFromPath) => + // This location has no forest around it... which probably means that things like toc and navigation can't + // be rendered inside snippets, which I'm ok with. + val newLocation = Location(Tree.leaf(Page.included(includedFrom, includedFromPath, + context.location.tree.label, included)), Nil, Nil, Nil) + printer.print(context.writer.writeContent(included, context.copy(location = newLocation))) + true + case _ => false + } +} \ No newline at end of file diff --git a/core/src/main/scala/com/lightbend/paradox/markdown/Index.scala b/core/src/main/scala/com/lightbend/paradox/markdown/Index.scala index 76c2ebb3..fa8ce841 100644 --- a/core/src/main/scala/com/lightbend/paradox/markdown/Index.scala +++ b/core/src/main/scala/com/lightbend/paradox/markdown/Index.scala @@ -65,6 +65,8 @@ object Index { // if so maybe move that cast to DirectiveNode val newGroup = node.attributes.classes().asScala.find(_.startsWith("group-")).map(_.substring("group-".size)) headerRefs(node.contentsNode.asInstanceOf[RootNode], newGroup) + case IncludeNode(included, _, _) => + headerRefs(included, group) case _ => Nil } } diff --git a/core/src/main/scala/com/lightbend/paradox/markdown/Snippet.scala b/core/src/main/scala/com/lightbend/paradox/markdown/Snippet.scala index 92f50e1c..34301f15 100644 --- a/core/src/main/scala/com/lightbend/paradox/markdown/Snippet.scala +++ b/core/src/main/scala/com/lightbend/paradox/markdown/Snippet.scala @@ -25,24 +25,26 @@ object Snippet { class SnippetException(message: String) extends RuntimeException(message) - def apply(file: File, labels: Seq[String]): (String, String) = { + def apply(file: File, labels: Seq[String], filterLabelLines: Boolean): (String, String) = { val source = Source.fromFile(file)("UTF-8") try { val lines = source.getLines.toSeq - (extract(file, lines, labels), language(file)) + (extract(file, lines, labels, filterLabelLines), language(file)) } finally { source.close() } } - def extract(file: File, lines: Seq[String], labels: Seq[String]): String = { + def extract(file: File, lines: Seq[String], labels: Seq[String], filterLabelLines: Boolean): String = { labels match { case Seq() => - val extractionState = extractFrom(lines, _ => true, _ => false, addFilteredLine) - cutIndentation(extractionState.snippetLines) + val addLine = addFilteredLine(filterLabelLines) + cutIndentation(lines.zipWithIndex.foldLeft(Seq.empty[(Int, String)]) { + case (lines, (line, lineIndex)) => addLine(line, lines, lineIndex + 1) + }.map(_._2)) case _ => labels.map { label => - val extractionState = extractState(file, lines, label) + val extractionState = extractState(file, lines, label, filterLabelLines) cutIndentation(extractionState.snippetLines) }.mkString("\n") } @@ -62,11 +64,11 @@ object Snippet { snippetLines.map(ln => dropIndent(minIndent, ln)).mkString("\n") } - def extractLabelRange(file: File, label: String): Option[(Int, Int)] = { + def extractLabelRange(file: File, label: String, filterLabelLines: Boolean = true): Option[(Int, Int)] = { val source = Source.fromFile(file)("UTF-8") try { val lines = source.getLines.toSeq - val lineNumbers = extractState(file, lines, label).lines.map(_._1) + val lineNumbers = extractState(file, lines, label, filterLabelLines).lines.map(_._1) if (lineNumbers.isEmpty) None else @@ -78,7 +80,7 @@ object Snippet { type Line = (Int, String) - private def extractState(file: File, lines: Seq[String], label: String): ExtractionState = { + private def extractState(file: File, lines: Seq[String], label: String, filterLabelLines: Boolean): ExtractionState = { if (!verifyLabel(label)) throw new SnippetException(s"Label [$label] for [$file] contains illegal characters. " + "Only [a-zA-Z0-9_-] are allowed.") // A label can be followed by an end of line or one or more spaces followed by an @@ -86,7 +88,7 @@ object Snippet { // (anything not in the group [a-zA-Z0-9_]) val labelPattern = ("""#\Q""" + label + """\E( +[^w \t]*)?$""").r val hasLabel = (s: String) => labelPattern.findFirstIn(s).nonEmpty - val extractionState = extractFrom(lines, hasLabel, hasLabel, addFilteredLine) + val extractionState = extractFrom(lines, hasLabel, hasLabel, addFilteredLine(filterLabelLines)) if (extractionState.snippetLines.isEmpty) throw new SnippetException(s"Label [$label] not found in [$file]") extractionState.block match { @@ -100,11 +102,9 @@ object Snippet { lines.zipWithIndex.foldLeft(ExtractionState(block = NoBlock, lines = Seq.empty)) { case (es, (l, lineIndex)) => es.block match { - case NoBlock if blockStart(l) => - es.copy(block = InBlock, lines = addLine(l, es.lines, lineIndex + 1)) - case NoBlock => es - case InBlock if blockEnd(l) => - es.copy(block = NoBlock, lines = addLine(l, es.lines, lineIndex + 1)) + case NoBlock if blockStart(l) => es.withBlock(InBlock) + case NoBlock => es + case InBlock if blockEnd(l) => es.withBlock(NoBlock) case InBlock => es.copy(lines = addLine(l, es.lines, lineIndex + 1)) } @@ -126,6 +126,7 @@ object Snippet { private case class ExtractionState(block: Block, lines: Seq[Line]) { def snippetLines = lines.map(_._2) + def withBlock(block: Block) = copy(block = block) } private sealed trait Block @@ -137,9 +138,15 @@ object Snippet { private def containsLabel(line: String): Option[String] = anyLabelRegex.findFirstIn(line) - private def addFilteredLine(line: String, lines: Seq[Line], lineNumber: Int): Seq[Line] = + private def addFilteredLine(filterLabels: Boolean): (String, Seq[Line], Int) => Seq[Line] = + if (filterLabels) addLineFilterLabels else addLineNonFiltered + + private def addLineFilterLabels(line: String, lines: Seq[Line], lineNumber: Int): Seq[Line] = containsLabel(line).map(_ => lines).getOrElse(lines :+ (lineNumber, line)) + private def addLineNonFiltered(line: String, lines: Seq[Line], lineNumber: Int): Seq[Line] = + lines :+ (lineNumber, line) + private def verifyLabel(label: String): Boolean = containsLabel(s"#$label").nonEmpty def language(file: File): String = { diff --git a/core/src/main/scala/com/lightbend/paradox/markdown/Writer.scala b/core/src/main/scala/com/lightbend/paradox/markdown/Writer.scala index 8dae51e3..7e555cb2 100644 --- a/core/src/main/scala/com/lightbend/paradox/markdown/Writer.scala +++ b/core/src/main/scala/com/lightbend/paradox/markdown/Writer.scala @@ -117,7 +117,8 @@ object Writer { def defaultPlugins(directives: Seq[Context => Directive]): Seq[Context => ToHtmlSerializerPlugin] = Seq( context => new ClassyLinkSerializer, context => new AnchorLinkSerializer, - context => new DirectiveSerializer(directives.map(d => d(context))) + context => new DirectiveSerializer(directives.map(d => d(context))), + context => new IncludeNodeSerializer(context) ) def defaultDirectives: Seq[Context => Directive] = Seq( @@ -137,7 +138,7 @@ object Writer { context => InlineWrapDirective("span"), context => InlineGroupDirective(context.groups.values.flatten.map(_.toLowerCase).toSeq), context => DependencyDirective(context.properties), - context => IncludeDirective(context) + context => IncludeDirective(context.location.tree.label, context.properties) ) class DefaultLinkRenderer(context: Context) extends LinkRenderer { diff --git a/testkit/src/main/scala/com/lightbend/paradox/markdown/MarkdownTestkit.scala b/testkit/src/main/scala/com/lightbend/paradox/markdown/MarkdownTestkit.scala index a2f8101b..eef1f409 100644 --- a/testkit/src/main/scala/com/lightbend/paradox/markdown/MarkdownTestkit.scala +++ b/testkit/src/main/scala/com/lightbend/paradox/markdown/MarkdownTestkit.scala @@ -18,13 +18,17 @@ package com.lightbend.paradox.markdown import com.lightbend.paradox.tree.Tree.{ Forest, Location } import java.io.{ File, PrintWriter } + import com.lightbend.paradox.template.PageTemplate import java.nio.file._ +import com.lightbend.paradox.ParadoxProcessor + abstract class MarkdownTestkit { val markdownReader = new Reader val markdownWriter = new Writer + val paradoxProcessor = new ParadoxProcessor(markdownReader, markdownWriter) def markdown(text: String)(implicit context: Location[Page] => Writer.Context = writerContext): String = { markdownPages("test.md" -> text).getOrElse("test.html", "") @@ -96,7 +100,8 @@ abstract class MarkdownTestkit { val parsed = mappings map { case (path, text) => val frontin = Frontin(prepare(text)) - (new File(path), path, markdownReader.read(frontin.body), frontin.header) + val file = new File(path) + (new File(path), path, paradoxProcessor.parseAndProcessMarkdown(file, frontin.body, globalProperties ++ frontin.header), frontin.header) } Page.forest(parsed, Path.replaceSuffix(Writer.DefaultSourceSuffix, Writer.DefaultTargetSuffix), globalProperties) } @@ -123,7 +128,7 @@ abstract class MarkdownTestkit { tidy.setShowWarnings(false) tidy.setQuiet(true) tidy.parse(reader, writer) - writer.toString.replace("\r\n", "\n").replace("\r", "\n") + writer.toString.replace("\r\n", "\n").replace("\r", "\n").trim } case class PartialPageContent(properties: Map[String, String], content: String) extends PageTemplate.Contents { diff --git a/tests/src/test/resources/example.conf b/tests/src/test/resources/example.conf new file mode 100644 index 00000000..979703b2 --- /dev/null +++ b/tests/src/test/resources/example.conf @@ -0,0 +1,17 @@ +# Copyright © 2015 - 2017 Lightbend, Inc. +# +# Licensed 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. + +//#example +a = b +//#example diff --git a/tests/src/test/resources/headers.md b/tests/src/test/resources/headers.md new file mode 100644 index 00000000..477c2131 --- /dev/null +++ b/tests/src/test/resources/headers.md @@ -0,0 +1,2 @@ +## Heading 1 +## Heading 2 \ No newline at end of file diff --git a/tests/src/test/resources/include-code-snip.md b/tests/src/test/resources/include-code-snip.md new file mode 100644 index 00000000..b7a03808 --- /dev/null +++ b/tests/src/test/resources/include-code-snip.md @@ -0,0 +1 @@ +@@snip[example.conf](example.conf) { #example } diff --git a/tests/src/test/scala/com/lightbend/paradox/markdown/IncludeDirectiveSpec.scala b/tests/src/test/scala/com/lightbend/paradox/markdown/IncludeDirectiveSpec.scala index 09353221..82cb1104 100644 --- a/tests/src/test/scala/com/lightbend/paradox/markdown/IncludeDirectiveSpec.scala +++ b/tests/src/test/scala/com/lightbend/paradox/markdown/IncludeDirectiveSpec.scala @@ -50,4 +50,29 @@ class IncludeDirectiveSpec extends MarkdownBaseSpec { |

This file should be included by IncludeDirectiveSpec

""") } + it should "include nested code snippets" in { + markdown("""@@include(tests/src/test/resources/include-code-snip.md)""") shouldEqual html(""" + |
+      |
+      |a = b
+      |
""") + } + + it should "include headers from nested snippets in the toc" in { + markdown( + """ + |@@toc { depth=2 } + | + |# Hello + | + |@@include(tests/src/test/resources/headers.md) + |""") should include(html(""" + |
+ | + |
""")) + } + } diff --git a/tests/src/test/scala/com/lightbend/paradox/markdown/SnipDirectiveSpec.scala b/tests/src/test/scala/com/lightbend/paradox/markdown/SnipDirectiveSpec.scala index 0c01649d..473b4659 100644 --- a/tests/src/test/scala/com/lightbend/paradox/markdown/SnipDirectiveSpec.scala +++ b/tests/src/test/scala/com/lightbend/paradox/markdown/SnipDirectiveSpec.scala @@ -153,4 +153,25 @@ class SnipDirectiveSpec extends MarkdownBaseSpec { |} |""") } + + it should "filter labels by default" in { + markdown("""@@snip[example.scala](tests/src/test/scala/com/lightbend/paradox/markdown/example.scala) { #example-with-label }""") shouldEqual html( + """
+        |
+        |object Constants {
+        |}
+        |
""" + ) + } + + it should "allow including labels if specified" in { + markdown("""@@snip[example.scala](tests/src/test/scala/com/lightbend/paradox/markdown/example.scala) { #example-with-label filterLabels=false }""") shouldEqual html( + """
+        |
+        |object Constants {
+        |  val someString = " #foo "
+        |}
+        |
""" + ) + } } diff --git a/tests/src/test/scala/com/lightbend/paradox/markdown/SnippetIndentationTest.scala b/tests/src/test/scala/com/lightbend/paradox/markdown/SnippetIndentationTest.scala index df7039ef..fd770b34 100644 --- a/tests/src/test/scala/com/lightbend/paradox/markdown/SnippetIndentationTest.scala +++ b/tests/src/test/scala/com/lightbend/paradox/markdown/SnippetIndentationTest.scala @@ -151,6 +151,6 @@ class SnippetIndentationTest extends FlatSpec with Matchers { def extractToString(inString: String, label: String, indentationPerSnippet: Boolean = true): String = { val in = inString.split("\n").toList - Snippet.extract(new File(""), in, Seq(label)) + Snippet.extract(new File(""), in, Seq(label), true) } } diff --git a/tests/src/test/scala/com/lightbend/paradox/markdown/example.scala b/tests/src/test/scala/com/lightbend/paradox/markdown/example.scala index 690a061b..0116ddb6 100644 --- a/tests/src/test/scala/com/lightbend/paradox/markdown/example.scala +++ b/tests/src/test/scala/com/lightbend/paradox/markdown/example.scala @@ -73,3 +73,10 @@ class AnotherClass //#multi-indented-example // format: ON + + +//#example-with-label +object Constants { + val someString = " #foo " +} +//#example-with-label \ No newline at end of file From de2eed1c0ca1526a6ac4569dc3641ce0e1b65bc4 Mon Sep 17 00:00:00 2001 From: James Roper Date: Fri, 15 Feb 2019 10:42:36 +1100 Subject: [PATCH 2/3] Fixed toc handling with includes This fixes the toc handling so that the headersBelow function takes into consideration whether a header from an included file is below the toc directive, and vice versa, if the toc directive is in an included file, will correctly work out if a header in the including file is below it. It does this by tracking nested index locations of the includes. Also fixed the context propagation of rendering included sections so that included sections can contain a toc. --- .../lightbend/paradox/ParadoxProcessor.scala | 5 ++- .../paradox/markdown/Directive.scala | 4 +-- .../paradox/markdown/IncludeNode.scala | 8 +++-- .../lightbend/paradox/markdown/Index.scala | 26 +++++++++----- .../com/lightbend/paradox/markdown/Page.scala | 12 +++---- .../paradox/markdown/TableOfContents.scala | 31 +++++++++++++---- .../lightbend/paradox/markdown/Writer.scala | 22 ++++++------ tests/src/test/resources/toc.md | 9 +++++ .../markdown/IncludeDirectiveSpec.scala | 34 +++++++++++++++++-- 9 files changed, 111 insertions(+), 40 deletions(-) create mode 100644 tests/src/test/resources/toc.md diff --git a/core/src/main/scala/com/lightbend/paradox/ParadoxProcessor.scala b/core/src/main/scala/com/lightbend/paradox/ParadoxProcessor.scala index 4e4262c5..953de051 100644 --- a/core/src/main/scala/com/lightbend/paradox/ParadoxProcessor.scala +++ b/core/src/main/scala/com/lightbend/paradox/ParadoxProcessor.scala @@ -218,7 +218,10 @@ class ParadoxProcessor(reader: Reader = new Reader, writer: Writer = new Writer) throw IncludeDirective.IncludeFormatException(snippetLang) } val includedRoot = parseAndProcessMarkdown(includeFile, text, properties ++ frontin.header) - newChildren.add(IncludeNode(includedRoot, includeFile, source)) + val includeNode = IncludeNode(includedRoot, includeFile, source) + includeNode.setStartIndex(include.getStartIndex) + includeNode.setEndIndex(include.getEndIndex) + newChildren.add(includeNode) case other => newChildren.add(other) } diff --git a/core/src/main/scala/com/lightbend/paradox/markdown/Directive.scala b/core/src/main/scala/com/lightbend/paradox/markdown/Directive.scala index 03f133b0..02b960db 100644 --- a/core/src/main/scala/com/lightbend/paradox/markdown/Directive.scala +++ b/core/src/main/scala/com/lightbend/paradox/markdown/Directive.scala @@ -450,7 +450,7 @@ object FiddleDirective { * Placeholder to insert a serialized table of contents, using the page and header trees. * Depth and whether to include pages or headers can be specified in directive attributes. */ -case class TocDirective(location: Location[Page]) extends LeafBlockDirective("toc") { +case class TocDirective(location: Location[Page], includeIndexes: List[Int]) extends LeafBlockDirective("toc") { def render(node: DirectiveNode, visitor: Visitor, printer: Printer): Unit = { val classes = node.attributes.classesString val depth = node.attributes.intValue("depth", 6) @@ -459,7 +459,7 @@ case class TocDirective(location: Location[Page]) extends LeafBlockDirective("to val ordered = node.attributes.booleanValue("ordered", false) val toc = new TableOfContents(pages, headers, ordered, depth) printer.println.print(s"""
""") - toc.markdown(location, node.getStartIndex).accept(visitor) + toc.markdown(location, node.getStartIndex, includeIndexes).accept(visitor) printer.println.print("
") } } diff --git a/core/src/main/scala/com/lightbend/paradox/markdown/IncludeNode.scala b/core/src/main/scala/com/lightbend/paradox/markdown/IncludeNode.scala index 45426bae..3cff608f 100644 --- a/core/src/main/scala/com/lightbend/paradox/markdown/IncludeNode.scala +++ b/core/src/main/scala/com/lightbend/paradox/markdown/IncludeNode.scala @@ -33,12 +33,14 @@ case class IncludeNode(included: RootNode, includedFrom: File, includedFromPath: class IncludeNodeSerializer(context: Context) extends ToHtmlSerializerPlugin { override def visit(node: Node, visitor: Visitor, printer: Printer): Boolean = node match { - case IncludeNode(included, includedFrom, includedFromPath) => + case include @ IncludeNode(included, includedFrom, includedFromPath) => // This location has no forest around it... which probably means that things like toc and navigation can't // be rendered inside snippets, which I'm ok with. val newLocation = Location(Tree.leaf(Page.included(includedFrom, includedFromPath, - context.location.tree.label, included)), Nil, Nil, Nil) - printer.print(context.writer.writeContent(included, context.copy(location = newLocation))) + context.location.tree.label, included)), context.location.lefts, context.location.rights, context.location.parents) + printer.print(context.writer.writeContent(included, context.copy( + location = newLocation, + includeIndexes = context.includeIndexes :+ include.getStartIndex))) true case _ => false } diff --git a/core/src/main/scala/com/lightbend/paradox/markdown/Index.scala b/core/src/main/scala/com/lightbend/paradox/markdown/Index.scala index fa8ce841..c83fa775 100644 --- a/core/src/main/scala/com/lightbend/paradox/markdown/Index.scala +++ b/core/src/main/scala/com/lightbend/paradox/markdown/Index.scala @@ -28,7 +28,15 @@ import scala.collection.JavaConverters._ */ object Index { - case class Ref(level: Int, path: String, markdown: Node, group: Option[String]) + /** + * @param level + * @param path + * @param markdown + * @param group + * @param includeIndexes If this header came from an included file, this has the index of the include file, + * starting from the top level page include, down to the deepest nesting. + */ + case class Ref(level: Int, path: String, markdown: Node, group: Option[String], includeIndexes: List[Int]) case class Page(file: File, path: String, markdown: RootNode, properties: Map[String, String], indices: Forest[Ref], headers: Forest[Ref]) @@ -46,27 +54,27 @@ object Index { * Create a tree of header refs from a parsed markdown page. */ def headers(root: RootNode): Forest[Ref] = { - Tree.hierarchy(headerRefs(root, group = None))(Ordering[Int].on[Ref](_.level)) + Tree.hierarchy(headerRefs(root, group = None, includeIndexes = Nil))(Ordering[Int].on[Ref](_.level)) } /** * Extract refs from markdown headers. */ - private def headerRefs(root: RootNode, group: Option[String]): List[Ref] = { + private def headerRefs(root: RootNode, group: Option[String], includeIndexes: List[Int]): List[Ref] = { root.getChildren.asScala.toList.flatMap { case header: HeaderNode => header.getChildren.asScala.toList.flatMap { - case anchor: AnchorLinkSuperNode => List(Ref(header.getLevel, "#" + anchor.name, anchor.contents, group)) - case anchor: AnchorLinkNode => List(Ref(header.getLevel, "#" + anchor.getName, new TextNode(anchor.getText), group)) + case anchor: AnchorLinkSuperNode => List(Ref(header.getLevel, "#" + anchor.name, anchor.contents, group, includeIndexes)) + case anchor: AnchorLinkNode => List(Ref(header.getLevel, "#" + anchor.getName, new TextNode(anchor.getText), group, includeIndexes)) case _ => Nil } case node: DirectiveNode if node.format == DirectiveNode.Format.ContainerBlock => // TODO check whether my assumption that Container DirectiveNode's always contain RootNode's holds, // if so maybe move that cast to DirectiveNode val newGroup = node.attributes.classes().asScala.find(_.startsWith("group-")).map(_.substring("group-".size)) - headerRefs(node.contentsNode.asInstanceOf[RootNode], newGroup) - case IncludeNode(included, _, _) => - headerRefs(included, group) + headerRefs(node.contentsNode.asInstanceOf[RootNode], newGroup, includeIndexes) + case node @ IncludeNode(included, _, _) => + headerRefs(included, group, includeIndexes :+ node.getStartIndex) case _ => Nil } } @@ -111,7 +119,7 @@ object Index { @tailrec private def linkRef(node: Node, level: Int): Option[Ref] = { node match { - case link: ExpLinkNode => Some(Ref(level, link.url, link.getChildren.get(0), group = None)) + case link: ExpLinkNode => Some(Ref(level, link.url, link.getChildren.get(0), group = None, Nil)) case other => other.getChildren.asScala.toList match { // only check first children case first :: _ => linkRef(first, level) diff --git a/core/src/main/scala/com/lightbend/paradox/markdown/Page.scala b/core/src/main/scala/com/lightbend/paradox/markdown/Page.scala index 5dff5466..2c4f5f01 100644 --- a/core/src/main/scala/com/lightbend/paradox/markdown/Page.scala +++ b/core/src/main/scala/com/lightbend/paradox/markdown/Page.scala @@ -36,7 +36,7 @@ sealed abstract class Linkable { /** * Header in a page, with anchor path and markdown nodes. */ -case class Header(path: String, label: Node, group: Option[String]) extends Linkable +case class Header(path: String, label: Node, group: Option[String], includeIndexes: List[Int]) extends Linkable /** * Markdown page with target path, parsed markdown, and headers. @@ -94,10 +94,10 @@ object Page { val targetPath = properties.convertToTarget(convertPath)(page.path) val rootSrcPage = Path.relativeRootPath(page.file, page.path) val (h1, subheaders) = page.headers match { - case h :: hs => (Header(h.label.path, h.label.markdown, h.label.group), h.children ++ hs) - case Nil => (Header(targetPath, new SpecialTextNode(targetPath), None), Nil) + case h :: hs => (Header(h.label.path, h.label.markdown, h.label.group, h.label.includeIndexes), h.children ++ hs) + case Nil => (Header(targetPath, new SpecialTextNode(targetPath), None, Nil), Nil) } - val headers = subheaders map (_ map (h => Header(h.path, h.markdown, h.group))) + val headers = subheaders map (_ map (h => Header(h.path, h.markdown, h.group, h.includeIndexes))) Page(page.file, targetPath, rootSrcPage, h1.label, h1, headers, page.markdown, h1.group, properties) } @@ -149,8 +149,8 @@ object Page { */ def included(file: File, includeFilePath: String, includedIn: Page, markdown: RootNode): Page = { val rootSrcPage = Path.relativeRootPath(file, includeFilePath) - val h1 = Header(includedIn.path, new SpecialTextNode(includedIn.path), None) - Page(file, includedIn.path, rootSrcPage, h1.label, h1, Nil, markdown, h1.group, includedIn.properties) + Page(file, includedIn.path, rootSrcPage, includedIn.h1.label, includedIn.h1, includedIn.headers, markdown, + includedIn.group, includedIn.properties) } } diff --git a/core/src/main/scala/com/lightbend/paradox/markdown/TableOfContents.scala b/core/src/main/scala/com/lightbend/paradox/markdown/TableOfContents.scala index f0f2685d..0215442f 100644 --- a/core/src/main/scala/com/lightbend/paradox/markdown/TableOfContents.scala +++ b/core/src/main/scala/com/lightbend/paradox/markdown/TableOfContents.scala @@ -35,8 +35,14 @@ class TableOfContents(pages: Boolean = true, headers: Boolean = true, ordered: B /** * Create a TOC bullet list for a TOC at a certain point within the section hierarchy. */ - def markdown(location: Location[Page], tocIndex: Int): Node = { - markdown(location.tree.label.base, Some(location), nested(location.tree, tocIndex)) + @deprecated("0.5.1", "Use the includeIndexes variant to ensure it works with include files") + def markdown(location: Location[Page], tocIndex: Int): Node = markdown(location, tocIndex, Nil) + + /** + * Create a TOC bullet list for a TOC at a certain point within the section hierarchy. + */ + def markdown(location: Location[Page], tocIndex: Int, includeIndexes: List[Int]): Node = { + markdown(location.tree.label.base, Some(location), nested(location.tree, tocIndex, includeIndexes)) } /** @@ -65,9 +71,9 @@ class TableOfContents(pages: Boolean = true, headers: Boolean = true, ordered: B /** * Create a new Page Tree for a TOC at a certain point within the section hierarchy. */ - private def nested(tree: Tree[Page], tocIndex: Int): Tree[Page] = { + private def nested(tree: Tree[Page], tocIndex: Int, includeIndexes: List[Int]): Tree[Page] = { val page = tree.label - val (level, headers) = headersBelow(Location.forest(page.headers), tocIndex) + val (level, headers) = headersBelow(Location.forest(page.headers), tocIndex, includeIndexes) val subPages = if (level == 0) tree.children else Nil Tree(page.copy(headers = headers), subPages) } @@ -76,13 +82,24 @@ class TableOfContents(pages: Boolean = true, headers: Boolean = true, ordered: B * Find the headers below the buffer index for a toc directive. * Return the level of the next header and sub-headers to render. */ - private def headersBelow(location: Option[Location[Header]], index: Int): (Int, Forest[Header]) = location match { + private def headersBelow(location: Option[Location[Header]], index: Int, includeIndexes: List[Int]): (Int, Forest[Header]) = location match { case Some(loc) => - if (loc.tree.label.label.getStartIndex > index) (loc.depth, loc.tree :: loc.rights) - else headersBelow(loc.next, index) + if (isBelow(index, includeIndexes, loc.tree.label.label.getStartIndex, loc.tree.label.includeIndexes)) + (loc.depth, loc.tree :: loc.rights) + else headersBelow(loc.next, index, includeIndexes) case None => (0, Nil) } + private def isBelow(tocIndex: Int, tocIncludeIndexes: List[Int], headerIndex: Int, headerIncludeIndexes: List[Int]): Boolean = { + // If the current level of include indexes are equal, then we need to recursively check the next level. + // Otherwise, we compare the current level of include indexes if they exist, or the current indexes themselves. + (tocIncludeIndexes, headerIncludeIndexes) match { + case (i :: itail, h :: htail) if i == h => isBelow(tocIndex, itail, headerIndex, htail) + case _ => + headerIncludeIndexes.headOption.getOrElse(headerIndex) > tocIncludeIndexes.headOption.getOrElse(tocIndex) + } + } + private def subList[A <: Linkable](base: String, active: Option[Location[Page]], tree: Tree[A], depth: Int, expandDepth: Option[Int]): Option[Node] = { tree.label match { case page: Page => diff --git a/core/src/main/scala/com/lightbend/paradox/markdown/Writer.scala b/core/src/main/scala/com/lightbend/paradox/markdown/Writer.scala index 7e555cb2..2a30cc24 100644 --- a/core/src/main/scala/com/lightbend/paradox/markdown/Writer.scala +++ b/core/src/main/scala/com/lightbend/paradox/markdown/Writer.scala @@ -94,15 +94,17 @@ object Writer { * Write context which is passed through to directives. */ case class Context( - location: Location[Page], - paths: Set[String], - reader: Reader, - writer: Writer, - pageMappings: String => String = Path.replaceExtension(DefaultSourceSuffix, DefaultTargetSuffix), - sourceSuffix: String = DefaultSourceSuffix, - targetSuffix: String = DefaultTargetSuffix, - groups: Map[String, Seq[String]] = Map.empty, - properties: Map[String, String] = Map.empty) + location: Location[Page], + paths: Set[String], + reader: Reader, + writer: Writer, + pageMappings: String => String = Path.replaceExtension(DefaultSourceSuffix, DefaultTargetSuffix), + sourceSuffix: String = DefaultSourceSuffix, + targetSuffix: String = DefaultTargetSuffix, + groups: Map[String, Seq[String]] = Map.empty, + properties: Map[String, String] = Map.empty, + includeIndexes: List[Int] = Nil + ) def defaultLinks(context: Context): LinkRenderer = new DefaultLinkRenderer(context) @@ -129,7 +131,7 @@ object Writer { context => GitHubDirective(context.location.tree.label, context.properties), context => SnipDirective(context.location.tree.label, context.properties), context => FiddleDirective(context.location.tree.label, context.properties), - context => TocDirective(context.location), + context => TocDirective(context.location, context.includeIndexes), context => VarDirective(context.properties), context => VarsDirective(context.properties), context => CalloutDirective("note", "Note"), diff --git a/tests/src/test/resources/toc.md b/tests/src/test/resources/toc.md new file mode 100644 index 00000000..6aeb98b5 --- /dev/null +++ b/tests/src/test/resources/toc.md @@ -0,0 +1,9 @@ + +## This shouldn't be included since it's not below the toc + +More text here to ensure that the headers in the outer file which are below this pages +inclusion but with a higher start index do still get included. + +Blah blah blah blah blah blah blah. + +@@toc { depth=1 } diff --git a/tests/src/test/scala/com/lightbend/paradox/markdown/IncludeDirectiveSpec.scala b/tests/src/test/scala/com/lightbend/paradox/markdown/IncludeDirectiveSpec.scala index 82cb1104..7ec85948 100644 --- a/tests/src/test/scala/com/lightbend/paradox/markdown/IncludeDirectiveSpec.scala +++ b/tests/src/test/scala/com/lightbend/paradox/markdown/IncludeDirectiveSpec.scala @@ -61,16 +61,46 @@ class IncludeDirectiveSpec extends MarkdownBaseSpec { it should "include headers from nested snippets in the toc" in { markdown( """ - |@@toc { depth=2 } + |# Page heading | - |# Hello + |This text appears here to push down the toc so to ensure that the headers below + |calculation works. + | + |@@toc { depth=1 } | |@@include(tests/src/test/resources/headers.md) + | + |## Heading 3 + | + |""") should include(html(""" + |
+ | + |
""")) + } + + it should "include headers from outer snippets in a nested toc" in { + markdown( + """ + |# Page heading + | + |## Above toc + | + |@@include(tests/src/test/resources/toc.md) + | + |## Heading 1 + |## Heading 2 + |## Heading 3 + | |""") should include(html(""" |
| |
""")) } From 2c12d4b8b08f719f56ba448185e3f68ec8f35bde Mon Sep 17 00:00:00 2001 From: James Roper Date: Mon, 18 Feb 2019 10:14:24 +1100 Subject: [PATCH 3/3] Made filterLabels configurable globally and per file --- .../main/scala/com/lightbend/paradox/ParadoxProcessor.scala | 4 +++- .../main/scala/com/lightbend/paradox/markdown/Directive.scala | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/core/src/main/scala/com/lightbend/paradox/ParadoxProcessor.scala b/core/src/main/scala/com/lightbend/paradox/ParadoxProcessor.scala index 953de051..8ab289fa 100644 --- a/core/src/main/scala/com/lightbend/paradox/ParadoxProcessor.scala +++ b/core/src/main/scala/com/lightbend/paradox/ParadoxProcessor.scala @@ -211,7 +211,9 @@ class ParadoxProcessor(reader: Reader = new Reader, writer: Writer = new Writer) } val includeFile = SourceDirective.resolveFile("include", source, file, properties) val frontin = Frontin(includeFile) - val filterLabels = include.attributes.booleanValue("filterLabels", false) + val filterLabels = include.attributes.booleanValue( + "filterLabels", + properties.get("include.filterLabels").exists(_ == "true")) val (text, snippetLang) = Snippet(includeFile, labels, filterLabels) // I guess we could support multiple markup languages in future... if (snippetLang != "md" && snippetLang != "markdown") { diff --git a/core/src/main/scala/com/lightbend/paradox/markdown/Directive.scala b/core/src/main/scala/com/lightbend/paradox/markdown/Directive.scala index 02b960db..6c76a1c3 100644 --- a/core/src/main/scala/com/lightbend/paradox/markdown/Directive.scala +++ b/core/src/main/scala/com/lightbend/paradox/markdown/Directive.scala @@ -364,7 +364,7 @@ case class SnipDirective(page: Page, variables: Map[String, String]) try { val labels = node.attributes.values("identifier").asScala val source = resolvedSource(node, page) - val filterLabels = node.attributes.booleanValue("filterLabels", true) + val filterLabels = node.attributes.booleanValue("filterLabels", variables.get("snip.filterLabels").forall(_ == "true")) val file = resolveFile("snip", source, page, variables) val (text, snippetLang) = Snippet(file, labels, filterLabels) val lang = Option(node.attributes.value("type")).getOrElse(snippetLang) @@ -417,7 +417,7 @@ case class FiddleDirective(page: Page, variables: Map[String, String]) val source = resolvedSource(node, page) val file = resolveFile("fiddle", source, page, variables) - val filterLabels = node.attributes.booleanValue("filterLabels", true) + val filterLabels = node.attributes.booleanValue("filterLabels", variables.get("fiddle.filterLabels").forall(_ == "true")) val (code, _) = Snippet(file, labels, filterLabels) printer.println.print(s"""