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

nim doc --project shows '@@/' instead of '../' for relative paths to submodules #14448

Closed
kaushalmodi opened this issue May 25, 2020 · 5 comments · Fixed by #14451
Closed

nim doc --project shows '@@/' instead of '../' for relative paths to submodules #14448

kaushalmodi opened this issue May 25, 2020 · 5 comments · Fixed by #14451

Comments

@kaushalmodi
Copy link
Contributor

kaushalmodi commented May 25, 2020

The nim doc --project shows @@/ instead of ../ in the relative paths to the nim project's submodules.

This issue can be reproduced using the latest nim devel as of today (2020-05-25) as well.. f96555b

I have set up a minimal example repo to show this issue.

Example

git clone https://gitlab.com/kaushalmodi/nimdoc-atsigns-in-submodule-paths nimdoc_issue
cd nimdoc_issue/prj_top
nim doc --project top.nim

Current Output

Open the generated htmldocs/top.html in a browser and you will see:

image

Expected Output

Probably one of the two:

  • [preferred] Get rid of the relative paths altogether because it is very unlikely that a project would use exactly same named modules from different paths. So there should be no need to distinguish the modules perfectly using the relative paths.
  • Show ../ instead of @@/

Additional Information

  • At work, I have a big wrapper nim module that imports 6 other nim libs from the whole project. I compile that 1 wrapper nim module to a .so to link with SystemVerilog code. I would like to publish the documentation for all the API exposed by that .so to the team. This is when I noticed the use of @@/ to uniquify the modules that has leaked into the documentation too.
  • I don't believe we need that module uniquification within the same project.
$ nim -v
Nim Compiler Version 1.3.5 [Linux: amd64]
Compiled at 2020-05-25
Copyright (c) 2006-2020 by Andreas Rumpf

git hash: f96555bd102d534342eac14063bd8d93ec08a6dc
active boot switches: -d:release

/cc @timotheecour @mratsim

@timotheecour
Copy link
Member

timotheecour commented May 25, 2020

Show ../ instead of @@/

that's trivial to change; PR welcome (good change even if we later avoid hierarchy)

[preferred] Get rid of the relative paths altogether

well that's what I wanted, to flatten hierarchy like we do for nimcache

it would've simplified a lot of things IMO, but see araq's answer:

#13150 (comment)

Huh? It was designed not to flatten it.
Oh I'm sure it's currently broken but I've regretted the flattening we do for nimcache so it would be nice if the docgen could simply replicate the hierarchy -- maybe introduce helper names for file name clashes.

however we need to handle clashes

... because it is very unlikely that a project would use exactly same named modules from different paths. So there should be no need to distinguish the modules perfectly using the relative paths.

no, we need to handle name clashes somehow; we can't just ignore this edge case which nim allows;

other aspects

note that --docroot helps here:

nim doc --project --outdir:htmldocs2 --docroot -r top.nim

tree htmldocs2
htmldocs2
├── liba.html
├── libb.html
├── libc.html
├── libd.html
├── nimdoc.out.css
└── top.html

docroot does not flatten (unlike what would appear from this example), but it shows path foo relative to shortest of:

  • relativepath wrt nimbleroot
  • relativepath wrt entries found in --path
    which is why it looks flat since your path includes
switch("path", ".." / ".." / ".." / ".." / "a" / "liba")
switch("path", ".." / ".." / ".." / ".." / "b1" / "b2" / "libb")
switch("path", ".." / "libc")
switch("path", "." / "libd")

we can add option: --docroot:@flat

I think this option would make a lot of sense to add, it'd basically force a flat hierarchy; since it's incompatible with any other choice for --docroot, it avoids any conflicting option.
Whether that'd be default or not depends on convincing araq (refs: #13150 (comment))
but we can still fake relative paths in what's shown in html (if needed).
I think that'd work great for monorepos (ie, with multiple nimble packages) such as nim (which holds compiler, stdlib, tools etc); you'd have 1 dir per nimble project, each with it's own theindex.html, and everything would be flat wrt to their nimble project eg:

compiler/theindex.html # the index just for compiler
compiler/ast.html # for compiler/ast.nim
compiler/itersgen.html # instead of compiler/plugins/itersgen.html
...
std/theindex.html
std/sytem.html
std/set.html # instead of lib/pure/collections/sets.html

name clashes do occur

stdlib is careful about avoiding those (lib/system/sets.nim is include and doesn't clash with lib/pure/collections/sets.nim, ditto with atomics and alloc) but it may not be desirable in other projects; or even in collections of files for which you want to generate docs;
for eg:

there are 46 names clashes within $nim/tests (eg: tests/misc/tcast.nim clash tests/destructor/tcast.nim); sure it's not a real nimble package, but it could make sense to create docs for those (eg to link to additional tests for a given module or concept); treating is dir as a separate package would be overkill, and wouldn't work well because of cross references (eg foo importing sub/bar)

resolving name clashes with flat hierarchy

we can use any reasonable mangling scheme, but it will "leak" in the hrefs so it shouldn't be ugly

that's a practical choice; it works well with --path semantics

external hrefs

another issue to consider is if an href is created for mypkg/foo.html, the external links to it are created; then later mypkg/other/foo.nim is added; this will either invalidate mypkg/foo.html or cause it to point to wrong file depending on mangling scheme used. We can handle all this well with docgen to guarantee all internal links are correct; but for external href's it's harder.

@kaushalmodi
Copy link
Contributor Author

@timotheecour Thanks for the quick fix!


From the flat hierarchy examples:

mypkg/foo/bar # => mypkg/[email protected]

Those links won't look too good.. they will look like https://example.com/%40%40/libc/libc.html#LibCType. But we discuss this in the PR.

@timotheecour
Copy link
Member

Those links won't look too good.. they will look like https://example.com/%40%40/libc/libc.html#LibCType. But we discuss this in the PR.

wait which PR? also, do you have a repro that shows the %40%40 ?

@kaushalmodi
Copy link
Contributor Author

wait which PR?

I meant your upcoming PR for

we can add option: --docroot:@Flat

:)

do you have a repro that shows the %40%40

That's how the links generate in theindex.html.. take the minimal example above, navigate to its TheIndex, and see any link, like ...c1/c2/c3/prj_top/htmldocs/%40%40/%40%40/%40%40/%40%40/a/liba/liba.html#LibAType.

@timotheecour
Copy link
Member

=> fixing it in #14454, please try and report back on PR :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants