Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix #13150 nim doc --project now works reliably #13223

Merged
merged 12 commits into from
Feb 6, 2020
20 changes: 20 additions & 0 deletions compiler/ast.nim
Original file line number Diff line number Diff line change
Expand Up @@ -1026,6 +1026,26 @@ const
defaultAlignment = -1
defaultOffset = -1


proc getnimblePkg*(a: PSym): PSym =
result = a
while result != nil:
case result.kind
of skModule:
result = result.owner
assert result.kind == skPackage
of skPackage:
if result.owner == nil:
break
else:
result = result.owner
else:
assert false, $result.kind

proc getnimblePkgId*(a: PSym): int =
let b = a.getnimblePkg
result = if b == nil: -1 else: b.id

var ggDebug* {.deprecated.}: bool ## convenience switch for trying out things
#var
# gMainPackageId*: int
Expand Down
2 changes: 2 additions & 0 deletions compiler/commands.nim
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,8 @@ proc processSwitch*(switch, arg: string, pass: TCmdLinePass, info: TLineInfo;
of "docseesrcurl":
expectArg(conf, switch, arg, pass, info)
conf.docSeeSrcUrl = arg
of "docroot":
conf.docRoot = if arg.len == 0: "@default" else: arg
of "mainmodule", "m":
discard "allow for backwards compatibility, but don't do anything"
of "define", "d":
Expand Down
69 changes: 57 additions & 12 deletions compiler/docgen.nim
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,47 @@ 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 presentationPath*(conf: ConfigRef, file: AbsoluteFile, isTitle = false): RelativeFile =
## returns a relative file that will be appended to outDir
let file2 = $file
template bail() =
result = relativeTo(file, conf.projectPath)
proc nimbleDir(): AbsoluteDir =
getNimbleFile(conf, file2).parentDir.AbsoluteDir
case conf.docRoot:
of "@default": # using `@` instead of `$` to avoid shell quoting complications
result = getRelativePathFromConfigPath(conf, file)
let dir = nimbleDir()
if not dir.isEmpty:
let result2 = relativeTo(file, dir)
if not result2.isEmpty and (result.isEmpty or result2.string.len < result.string.len):
result = result2
if result.isEmpty: bail()
of "@pkg":
let dir = nimbleDir()
if dir.isEmpty: bail()
else: result = relativeTo(file, dir)
of "@path":
result = getRelativePathFromConfigPath(conf, file)
if result.isEmpty: bail()
elif conf.docRoot.len > 0:
doAssert conf.docRoot.isAbsolute, conf.docRoot # or globalError
doAssert conf.docRoot.existsDir, conf.docRoot
result = relativeTo(file, conf.docRoot.AbsoluteDir)
else:
bail()
if isTitle:
result = result.string.nativeToUnix.RelativeFile
else:
result = result.string.replace("..", "@@").RelativeFile ## refs #13223
doAssert not result.isEmpty

proc whichType(d: PDoc; n: PNode): PSym =
if n.kind == nkSym:
if d.types.strTableContains(n.sym):
Expand Down Expand Up @@ -175,7 +216,7 @@ proc newDocumentor*(filename: AbsoluteFile; cache: IdentCache; conf: ConfigRef,
if execShellCmd(c2) != status:
rawMessage(conf, errGenerated, "executing of external program failed: " & c2)
result.emitted = initIntSet()
result.destFile = getOutFile2(conf, relativeTo(filename, conf.projectPath),
result.destFile = getOutFile2(conf, presentationPath(conf, filename),
outExt, htmldocsDir, false)
result.thisDir = result.destFile.splitFile.dir

Expand Down Expand Up @@ -295,14 +336,13 @@ proc getPlainDocstring(n: PNode): string =
if result.len > 0: return

proc belongsToPackage(conf: ConfigRef; module: PSym): bool =
result = module.kind == skModule and module.owner != nil and
module.owner.id == conf.mainPackageId
result = module.kind == skModule and module.getnimblePkgId == conf.mainPackageId

proc externalDep(d: PDoc; module: PSym): string =
if optWholeProject in d.conf.globalOptions:
if optWholeProject in d.conf.globalOptions or d.conf.docRoot.len > 0:
let full = AbsoluteFile toFullPath(d.conf, FileIndex module.position)
let tmp = getOutFile2(d.conf, full.relativeTo(d.conf.projectPath), HtmlExt,
htmldocsDir, sfMainModule notin module.flags)
let tmp = getOutFile2(d.conf, presentationPath(d.conf, full), HtmlExt,
htmldocsDir, sfMainModule notin module.flags)
result = relativeTo(tmp, d.thisDir, '/').string
else:
result = extractFilename toFullPath(d.conf, FileIndex module.position)
Expand Down Expand Up @@ -1031,11 +1071,12 @@ 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 = AbsoluteFile(d.filename).relativeTo(d.conf.projectPath, '/').changeFileExt(HtmlExt).string
let external = presentationPath(d.conf, AbsoluteFile d.filename).changeFileExt(HtmlExt).string.nativeToUnix
setIndexTerm(d[], external, "", title)
else:
# Modules get an automatic title for the HTML, but no entry in the index.
title = extractFilename(changeFileExt(d.filename, ""))
# 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"
elif d.hasToc: "doc.body_toc"
Expand All @@ -1060,10 +1101,14 @@ proc generateIndex*(d: PDoc) =
let dir = if not d.conf.outDir.isEmpty: d.conf.outDir
else: d.conf.projectPath / htmldocsDir
createDir(dir)
let dest = dir / changeFileExt(relativeTo(AbsoluteFile d.filename,
d.conf.projectPath), IndexExt)
let dest = dir / changeFileExt(presentationPath(d.conf, AbsoluteFile d.filename), IndexExt)
writeIndexFile(d[], dest.string)

proc updateOutfile(d: PDoc, outfile: AbsoluteFile) =
if d.module == nil or sfMainModule in d.module.flags: # nil for eg for commandRst2Html
if d.conf.outFile.isEmpty and not d.conf.outDir.isEmpty:
d.conf.outFile = outfile.relativeTo(d.conf.outDir)

proc writeOutput*(d: PDoc, useWarning = false) =
runAllExamples(d)
var content = genOutFile(d)
Expand All @@ -1073,7 +1118,7 @@ proc writeOutput*(d: PDoc, useWarning = false) =
template outfile: untyped = d.destFile
#let outfile = getOutFile2(d.conf, shortenDir(d.conf, filename), outExt, htmldocsDir)
createDir(outfile.splitFile.dir)
d.conf.outFile = outfile.extractFilename.RelativeFile
updateOutfile(d, outfile)
if not writeRope(content, outfile):
rawMessage(d.conf, if useWarning: warnCannotOpenFile else: errCannotOpenFile,
outfile.string)
Expand All @@ -1100,7 +1145,7 @@ proc writeOutputJson*(d: PDoc, useWarning = false) =
if open(f, d.destFile.string, fmWrite):
write(f, $content)
close(f)
d.conf.outFile = d.destFile.extractFilename.RelativeFile
updateOutfile(d, d.destFile)
else:
localError(d.conf, newLineInfo(d.conf, AbsoluteFile d.filename, -1, -1),
warnUser, "unable to open file \"" & d.destFile.string &
Expand Down
4 changes: 2 additions & 2 deletions compiler/docgen2.nim
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ type
config: ConfigRef
PGen = ref TGen

template shouldProcess(g): bool =
(g.module.owner.id == g.doc.conf.mainPackageId and optWholeProject in g.doc.conf.globalOptions) or
proc shouldProcess(g: PGen): bool =
(optWholeProject in g.doc.conf.globalOptions and g.module.getnimblePkgId == g.doc.conf.mainPackageId) or
sfMainModule in g.module.flags or g.config.projectMainIdx == g.module.info.fileIndex

template closeImpl(body: untyped) {.dirty.} =
Expand Down
3 changes: 3 additions & 0 deletions compiler/main.nim
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,9 @@ proc mainCommand*(graph: ModuleGraph) =
for s in definedSymbolNames(conf.symbols): definedSymbols.elems.add(%s)

var libpaths = newJArray()
var lazyPaths = newJArray()
for dir in conf.searchPaths: libpaths.elems.add(%dir.string)
for dir in conf.lazyPaths: lazyPaths.elems.add(%dir.string)

var hints = newJObject() # consider factoring with `listHints`
for a in hintMin..hintMax:
Expand All @@ -311,6 +313,7 @@ proc mainCommand*(graph: ModuleGraph) =
(key: "project_path", val: %conf.projectFull.string),
(key: "defined_symbols", val: definedSymbols),
(key: "lib_paths", val: %libpaths),
(key: "lazyPaths", val: %lazyPaths),
(key: "outdir", val: %conf.outDir.string),
(key: "out", val: %conf.outFile.string),
(key: "nimcache", val: %getNimcacheDir(conf).string),
Expand Down
3 changes: 2 additions & 1 deletion compiler/modules.nim
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ proc partialInitModule(result: PSym; graph: ModuleGraph; fileIdx: FileIndex; fil
# but starting with version 0.20 we now produce a fake Nimble package instead
# to resolve the conflicts:
let pck3 = fakePackageName(graph.config, filename)
packSym = newSym(skPackage, getIdent(graph.cache, pck3), nil, result.info)
# this makes the new `packSym`'s owner be the original `packSym`
packSym = newSym(skPackage, getIdent(graph.cache, pck3), packSym, result.info)
initStrTable(packSym.tab)
graph.packageSyms.strTableAdd(packSym)

Expand Down
19 changes: 19 additions & 0 deletions compiler/options.nim
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,7 @@ type
implicitIncludes*: seq[string] # modules that are to be implicitly included
docSeeSrcUrl*: string # if empty, no seeSrc will be generated. \
# The string uses the formatting variables `path` and `line`.
docRoot*: string ## see nim --fullhelp for --docRoot

# the used compiler
cIncludes*: seq[AbsoluteDir] # directories to search for included files
Expand Down Expand Up @@ -656,6 +657,24 @@ template patchModule(conf: ConfigRef) {.dirty.} =
let ov = conf.moduleOverrides[key]
if ov.len > 0: result = AbsoluteFile(ov)

when (NimMajor, NimMinor) < (1, 1) or not declared(isRelativeTo):
proc isRelativeTo(path, base: string): bool =
# pending #13212 use os.isRelativeTo
let path = path.normalizedPath
let base = base.normalizedPath
let ret = relativePath(path, base)
result = path.len > 0 and not ret.startsWith ".."

proc getRelativePathFromConfigPath*(conf: ConfigRef; f: AbsoluteFile): RelativeFile =
let f = $f
template search(paths) =
for it in paths:
let it = $it
if f.isRelativeTo(it):
return relativePath(f, it).RelativeFile
search(conf.searchPaths)
search(conf.lazyPaths)

proc findFile*(conf: ConfigRef; f: string; suppressStdlib = false): AbsoluteFile =
if f.isAbsolute:
result = if f.existsFile: AbsoluteFile(f) else: AbsoluteFile""
Expand Down
13 changes: 8 additions & 5 deletions compiler/packagehandling.nim
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,8 @@ iterator myParentDirs(p: string): string =
if current.len == 0: break
yield current

proc resetPackageCache*(conf: ConfigRef) =
conf.packageCache = newPackageCache()

proc getPackageName*(conf: ConfigRef; path: string): string =
proc getNimbleFile*(conf: ConfigRef; path: string): string =
## returns absolute path to nimble file, eg: /pathto/cligen.nimble
var parents = 0
block packageSearch:
for d in myParentDirs(path):
Expand All @@ -27,7 +25,7 @@ proc getPackageName*(conf: ConfigRef; path: string): string =
return conf.packageCache[d]
inc parents
for file in walkFiles(d / "*.nimble"):
result = file.splitFile.name
result = file
break packageSearch
# we also store if we didn't find anything:
when not defined(nimNoNilSeqs):
Expand All @@ -38,6 +36,11 @@ proc getPackageName*(conf: ConfigRef; path: string): string =
dec parents
if parents <= 0: break

proc getPackageName*(conf: ConfigRef; path: string): string =
## returns nimble package name, eg: `cligen`
let path = getNimbleFile(conf, path)
result = path.splitFile.name

proc fakePackageName*(conf: ConfigRef; path: AbsoluteFile): string =
# Convert `path` so that 2 modules with same name
# in different directory get different name and they can be
Expand Down
7 changes: 5 additions & 2 deletions compiler/pathutils.nim
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ when true:

proc `/`*(base: AbsoluteDir; f: RelativeFile): AbsoluteFile =
let base = postProcessBase(base)
assert(not isAbsolute(f.string))
assert(not isAbsolute(f.string), f.string)
result = AbsoluteFile newStringOfCap(base.string.len + f.string.len)
var state = 0
addNormalizePath(base.string, result.string, state)
Expand All @@ -83,7 +83,10 @@ when true:

proc relativeTo*(fullPath: AbsoluteFile, baseFilename: AbsoluteDir;
sep = DirSep): RelativeFile =
RelativeFile(relativePath(fullPath.string, baseFilename.string, sep))
# this currently fails for `tests/compilerapi/tcompilerapi.nim`
# it's needed otherwise would returns an absolute path
# assert not baseFilename.isEmpty, $fullPath
result = RelativeFile(relativePath(fullPath.string, baseFilename.string, sep))

proc toAbsolute*(file: string; base: AbsoluteDir): AbsoluteFile =
if isAbsolute(file): result = AbsoluteFile(file)
Expand Down
7 changes: 7 additions & 0 deletions doc/advopt.txt
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,13 @@ Advanced options:
--clib:LIBNAME link an additional C library
(you should omit platform-specific extensions)
--project document the whole project (doc2)
--docRoot:path nim doc --docRoot:/foo --project --outdir:docs /foo/sub/main.nim
generates: docs/sub/main.html
if path == @pkg, will use nimble file enclosing dir
if path == @path, will use first matching dir in --path
if path == @default (the default and most useful), will use
best match among @pkg,@path.
if these are nonexistant, will use project path
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this? Can't we be smart about the project path and do without this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

short answer: I've now added a new smart default for --docRoot:
--docRoot with no arg now implies --docRoot:@default which picks the best match among @path, @pkg (and if these have no match, defaults to project path)

most users should simply use --docRoot with no arg.

long answer

before introducing @default, the way I wrote it accomodated all scenarios, which all happen in practice:

  • user has a nimble package and no --path => use @pkg; eg: compiler nimble package (fallsback to project path if no nimble file found)
  • user has (1 or more) --path => use @path, eg: stdlib which has --path:$nim/lib/pure, --path:$nim/lib/pure/concurrency etc (fallsback to project path if no enclosing --path found)
  • user has no nimble package and no path => use provided absolute dir

In all likelihood, @default will be more common for user docs, as I noted in the docs

now that I have @default, I'd be ok to remove @pkg and @path (but keep option for user provided absolute dir) but maybe it's best to keep these as option

--docSeeSrcUrl:url activate 'see source' for doc and doc2 commands
(see doc.item.seesrc in config/nimdoc.cfg)
--docInternal also generate documentation for non-exported symbols
Expand Down
4 changes: 2 additions & 2 deletions nimdoc/testproject/expected/subdir/subdir_b/utils.html
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
<link href='https://fonts.googleapis.com/css?family=Source+Code+Pro:400,500,600' rel='stylesheet' type='text/css'/>

<!-- CSS -->
<title>utils</title>
<title>subdir/subdir_b/utils</title>
<link rel="stylesheet" type="text/css" href="../../nimdoc.out.css">

<script type="text/javascript" src="dochack.js"></script>
Expand Down Expand Up @@ -71,7 +71,7 @@
<body onload="main()">
<div class="document" id="documentId">
<div class="container">
<h1 class="title">utils</h1>
<h1 class="title">subdir/subdir_b/utils</h1>
<div class="row">
<div class="three columns">
<div class="theme-switch-wrapper">
Expand Down
8 changes: 5 additions & 3 deletions tools/kochdocs.nim
Original file line number Diff line number Diff line change
Expand Up @@ -319,9 +319,11 @@ proc buildDoc(nimArgs, destPath: string) =
destPath / changeFileExt(splitFile(d).name, "html"), d]
i.inc
for d in items(doc):
commands[i] = nim & " doc $# --git.url:$# -o:$# --index:on $#" %
[nimArgs, gitUrl,
destPath / changeFileExt(splitFile(d).name, "html"), d]
var nimArgs2 = nimArgs
if d.isRelativeTo("compiler"):
nimArgs2.add " --docroot"
commands[i] = nim & " doc $# --git.url:$# --outdir:$# --index:on $#" %
[nimArgs2, gitUrl, destPath, d]
i.inc
for d in items(withoutIndex):
commands[i] = nim & " doc2 $# --git.url:$# -o:$# $#" %
Expand Down