You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by "olabusayoT (via GitHub)" <gi...@apache.org> on 2024/10/15 22:40:02 UTC

[PR] Add SBT task to generate tunables doc [daffodil]

olabusayoT opened a new pull request, #1340:
URL: https://github.com/apache/daffodil/pull/1340

   - currently we hardcode the description of tunables on daffodil-site. This list is out of date. Instead, the fix uses an sbt task 'genTunablesDoc' to generate a markdown document from the dafext.xsd and outputs it to target/tunables.md
   - related daffodil-site PR https://github.com/apache/daffodil-site/pull/148
   
   DAFFODIL-2758


-- 
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: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add SBT task to generate tunables doc [daffodil]

Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on code in PR #1340:
URL: https://github.com/apache/daffodil/pull/1340#discussion_r1803248974


##########
build.sbt:
##########
@@ -481,6 +489,99 @@ lazy val unidocSettings =
       (JavaUnidoc / unidoc / unidocAllSources).value.map(apiDocSourceFilter)
   )
 
+lazy val genTunablesDocSettings = Seq(
+  Compile / genTunablesDoc := {
+    val dafExtFile =
+      (propgen / Compile / resources).value.filter(_.getName == "dafext.xsd").head
+    val outputDocFile = (Compile / target).value / "tunables.md"
+    // parse xsd file
+    val dafExtXml = scala.xml.XML.loadFile(dafExtFile)
+    // extract information
+    val tunablesSeq = (dafExtXml \ "element" \\ "element")
+    // build documentation
+    val sectionsSeq = tunablesSeq.flatMap { ele =>
+      val subtitle = ele \@ "name"
+      val documentation = (ele \ "annotation" \ "documentation").text
+      val sections =
+        if (documentation.nonEmpty && !documentation.trim.startsWith("Deprecated")) {
+          val s =
+            s"""
+            |#### $subtitle
+            |$documentation

Review Comment:
   I noticed in the daffodil-site PR that $documentation includes indentation from the dafext.xsd, which makes it show up as code. We probably want to strip that off so it shows up as normal text.



-- 
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: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add SBT task to generate tunables doc [daffodil]

Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on code in PR #1340:
URL: https://github.com/apache/daffodil/pull/1340#discussion_r1803250841


##########
build.sbt:
##########
@@ -481,6 +489,99 @@ lazy val unidocSettings =
       (JavaUnidoc / unidoc / unidocAllSources).value.map(apiDocSourceFilter)
   )
 
+lazy val genTunablesDocSettings = Seq(
+  Compile / genTunablesDoc := {
+    val dafExtFile =
+      (propgen / Compile / resources).value.filter(_.getName == "dafext.xsd").head
+    val outputDocFile = (Compile / target).value / "tunables.md"
+    // parse xsd file
+    val dafExtXml = scala.xml.XML.loadFile(dafExtFile)
+    // extract information
+    val tunablesSeq = (dafExtXml \ "element" \\ "element")
+    // build documentation
+    val sectionsSeq = tunablesSeq.flatMap { ele =>
+      val subtitle = ele \@ "name"
+      val documentation = (ele \ "annotation" \ "documentation").text
+      val sections =
+        if (documentation.nonEmpty && !documentation.trim.startsWith("Deprecated")) {
+          val s =
+            s"""
+            |#### $subtitle
+            |$documentation
+            |""".stripMargin
+          Some(s)
+        } else {
+          None
+        }
+      sections
+    }
+    val documentationPage =
+      s"""|---
+        |layout: page
+        |title: DFDL Tunables

Review Comment:
   Suggest either `Daffodil Tunables` since these aren't part of the DFDL spec, or just `Tunables`, since maybe these being for Daffodil is implied.



-- 
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: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add SBT task to generate tunables doc [daffodil]

Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on code in PR #1340:
URL: https://github.com/apache/daffodil/pull/1340#discussion_r1810797111


##########
build.sbt:
##########
@@ -481,6 +489,106 @@ lazy val unidocSettings =
       (JavaUnidoc / unidoc / unidocAllSources).value.map(apiDocSourceFilter)
   )
 
+lazy val genTunablesDocSettings = Seq(
+  Compile / genTunablesDoc := {
+    val stream = (propgen / streams).value
+    val dafExtFile =
+      (propgen / Compile / resources).value.find(_.getName == "dafext.xsd").get
+    val outputDocFile = (Compile / target).value / "tunables.md"
+    // parse xsd file
+    val dafExtXml = scala.xml.XML.loadFile(dafExtFile)
+    // extract tunables information
+    val tunablesElements =
+      (dafExtXml \ "element").filter(_ \@ "name" == "tunables") \\ "all" \ "element"
+    // build documentation
+    val documentationTuple = tunablesElements.map { ele =>
+      val subtitle = ele \@ "name"
+      val documentation = (ele \ "annotation" \ "documentation").text.trim.replaceAll(" +", " ")
+      val default = ele \@ "default"
+      (subtitle, documentation, default)
+    }
+    val (deprecated, nonDeprecated) = documentationTuple.partition { t =>
+      t._2.startsWith("Deprecated")
+    }
+    if (nonDeprecated.isEmpty) {
+      sys.error("tunables document generation failed as non-deprecated elements list is empty")

Review Comment:
   This just outputs a log message but doesn't actually fail the task. I've found the best way to create a failure with an error message is to replace `sys.error` with this:
   ```scala
   throw new MessageOnlyException("error message here")
   ```
   It also means you can move the following code out of the else block.



##########
build.sbt:
##########
@@ -481,6 +489,106 @@ lazy val unidocSettings =
       (JavaUnidoc / unidoc / unidocAllSources).value.map(apiDocSourceFilter)
   )
 
+lazy val genTunablesDocSettings = Seq(
+  Compile / genTunablesDoc := {
+    val stream = (propgen / streams).value
+    val dafExtFile =
+      (propgen / Compile / resources).value.find(_.getName == "dafext.xsd").get
+    val outputDocFile = (Compile / target).value / "tunables.md"
+    // parse xsd file
+    val dafExtXml = scala.xml.XML.loadFile(dafExtFile)
+    // extract tunables information
+    val tunablesElements =
+      (dafExtXml \ "element").filter(_ \@ "name" == "tunables") \\ "all" \ "element"
+    // build documentation
+    val documentationTuple = tunablesElements.map { ele =>
+      val subtitle = ele \@ "name"
+      val documentation = (ele \ "annotation" \ "documentation").text.trim.replaceAll(" +", " ")

Review Comment:
   Suggest doing `(ele \ "annotation" \ "documentation").text.split("\n").map(_.trim).mkString("\n")`. Triming each individual line will avoid a space at the beginning of lines. I *think* the extra space shouldn't affect the markdown since it's just a single space, but for a format that is somewhat space-sensitive it's probably best to ensure there are no leading spaces.



-- 
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: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add SBT task to generate tunables doc [daffodil]

Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on code in PR #1340:
URL: https://github.com/apache/daffodil/pull/1340#discussion_r1803031352


##########
build.sbt:
##########
@@ -481,6 +489,99 @@ lazy val unidocSettings =
       (JavaUnidoc / unidoc / unidocAllSources).value.map(apiDocSourceFilter)
   )
 
+lazy val genTunablesDocSettings = Seq(
+  Compile / genTunablesDoc := {
+    val dafExtFile =
+      (propgen / Compile / resources).value.filter(_.getName == "dafext.xsd").head
+    val outputDocFile = (Compile / target).value / "tunables.md"
+    // parse xsd file
+    val dafExtXml = scala.xml.XML.loadFile(dafExtFile)
+    // extract information
+    val tunablesSeq = (dafExtXml \ "element" \\ "element")

Review Comment:
   This could break if we ever add some new thing to dafext.xsd that has nested elements. Currently that's only tunables, but I could easily imagine a case where we add a more complex element.  I think something like this will do it:
   ```scala
   (dafExtXml \ "element").filter(_ \@ "name" == "tunables") \\ "element"
   ```



##########
build.sbt:
##########
@@ -481,6 +489,99 @@ lazy val unidocSettings =
       (JavaUnidoc / unidoc / unidocAllSources).value.map(apiDocSourceFilter)
   )
 
+lazy val genTunablesDocSettings = Seq(
+  Compile / genTunablesDoc := {
+    val dafExtFile =
+      (propgen / Compile / resources).value.filter(_.getName == "dafext.xsd").head
+    val outputDocFile = (Compile / target).value / "tunables.md"
+    // parse xsd file
+    val dafExtXml = scala.xml.XML.loadFile(dafExtFile)
+    // extract information
+    val tunablesSeq = (dafExtXml \ "element" \\ "element")
+    // build documentation
+    val sectionsSeq = tunablesSeq.flatMap { ele =>
+      val subtitle = ele \@ "name"
+      val documentation = (ele \ "annotation" \ "documentation").text
+      val sections =
+        if (documentation.nonEmpty && !documentation.trim.startsWith("Deprecated")) {
+          val s =
+            s"""
+            |#### $subtitle
+            |$documentation
+            |""".stripMargin
+          Some(s)
+        } else {
+          None
+        }
+      sections
+    }
+    val documentationPage =
+      s"""|---
+        |layout: page
+        |title: DFDL Tunables
+        |group: nav-right
+        |---
+        |<!--
+        |{% comment %}
+        |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.
+        |{% endcomment %}
+        |-->
+        |<!--
+        |{% comment %}
+        |This file is generated using ``sbt genTunablesDoc``. Update that task in Daffodil to update this file.
+        |{% endcomment %}
+        |-->
+        |
+        |Daffodil provides tunables as a way to change its behavior.
+        |Tunables are set by way of the ``tunables`` element in [config files](/configuration)
+        |or from the [cli](/cli) via the ``-T`` option.
+        |
+        |#### Config Example
+        | ``` xml
+        | <daf:dfdlConfig
+        |	xmlns:daf="urn:ogf:dfdl:2013:imp:daffodil.apache.org:2018:ext">
+        |    <daf:tunables>
+        |      <daf:suppressSchemaDefinitionWarnings>
+        |        encodingErrorPolicyError
+        |      </daf:suppressSchemaDefinitionWarnings>
+        |    </daf:tunables>
+        |</daf:dfdlConfig>
+        | ```
+        |
+        | The config file can then be passed into daffodil subcommands via the ``-c|--config`` options.
+        |
+        |#### CLI Example
+        | ``` bash
+        | daffodil parse -s schema.xsd -TsuppressSchemaDefinitionWarnings="encodingErrorPolicyError" data.bin
+        | ```
+        |
+        |
+        |### Definitions
+        |${sectionsSeq.mkString("\n")}
+        |""".stripMargin
+    IO.write(outputDocFile, s"$documentationPage")
+    println(s"Generated tunables documentation at: $outputDocFile")
+  },
+  Compile / compile := {
+    val res = (Compile / compile).value
+    (Compile / genTunablesDoc).value
+    res
+  }

Review Comment:
   I'd suggest we don't build tunables everytime we compile, we don't really need to generate tunables that often.
   
   However, I would suggest we add `genTunablesDoc` to the "Build Documentation" step in GitHub actions to ensure we don't accidentally break this somehow.
   
   Also, instead of having to manually update the web site, can you modify the daffodil release candidate container to run `genTunablesDoc` and copy the file to the right place in the site repo (we already do something similar for tutorial files)? That way every time we do a release we'll automatically install the updated docs and don't have to remember to do it manually.



##########
build.sbt:
##########
@@ -481,6 +489,99 @@ lazy val unidocSettings =
       (JavaUnidoc / unidoc / unidocAllSources).value.map(apiDocSourceFilter)
   )
 
+lazy val genTunablesDocSettings = Seq(
+  Compile / genTunablesDoc := {
+    val dafExtFile =
+      (propgen / Compile / resources).value.filter(_.getName == "dafext.xsd").head

Review Comment:
   Use `.find(...).get` instead of `.filter(...).head` if we are just going to get the first one and know it will exist.



##########
build.sbt:
##########
@@ -481,6 +489,99 @@ lazy val unidocSettings =
       (JavaUnidoc / unidoc / unidocAllSources).value.map(apiDocSourceFilter)
   )
 
+lazy val genTunablesDocSettings = Seq(
+  Compile / genTunablesDoc := {
+    val dafExtFile =
+      (propgen / Compile / resources).value.filter(_.getName == "dafext.xsd").head
+    val outputDocFile = (Compile / target).value / "tunables.md"
+    // parse xsd file
+    val dafExtXml = scala.xml.XML.loadFile(dafExtFile)
+    // extract information
+    val tunablesSeq = (dafExtXml \ "element" \\ "element")
+    // build documentation
+    val sectionsSeq = tunablesSeq.flatMap { ele =>
+      val subtitle = ele \@ "name"
+      val documentation = (ele \ "annotation" \ "documentation").text
+      val sections =
+        if (documentation.nonEmpty && !documentation.trim.startsWith("Deprecated")) {

Review Comment:
   That's probably  value in including deprecated tunables. If you don't want to include them in the main list maybe you could have a "Deprecated" sectdion at the end that lists the deprecated ones? That way if anyone ever comes across a tunable and doesn't know what it does, then at least there a record that it exists and doesn't actually do anything.



##########
build.sbt:
##########
@@ -481,6 +489,99 @@ lazy val unidocSettings =
       (JavaUnidoc / unidoc / unidocAllSources).value.map(apiDocSourceFilter)
   )
 
+lazy val genTunablesDocSettings = Seq(
+  Compile / genTunablesDoc := {
+    val dafExtFile =
+      (propgen / Compile / resources).value.filter(_.getName == "dafext.xsd").head
+    val outputDocFile = (Compile / target).value / "tunables.md"
+    // parse xsd file
+    val dafExtXml = scala.xml.XML.loadFile(dafExtFile)
+    // extract information
+    val tunablesSeq = (dafExtXml \ "element" \\ "element")
+    // build documentation
+    val sectionsSeq = tunablesSeq.flatMap { ele =>
+      val subtitle = ele \@ "name"
+      val documentation = (ele \ "annotation" \ "documentation").text
+      val sections =
+        if (documentation.nonEmpty && !documentation.trim.startsWith("Deprecated")) {
+          val s =
+            s"""
+            |#### $subtitle
+            |$documentation
+            |""".stripMargin
+          Some(s)
+        } else {
+          None
+        }
+      sections
+    }
+    val documentationPage =
+      s"""|---
+        |layout: page
+        |title: DFDL Tunables
+        |group: nav-right
+        |---
+        |<!--
+        |{% comment %}
+        |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.
+        |{% endcomment %}
+        |-->
+        |<!--
+        |{% comment %}
+        |This file is generated using ``sbt genTunablesDoc``. Update that task in Daffodil to update this file.
+        |{% endcomment %}
+        |-->
+        |
+        |Daffodil provides tunables as a way to change its behavior.
+        |Tunables are set by way of the ``tunables`` element in [config files](/configuration)
+        |or from the [cli](/cli) via the ``-T`` option.
+        |
+        |#### Config Example
+        | ``` xml
+        | <daf:dfdlConfig
+        |	xmlns:daf="urn:ogf:dfdl:2013:imp:daffodil.apache.org:2018:ext">
+        |    <daf:tunables>
+        |      <daf:suppressSchemaDefinitionWarnings>
+        |        encodingErrorPolicyError
+        |      </daf:suppressSchemaDefinitionWarnings>
+        |    </daf:tunables>
+        |</daf:dfdlConfig>
+        | ```
+        |
+        | The config file can then be passed into daffodil subcommands via the ``-c|--config`` options.
+        |
+        |#### CLI Example
+        | ``` bash
+        | daffodil parse -s schema.xsd -TsuppressSchemaDefinitionWarnings="encodingErrorPolicyError" data.bin
+        | ```
+        |
+        |
+        |### Definitions
+        |${sectionsSeq.mkString("\n")}
+        |""".stripMargin
+    IO.write(outputDocFile, s"$documentationPage")
+    println(s"Generated tunables documentation at: $outputDocFile")

Review Comment:
   Instead of using println, using `stream.log.info(...)`.
   
   Also, very nitpicky but the I think sbt is trying to standardize on lowercase messages, so `stream.log.info("generated tunables at ...")`. I try to follow that convention where possible.



##########
build.sbt:
##########
@@ -481,6 +489,99 @@ lazy val unidocSettings =
       (JavaUnidoc / unidoc / unidocAllSources).value.map(apiDocSourceFilter)
   )
 
+lazy val genTunablesDocSettings = Seq(
+  Compile / genTunablesDoc := {
+    val dafExtFile =
+      (propgen / Compile / resources).value.filter(_.getName == "dafext.xsd").head
+    val outputDocFile = (Compile / target).value / "tunables.md"
+    // parse xsd file
+    val dafExtXml = scala.xml.XML.loadFile(dafExtFile)
+    // extract information
+    val tunablesSeq = (dafExtXml \ "element" \\ "element")
+    // build documentation
+    val sectionsSeq = tunablesSeq.flatMap { ele =>
+      val subtitle = ele \@ "name"
+      val documentation = (ele \ "annotation" \ "documentation").text
+      val sections =
+        if (documentation.nonEmpty && !documentation.trim.startsWith("Deprecated")) {
+          val s =
+            s"""
+            |#### $subtitle
+            |$documentation
+            |""".stripMargin

Review Comment:
   Thoughts on extracting and adding `default: ...` after the documentation, since I don't think we always mention it in the action description.



##########
build.sbt:
##########
@@ -24,6 +24,7 @@ lazy val genProps = taskKey[Seq[File]]("Generate properties scala source")
 lazy val genSchemas = taskKey[Seq[File]]("Generate DFDL schemas")
 lazy val genCExamples = taskKey[Seq[File]]("Generate C example files")
 lazy val genVersion = taskKey[Seq[File]]("Generate VERSION file")
+lazy val genTunablesDoc = taskKey[Unit]("Generate tunables docs from dafext.xsd file")

Review Comment:
   The type of this probably wants to be `Seq[File]`, and the task will just return `Seq(outputDocFile)`. That way if we ever create a task that wants to do something with generated files it can do so.



-- 
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: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add SBT task to generate tunables doc [daffodil]

Posted by "olabusayoT (via GitHub)" <gi...@apache.org>.
olabusayoT merged PR #1340:
URL: https://github.com/apache/daffodil/pull/1340


-- 
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: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org