walkDirRecFilter, update doc CI filter, compiler/index.nim for docs + various other fixes (#14501)

* update doc CI filter to include the files mostly likely to require doc rebuild
* remove code duplication in ./config/nimdoc.cfg; show link to compiler docs, various fixes
* walkDirRecFilter, factor nativeToUnixPath workaround
* glob for getRst2html
* docslocal: 40s to build all docs
* revert code dedup in github actions which did not work alas...
* fixups

.github/workflows/ci_docs.yml

@@ -1,18 +1,27 @@
 name: Nim Docs CI
 on:
   push:
-    # Run only on changes on these files
     paths:
-      - 'lib/**.nim'
+      - 'compiler/docgen.nim'
+      - 'compiler/renderverbatim.nim'
       - 'doc/**.rst'
       - 'doc/nimdoc.css'
+      - 'lib/**.nim'
+      - 'nimdoc/testproject/expected/testproject.html'
+      - 'tools/dochack/dochack.nim'
+      - 'tools/kochdocs.nim'
       - '.github/workflows/ci_docs.yml'
-
   pull_request:
     # Run only on changes on these files
     paths:
-      - 'lib/**.nim'
+      - 'compiler/docgen.nim'
+      - 'compiler/renderverbatim.nim'
       - 'doc/**.rst'
+      - 'doc/nimdoc.css'
+      - 'lib/**.nim'
+      - 'nimdoc/testproject/expected/testproject.html'
+      - 'tools/dochack/dochack.nim'
+      - 'tools/kochdocs.nim'
       - '.github/workflows/ci_docs.yml'
 
 jobs:
@@ -115,13 +124,6 @@ jobs:
         shell: bash
         run: ./koch doc --git.commit:devel
 
-      - name: 'Prepare documentation for deployment'
-        if: |
-          github.event_name == 'push' && github.ref == 'refs/heads/devel' &&
-          matrix.target == 'linux'
-        shell: bash
-        run: cp -f doc/html/{overview,index}.html
-
       - name: 'Publish documentation to Github Pages'
         if: |
           github.event_name == 'push' && github.ref == 'refs/heads/devel' &&

compiler/docgen.nim

@@ -19,6 +19,8 @@ import
   typesrenderer, astalgo, lineinfos, intsets,
   pathutils, trees, tables, nimpaths, renderverbatim
 
+from std/private/globs import nativeToUnixPath
+
 const
   exportSection = skField
   docCmdSkip = "skip"
@@ -53,12 +55,6 @@ type
 
   PDoc* = ref TDocumentor ## Alias to type less.
 
-proc nativeToUnix(path: string): string =
-  doAssert not path.isAbsolute # absolute files need more care for the drive
-  when DirSep == '\\':
-    result = replace(path, '\\', '/')
-  else: result = path
-
 proc docOutDir(conf: ConfigRef, subdir: RelativeDir = RelativeDir""): AbsoluteDir =
   if not conf.outDir.isEmpty: conf.outDir else: conf.projectPath / subdir
 
@@ -97,7 +93,7 @@ proc presentationPath*(conf: ConfigRef, file: AbsoluteFile, isTitle = false): Re
   if isAbsolute(result.string):
     result = file.string.splitPath()[1].RelativeFile
   if isTitle:
-    result = result.string.nativeToUnix.RelativeFile
+    result = result.string.nativeToUnixPath.RelativeFile
   else:
     result = result.string.replace("..", dotdotMangle).RelativeFile
   doAssert not result.isEmpty
@@ -1244,20 +1240,23 @@ proc genOutFile(d: PDoc): Rope =
   # Extract the title. Non API modules generate an entry in the index table.
   if d.meta[metaTitle].len != 0:
     title = d.meta[metaTitle]
-    let external = presentationPath(d.conf, AbsoluteFile d.filename).changeFileExt(HtmlExt).string.nativeToUnix
+    let external = presentationPath(d.conf, AbsoluteFile d.filename).changeFileExt(HtmlExt).string.nativeToUnixPath
     setIndexTerm(d[], external, "", title)
   else:
     # Modules get an automatic title for the HTML, but no entry in the index.
     # better than `extractFilename(changeFileExt(d.filename, ""))` as it disambiguates dups
     title = $presentationPath(d.conf, AbsoluteFile d.filename, isTitle = true).changeFileExt("")
 
-  let bodyname = if d.hasToc and not d.isPureRst: "doc.body_toc_group"
+  var groupsection = getConfigVar(d.conf, "doc.body_toc_groupsection")
+  let bodyname = if d.hasToc and not d.isPureRst:
+                   groupsection.setLen 0
+                   "doc.body_toc_group"
                  elif d.hasToc: "doc.body_toc"
                  else: "doc.body_no_toc"
   content = ropeFormatNamedVars(d.conf, getConfigVar(d.conf, bodyname), ["title",
-      "tableofcontents", "moduledesc", "date", "time", "content", "deprecationMsg", "theindexhref"],
+      "tableofcontents", "moduledesc", "date", "time", "content", "deprecationMsg", "theindexhref", "body_toc_groupsection"],
       [title.rope, toc, d.modDesc, rope(getDateStr()),
-      rope(getClockStr()), code, d.modDeprecationMsg, relLink(d.conf.outDir, d.destFile, theindexFname.RelativeFile)])
+      rope(getClockStr()), code, d.modDeprecationMsg, relLink(d.conf.outDir, d.destFile, theindexFname.RelativeFile), groupsection.rope])
   if optCompileOnly notin d.conf.globalOptions:
     # XXX what is this hack doing here? 'optCompileOnly' means raw output!?
     code = ropeFormatNamedVars(d.conf, getConfigVar(d.conf, "doc.file"), [

compiler/index.nim

@@ -0,0 +1,18 @@
+##[
+This module only exists to generate docs for the compiler.
+
+## links
+* [main docs](../lib.html)
+* [compiler user guide](../nimc.html)
+* [Internals of the Nim Compiler](../intern.html)
+]##
+
+#[
+note: this is named `index` so that navigating to https://nim-lang.github.io/Nim/compiler/
+will work.
+
+xxx this should also import other modules, not transitively imported by `compiler/nim.nim`,
+eg `evalffi`, otherwise these aren't shown. A glob could be used at CT.
+]#
+
+import nim

config/nimdoc.cfg

@@ -81,42 +81,14 @@ $content
 </ul>
 """
 
-doc.body_toc = """
+doc.body_toc_groupsection = """
-<div class="row">
+  <div class="search-groupby">
-  <div class="three columns">
+    Group by:
-  <div class="theme-switch-wrapper">
+    <select onchange="groupBy(this.value)">
-    <label class="theme-switch" for="checkbox">
+      <option value="section">Section</option>
-      <input type="checkbox" id="checkbox" />
+      <option value="type">Type</option>
-      <div class="slider round"></div>
+    </select>
-    </label>
-   &nbsp;&nbsp;&nbsp; <em>Dark Mode</em>
   </div>
-  <div id="global-links">
-    <ul class="simple-boot">
-      <li>
-        <a href="manual.html">Manual</a>
-      </li>
-      <li>
-        <a href="lib.html">Standard library</a>
-      </li>
-      <li>
-        <a href="$theindexhref">Index</a>
-      </li>
-    </ul>
-  </div>
-  <div id="searchInputDiv">
-    Search: <input type="text" id="searchInput"
-      onkeyup="search()" />
-  </div>
-  $tableofcontents
-  </div>
-  <div class="nine columns" id="content">
-  <div id="tocRoot"></div>
-  $deprecationMsg
-  <p class="module-desc">$moduledesc</p>
-  $content
-  </div>
-</div>
 """
 
 @if boot:
@@ -145,19 +117,16 @@ doc.body_toc_group = """
       <li>
         <a href="$theindexhref">Index</a>
       </li>
+      <li>
+        <a href="compiler/$theindexhref">Compiler docs</a>
+      </li>
     </ul>
   </div>
   <div id="searchInputDiv">
     Search: <input type="text" id="searchInput"
       onkeyup="search()" />
   </div>
-  <div class="search-groupby">
+  $body_toc_groupsection
-    Group by:
-    <select onchange="groupBy(this.value)">
-      <option value="section">Section</option>
-      <option value="type">Type</option>
-    </select>
-  </div>
   $tableofcontents
   </div>
   <div class="nine columns" id="content">
@@ -211,6 +180,8 @@ doc.body_toc_group = """
 """
 @end
 
+doc.body_toc %= "${doc.body_toc_group}" # should only be used for boot
+
 doc.body_no_toc = """
 $moduledesc
 $content

doc/idetools.rst

@@ -13,6 +13,8 @@
   "yes, I'm the creator" -- Araq, 2013-07-26 19:28:32.
   </p></blockquote>
 
+Note: this is mostly outdated, see instead `nimsuggest <nimsuggest.html>`_
+
 Nim differs from many other compilers in that it is really fast,
 and being so fast makes it suited to provide external queries for
 text editors about the source code being written. Through the

koch.nim

@@ -651,6 +651,13 @@ when isMainModule:
       of "latest": latest = true
       of "stable": latest = false
       of "nim": nimExe = op.val.absolutePath # absolute so still works with changeDir
+      of "docslocal":
+        # undocumented for now, allows to rebuild local docs in < 40s as follows:
+        # `./koch --nim:$nimb --docslocal:htmldocs2 --doccmd:skip --warnings:off --hints:off`
+        # whereas `./koch docs` takes 190s; useful for development.
+        doAssert op.val.len > 0
+        buildDocsDir(op.cmdLineRest, op.val)
+        break
       else: showHelp()
     of cmdArgument:
       case normalize(op.key)

lib/std/private/globs.nim

@@ -0,0 +1,54 @@
+##[
+unstable API, internal use only for now.
+this can eventually be moved to std/os and `walkDirRec` can be implemented in terms of this
+to avoid duplication
+]##
+
+import std/[os,strutils]
+
+type
+  PathEntry* = object
+    kind*: PathComponent
+    path*: string
+
+iterator walkDirRecFilter*(dir: string, follow: proc(entry: PathEntry): bool = nil,
+    relative = false, checkDir = true): PathEntry {.tags: [ReadDirEffect].} =
+  ## Improved `os.walkDirRec`.
+  #[
+  note: a yieldFilter isn't needed because caller can filter at call site, without
+  loss of generality, unlike `follow`.
+
+  Future work:
+  * need to document
+  * add a `sort` option, which can be implemented efficiently only here, not at call site.
+  * provide a way to do error reporting, which is tricky because iteration cannot be resumed
+  ]#
+  var stack = @["."]
+  var checkDir = checkDir
+  var entry: PathEntry
+  while stack.len > 0:
+    let d = stack.pop()
+    for k, p in walkDir(dir / d, relative = true, checkDir = checkDir):
+      let rel = d / p
+      entry.kind = k
+      if relative: entry.path = rel
+      else: entry.path = dir / rel
+      if k in {pcDir, pcLinkToDir}:
+        if follow == nil or follow(entry): stack.add rel
+      yield entry
+    checkDir = false
+      # We only check top-level dir, otherwise if a subdir is invalid (eg. wrong
+      # permissions), it'll abort iteration and there would be no way to
+      # continue iteration.
+
+proc nativeToUnixPath*(path: string): string =
+  # pending https://github.com/nim-lang/Nim/pull/13265
+  doAssert not path.isAbsolute # not implemented here; absolute files need more care for the drive
+  when DirSep == '\\':
+    result = replace(path, '\\', '/')
+  else: result = path
+
+when isMainModule:
+  import sugar
+  for a in walkDirRecFilter(".", follow = a=>a.path.lastPathPart notin ["nimcache", ".git", ".csources", "bin"]):
+    echo a

tests/misc/trunner.nim

@@ -11,6 +11,7 @@ import std/[strformat,os,osproc,unittest]
 from std/sequtils import toSeq,mapIt
 from std/algorithm import sorted
 import stdtest/[specialpaths, unittest_light]
+from std/private/globs import nativeToUnixPath
 
 import "$lib/../compiler/nimpaths"
 
@@ -90,11 +91,7 @@ else: # don't run twice the same test
       removeDir(htmldocsDir)
       let (outp, exitCode) = execCmdEx(cmd)
       check exitCode == 0
-      proc nativeToUnixPathWorkaround(a: string): string =
+      let ret = toSeq(walkDirRec(htmldocsDir, relative=true)).mapIt(it.nativeToUnixPath).sorted.join("\n")
-        # xxx pending https://github.com/nim-lang/Nim/pull/13265 `nativeToUnixPath`
-        a.replace(DirSep, '/')
-
-      let ret = toSeq(walkDirRec(htmldocsDir, relative=true)).mapIt(it.nativeToUnixPathWorkaround).sorted.join("\n")
       let context = $(i, ret, cmd)
       var expected = ""
       case i

tools/kochdocs.nim

@@ -1,6 +1,7 @@
 ## Part of 'koch' responsible for the documentation generation.
 
 import os, strutils, osproc, sets, pathnorm
+from std/private/globs import nativeToUnixPath, walkDirRecFilter, PathEntry
 import "../compiler/nimpaths"
 
 const
@@ -85,6 +86,16 @@ proc nimCompileFold*(desc, input: string, outputDir = "bin", mode = "c", options
   let cmd = findNim().quoteShell() & " " & mode & " -o:" & output & " " & options & " " & input
   execFold(desc, cmd)
 
+proc getRst2html(): seq[string] =
+  for a in walkDirRecFilter("doc"):
+    let path = a.path
+    if a.kind == pcFile and path.splitFile.ext == ".rst" and path.lastPathPart notin
+        ["docs.rst", "nimfix.rst"]:
+          # maybe we should still show nimfix, could help reviving it
+          # `docs` is redundant with `overview`, might as well remove that file?
+      result.add path
+  doAssert "doc/manual/var_t_return.rst".unixToNativePath in result # sanity check
+
 const
   pdf = """
 doc/manual.rst
@@ -95,39 +106,6 @@ doc/tut3.rst
 doc/nimc.rst
 doc/niminst.rst
 doc/gc.rst
-""".splitWhitespace()
-
-  rst2html = """
-doc/intern.rst
-doc/apis.rst
-doc/lib.rst
-doc/manual.rst
-doc/manual_experimental.rst
-doc/destructors.rst
-doc/tut1.rst
-doc/tut2.rst
-doc/tut3.rst
-doc/nimc.rst
-doc/hcr.rst
-doc/drnim.rst
-doc/overview.rst
-doc/filters.rst
-doc/tools.rst
-doc/niminst.rst
-doc/nimgrep.rst
-doc/gc.rst
-doc/estp.rst
-doc/idetools.rst
-doc/docgen.rst
-doc/koch.rst
-doc/backends.rst
-doc/nimsuggest.rst
-doc/nep1.rst
-doc/nims.rst
-doc/contributing.rst
-doc/codeowners.rst
-doc/packaging.rst
-doc/manual/var_t_return.rst
 """.splitWhitespace()
 
   doc0 = """
@@ -185,7 +163,7 @@ proc getDocList(): seq[string] =
   for a in withoutIndex: docIgnore.incl a
   for a in ignoredModules: docIgnore.incl a
 
-  # don't ignore these even though in lib/system
+  # don't ignore these even though in lib/system (not include files)
   const goodSystem = """
 lib/system/io.nim
 lib/system/nimscript.nim
@@ -194,14 +172,14 @@ lib/system/iterators.nim
 lib/system/dollars.nim
 lib/system/widestrs.nim
 """.splitWhitespace()
-
+  
-  for a in walkDirRec("lib"):
+  proc follow(a: PathEntry): bool =
-    if a.splitFile.ext != ".nim" or
+    a.path.lastPathPart notin ["nimcache", "htmldocs", "includes", "deprecated", "genode"]
-       a.isRelativeTo("lib/pure/includes") or
+  for entry in walkDirRecFilter("lib", follow = follow):
-       a.isRelativeTo("lib/genode") or
+    let a = entry.path
-       a.isRelativeTo("lib/deprecated") or
+    if entry.kind != pcFile or a.splitFile.ext != ".nim" or
-       (a.isRelativeTo("lib/system") and a.replace('\\', '/') notin goodSystem) or
+       (a.isRelativeTo("lib/system") and a.nativeToUnixPath notin goodSystem) or
-       a.replace('\\', '/') in docIgnore:
+       a.nativeToUnixPath in docIgnore:
          continue
     result.add a
   result.add normalizePath("nimsuggest/sexp.nim")
@@ -231,16 +209,28 @@ proc buildDocSamples(nimArgs, destPath: string) =
     [nimArgs, destPath / "docgen_sample.html", "doc" / "docgen_sample.nim"])
 
 proc buildDocPackages(nimArgs, destPath: string) =
-  # compiler docs, and later, other packages (perhaps tools, testament etc)
+  # compiler docs; later, other packages (perhaps tools, testament etc)
   let nim = findNim().quoteShell()
-  let extra = "-u:boot"
     # to avoid broken links to manual from compiler dir, but a multi-package
     # structure could be supported later
-  exec("$1 doc --project --outdir:$2/compiler $3 --git.url:$4 $5 compiler/nim.nim" %
+
-    [nim, destPath, nimArgs, gitUrl, extra])
+  proc docProject(outdir, options, mainproj: string) =
+    exec("$nim doc --project --outdir:$outdir $nimArgs --git.url:$gitUrl $options $mainproj" % [
+      "nim", nim,
+      "outdir", outdir,
+      "nimArgs", nimArgs,
+      "gitUrl", gitUrl,
+      "options", options,
+      "mainproj", mainproj,
+      ])
+  let extra = "-u:boot"
+  # xxx keep in sync with what's in $nim_prs_D/config/nimdoc.cfg, or, rather,
+  # start using nims instead of nimdoc.cfg
+  docProject(destPath/"compiler", extra, "compiler/index.nim")
 
 proc buildDoc(nimArgs, destPath: string) =
   # call nim for the documentation:
+  let rst2html = getRst2html()
   var
     commands = newSeq[string](rst2html.len + len(doc0) + len(doc) + withoutIndex.len)
     i = 0
@@ -305,18 +295,19 @@ proc buildJS(): string =
   let nim = findNim()
   exec(nim.quoteShell() & " js -d:release --out:$1 tools/nimblepkglist.nim" %
       [webUploadOutput / "nimblepkglist.js"])
+      # xxx deadcode? and why is it only for webUploadOutput, not for local docs?
   result = getDocHacksJs(nimr = getCurrentDir(), nim)
 
-proc buildDocs*(args: string) =
+proc buildDocsDir*(args: string, dir: string) =
+  let args = nimArgs & " " & args
   let docHackJsSource = buildJS()
-  template fn(args, dir) =
+  createDir(dir)
-    let dir2 = dir
+  buildDocSamples(args, dir)
-    let args2 = args
+  buildDoc(args, dir) # bottleneck
-    createDir(dir2)
+  copyFile(dir / "overview.html", dir / "index.html")
-    buildDocSamples(args2, dir2)
+  buildDocPackages(args, dir)
-    buildDoc(args2, dir2)
+  copyFile(docHackJsSource, dir / docHackJsSource.lastPathPart)
-    buildDocPackages(args2, dir2)
-    copyFile(docHackJsSource, dir2 / docHackJsSource.lastPathPart)
 
-  fn(nimArgs & " " & args, webUploadOutput / NimVersion)
+proc buildDocs*(args: string) =
-  fn(nimArgs, docHtmlOutput) # no `args` to avoid offline docs containing the 'gaCode'!
+  buildDocsDir(args, webUploadOutput / NimVersion)
+  buildDocsDir("", docHtmlOutput) # no `args` to avoid offline docs containing the 'gaCode'!

tools/nimblepkglist.nim

@@ -1,3 +1,6 @@
+#[
+deadcode?
+]#
 import base64, strutils, json, htmlgen, dom, algorithm
 
 type