Skip to content

Conversation

@JinShil
Copy link
Contributor

@JinShil JinShil commented Dec 11, 2018

/// This is module b
module b;

/// This is a function in module b
void bFunction() { }
/// This is module c
module c;

/// This is a function in module c
void cFunction() { }
/// This is module a
module a;

/// This is a public import comment
public import b;

/// This is a private import comment
private import c;

/// This is a function in module a
void aFunction() { }

image

@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @JinShil! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the annotated coverage diff directly on GitHub with CodeCov's browser extension
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close Bugzilla Severity Description
10665 enhancement The documentation produced by ddoc should clearly list all public imports of a module

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + dmd#9068"

@JinShil
Copy link
Contributor Author

JinShil commented Dec 11, 2018

It still needs a test, but I'd like some feedback first if this is what we want. I had a previous version that recursively added members from public imports, but I'm not sure if that's what we want.

@thewilsonator
Copy link
Contributor

I like it.

Couple of questions:
What does the default protection import produce? I'm guessing nothing because they're private?

What does no comment look like?

@JinShil
Copy link
Contributor Author

JinShil commented Dec 11, 2018

What does the default protection import produce? I'm guessing nothing because they're private?

Correct. If the import is private, nothing is emitted.

What does no comment look like?

In the example above, if /// This is a public import comment is removed from the public import b; statement then the resulting documentation is the same, but without the text "This is a public import".

@JinShil
Copy link
Contributor Author

JinShil commented Dec 15, 2018

This isn't ready yet. import core.stdc.string shows up as core; not fully qualified. Other things to test would be public import io = std.stdio;, etc.

@Geod24
Copy link
Member

Geod24 commented Dec 15, 2018

We definitely want it. Public imports are part of a module's interface, and should be documented as such.

@PetarKirov
Copy link
Member

PetarKirov commented Apr 7, 2019

If it weren't for DAutoTest, I'd say this is good to go, however it seems to segfault while building the docs for dmd:

[..]
/dev/shm/dtest/work/repo/dmd/generated/linux/release/64/dmd -o- -m64 -J../generated/linux/release/64 -J../res -c -w -Dd/dev/shm/dtest/work/repo/dlang.org -Idmd  -version=MARS -fPIC -J../generated/linux/release/64 -w -de -g -dip25 project.ddoc /dev/shm/dtest/work/repo/dlang.org/macros.ddoc /dev/shm/dtest/work/repo/dlang.org/html.ddoc /dev/shm/dtest/work/repo/dlang.org/dlang.org.ddoc /dev/shm/dtest/work/repo/dlang.org/.generated/2.085.1.ddoc /dev/shm/dtest/work/repo/dlang.org/std.ddoc /dev/shm/dtest/work/repo/dlang.org/std_navbar-prerelease.ddoc /dev/shm/dtest/work/repo/dlang.org/.generated/modlist-prerelease.ddoc /dev/shm/dtest/work/repo/dlang.org/nodatetime.ddoc -Df/dev/shm/dtest/work/repo/dlang.org/web/phobos-prerelease/dmd_backend_cdef.html dmd/backend/cdef.d
posix.mak:674: recipe for target '/dev/shm/dtest/work/repo/dlang.org/web/phobos-prerelease/dmd_backend_cdef.html' failed
make[2]: *** [/dev/shm/dtest/work/repo/dlang.org/web/phobos-prerelease/dmd_backend_cdef.html] Segmentation fault (core dumped)
make[2]: Leaving directory '/dev/shm/dtest/work/repo/dmd/src'
posix.mak:41: recipe for target 'html' failed
make[1]: *** [html] Error 2
make[1]: Leaving directory '/dev/shm/dtest/work/repo/dmd'
posix.mak:609: recipe for target 'dmd-prerelease' failed
[..]

@JinShil
Copy link
Contributor Author

JinShil commented Apr 7, 2019

This is not ready yet. Please wait until I add a final comment stating that this is ready.

@JinShil
Copy link
Contributor Author

JinShil commented May 12, 2019

@kinke LDC doesn't compile this function; see the Semaphore CI output.

override void visit(Import i)
{
    HdrGenState hgs;
    hgs.ddoc = true;
    emitProtection(buf, i);
    .toCBuffer(s, buf, &hgs); // dmd/doc.d(1270): Error: function dmd.doc.toDocBuffer.ToDocBuffer.visit cannot access frame of function dmd.doc.toDocBuffer
}

Please help. Thanks!

@kinke
Copy link
Contributor

kinke commented May 12, 2019

From a quick glance, your new override is the only one capturing something (s) from the function containing the extern(C++) Visitor class; I think nested extern(C++) classes cannot have a context. So changing s to i should work, I guess.

@thewilsonator
Copy link
Contributor

Is this good to go?

@JinShil
Copy link
Contributor Author

JinShil commented May 15, 2019

No, it's not ready yet. I'm sorry. I have one more issue to solve, and while the symptom is simple, the solution appears to be quite difficult. I'll keep picking away at it.

@JinShil
Copy link
Contributor Author

JinShil commented Jun 1, 2019

I think this is finally ready to go. The following code...

/// This is a public import
public import core.stdc.string;

/// This is a mutiple public import
public import core.stdc.stdarg, core.stdc.stdlib;

/// This is a public selective import
public import core.stdc.string : memcpy;

/// This is a public selective renamed import
public import core.stdc.string : copy = memcpy;

/// This is a public multiple selective import
public import core.stdc.string : memcpy, memcmp;

/// This is a public multiple selective renamed import
public import core.stdc.string : copy = memcpy, compare = memcmp;

/// This is a public renamed import
public import str = core.stdc.string;

... will produce the following output:

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants