dartdoc icon indicating copy to clipboard operation
dartdoc copied to clipboard

Tracking issue for private documentation (#2154)

Open Levi-Lesches opened this issue 2 years ago • 27 comments

Current implementation plan for #2154

  • New option (arg and dartdoc_options.yaml): documentPrivate. Controls whether dartdoc documents private members
  • New option (arg and dartdoc_options.yaml): privateLibrariesInSidebar. Whether src libraries are in the sidebar
  • Changing logic of Documentable.isDocumented to ignore privacy if documentPrivate is true
    • that includes the logic of Canonicalization.isCanonical
  • Renaming all instances of Container.publicX to Container.documentedX
    • these fields control which elements appear in the documentation, but that's no longer driven by public vs private. Instead of checking Privacy.isPublic, they will check Documentable.isDocumented
    • this means switching most usages of model_utils.filterNonPublic to model_utils.filterNonDocumented, perhaps even removing the former entirely.

cc @jcollins-g, @srawlins, does this sound right to you?

Levi-Lesches avatar Jul 25 '22 00:07 Levi-Lesches

Question: The following getter (InheritedContainer.publicInterfaces) exposes a superchain of public interfaces. If documentPrivate is true, is this method redundant? Or does it do something else important?

https://github.com/dart-lang/dartdoc/blob/d3b0b724972ffcc70a47c35a31051af2c3ca012d/lib/src/model/inheriting_container.dart#L101-L137

Also, I plan to hide 3rd party private elements from the documentation. Does this getter do that?

Levi-Lesches avatar Jul 25 '22 01:07 Levi-Lesches

Thanks for taking this on!

New option (arg and dartdoc_options.yaml): privateLibrariesInSidebar. Whether src libraries are in the sidebar

I'm not in favor of a second option at this point. Each option represents complexity that must be maintained, and we shouldn't be maintaining a lot of complexity just for this feature. I think we should start by either not document any "private" libraries in the sidebar, or by including all of them.

On the one hand, I don't like generating documentation where the intended discoverability is "search for it." On the other, I can see how adding "private" libraries in the sidebar makes the sidebar completely useless.

I'm not sure which way I'd prefer at first...

Renaming all instances of Container.publicX to Container.documentedX

This sounds fine, I think (have to check that it isn't confused with any "has documentation" concept elsewhere. But let's do it in a PR which is separate from others.

is this method redundant?

Maybe? Redundant with what?

Or does it do something else important?

I doubt it.

Also, I plan to hide 3rd party private elements from the documentation. Does this getter do that?

Good question. Not sure about that...

srawlins avatar Jul 25 '22 03:07 srawlins

Thanks for helping! After going through it I can see why this was put off for a while, it's a lot more involved than I originally thought.

On the one hand, I don't like generating documentation where the intended discoverability is "search for it." On the other, I can see how adding "private" libraries in the sidebar makes the sidebar completely useless.

I thought about this a little more, and here's another thing we missed. Consider the "average" implementation file.

// lib/src/a.dart

/// This is a class called [A]. 
abstract class A { /* Some members here */ }

The thing about implementation files is that they're usually just containers around one or maybe a few classes/enums that already have separate documentation. At the same time, the library isn't a complete collection of such classes that need an overview, as public libraries are. In other words, there's usually no reason for implementation libraries to have their own documentation. Only when you export A, B, C, and the others can you describe how to use them with a library comment.

// lib/myPackage.dart

/// This library exports classes [A], [B], [C], and enum [D]. 
/// 
/// - [B] and [C] are concrete instances of [A]. 
/// - [D] is for arguments passed to the other classes. 
/// Note: Use [C] instead of [B] on Linux. 
library myPackage;

export "src/a.dart"; 
// ...

You're not losing much value by excluding implementation libraries from the sidebar by default. If, for some reason, the reader needs to see docs for a src library, they have two options:

  • Search for it explicitly
  • Use the breadcrumbs on any page that documents a class/enum/etc defined in that library
  • Third option again: maybe we can make a new Exported LIbraries section? Not sure if it's worth it or even possible

I think that's good enough if the important docs are reserved for public libraries (like they are today).

have to check that it isn't confused with any "has documentation" concept elsewhere. But let's do it in a PR which is separate from others.

There is -- Documentable.hasDocumentation. Took me a second, but I believe the difference is that hasDocumentation checks whether the element has doc comments and isDocumented tracks whether dartdoc will generate its docs. At least, that's how I'm using it so far.

Maybe? Redundant with what?

This function seems very careful about filtering private classes from the super chain/implementors chain. But if the user chooses to include private elements, does that mean all the logic here should be ignored? If so, I'd do if (documentPrivate) return directInterfaces; right away.

As for progress, I made most of the changes but the smoke test is throwing a stack overflow... I'll keep hacking at it. Should I open a draft PR in the meantime, or just wait until it's all done and tested?

Levi-Lesches avatar Jul 25 '22 03:07 Levi-Lesches

I think that's good enough if the important docs are reserved for public libraries (like they are today).

This makes sense to me. Let's not add non-"public" libraries to the sidebar, and I don't think we need an option to add them either. I think a better system in the future would be some JS in the front-end to toggle them.

Took me a second, but I believe the difference is that hasDocumentation checks whether the element has doc comments and isDocumented tracks whether dartdoc will generate its docs. At least, that's how I'm using it so far.

SG. Just to keep it in mind. Naming is hard 🤣

But if the user chooses to include private elements, does that mean all the logic here should be ignored?

Yes, I think so.

If so, I'd do if (documentPrivate) return directInterfaces; right away.

That makes sense.

As for progress, I made most of the changes but the smoke test is throwing a stack overflow... I'll keep hacking at it. Should I open a draft PR in the meantime, or just wait until it's all done and tested?

You could draft a PR in the meantime; might take me more than a day to get to a review, so OK to start early.

srawlins avatar Jul 26 '22 17:07 srawlins

Thanks for the feedback. I opened a draft PR at #3097 (currently blocked by #3099), I'll update there when it's ready for review.

Levi-Lesches avatar Jul 27 '22 00:07 Levi-Lesches

Any update on this?

BellRampion avatar Sep 13 '22 16:09 BellRampion

No update from me.

srawlins avatar Sep 13 '22 16:09 srawlins

Unfortunately, none from me either, as I've had a few busy weeks. But things are generally clearing up and I hope to get back to this (after taking into account all the PRs that have been merged since I last tested)

Levi-Lesches avatar Sep 14 '22 21:09 Levi-Lesches

This has bitten me.

I expected to use dart doc to document my code, in under src/. What other purpose might it have? I cannot imagine, but obviously it does.

I have been diligently using doc comments on all my classes and methods, and expected that the documentation tool would read them and write a useful HTML tree of documentation for those comments.

Silly me, it seems. Am I missing something?

Took a while to find this issue, and realise it was not me that had done it wrong, but that dart doc is for something other than documenting the code I am writing.

I definitely want a tool that reads my code and processes my doc comments. I am wondering what purpose (what possible purpose) dart doc can have if it ignores all my code under src/.

WorikQCI avatar Nov 03 '22 20:11 WorikQCI

Am I missing something?

Possibly. The primary purpose of dartdoc is to document public API. Public API is found in libraries declared directly in a package's lib directory, including elements in libraries exported from such libraries. You may not have a public API if you have no elements that match this description.

srawlins avatar Nov 03 '22 21:11 srawlins

Possibly. The primary purpose of dartdoc is to document public API

Thank you. I was missing that.

The code we write is not for releasing as a package. I take it that if it were, we would find dart doc very useful.

What we want dart doc for is communicating between developers. We have a team. If dart doc would process our code it would be very useful to us.

It is a very important use case, is it not?

Never mind. I learnt something today. I do hope you extend dart doc so that I have a use for it.

Thank you for your attention!

WorikQCI avatar Nov 03 '22 21:11 WorikQCI

What we want dart doc for is communicating between developers. We have a team. If dart doc would process our code it would be very useful to us.

It is a very important use case, is it not?

It is to me! And to all Flutter developers -- none of their code is being published on pub.dev, but they still need to be documented. I'm a bit swamped with other projects at the moment but I'm getting notifications to this repository to remind me every day I have a PR in the works. @srawlins if you feel I'm taking too long feel free to use my few commits and our discussions as a jumping point, but I'll take a look at this again in a few weeks.

Levi-Lesches avatar Nov 04 '22 02:11 Levi-Lesches

I'm just another new Dart developer who found this issue while searching for the reason that my classes aren't processed by dart doc. I want to add to the voices expressing the belief that "public documented API" is a concept that should be completely independent of "directory structure". Looking forward to seeing this feature implemented! Thank you @Levi-Lesches for your help.

philipmjohnson avatar Nov 29 '22 16:11 philipmjohnson

+++

Atom735 avatar Mar 16 '23 09:03 Atom735

I tried to Very dirty hack: You can add directy dependency from dartdoc to pubscpec.yaml, and then copy from thats package src/dartdoc.dart file to your project src folder (not /lib/src/dartdoc.dart, only /src/dartdoc.dart). Then resolve pub get. And at the end edit file https://github.com/dart-lang/dartdoc/blob/7fde7a4d61dd251a4e48bf85ccab34f28ec8da71/lib/src/model/package_builder.dart#L317 And then run in shell dart run bin/dartdoc.dart --show-progress But it not working(((

Atom735 avatar Mar 16 '23 09:03 Atom735

Also needed here for flutter... :-/

TechupBusiness avatar Apr 25 '23 19:04 TechupBusiness

Still very much interested in this! The very project that pushed me to start this PR is getting extremely busy until June, but I should have some more time after that.

Levi-Lesches avatar Apr 28 '23 06:04 Levi-Lesches

Also very interested in this, @Levi-Lesches let me know if you need help with implementation

paul-paliychuk avatar Aug 07 '23 20:08 paul-paliychuk

Hi all! 👋 I am also super interested in it, it was the perfect tool for my need until I realise as well that none of the files under the src would be documented as it is considered private (I also want to have my code documented for my colleagues).

I'm also willing to help @Levi-Lesches (or anybody else) if it can make things move faster, I've seen this issue exists for at least 8 years now 🦦

Cyrille-Dakhlia avatar Oct 02 '23 16:10 Cyrille-Dakhlia

Hi! Believe it or not I haven't totally forgotten about this, just have other work that's still keeping me busy. @Cyrille-Dakhlia I'm open to help over at #3097 which should be mostly done barring some actual unit tests. Also I know there have been upstream changes to the API since I made my PR but I believe a lot of them are non-breaking.

Levi-Lesches avatar Oct 02 '23 17:10 Levi-Lesches

Glad to hear that @Levi-Lesches ! I'll use some of my free time (as much as I could, it will take time that's for sure) to review your PR and try to add some unit tests if possible.

I've seen a lot of build failures so it will be a good place to start maybe, I'll update you here!

Cyrille-Dakhlia avatar Oct 04 '23 15:10 Cyrille-Dakhlia

This is a needed feature. Any updates?

jonpittock avatar Dec 06 '23 11:12 jonpittock

Not yet sadly. Still working on other projects in the meantime, maybe sometime in the next month I can pick it back up but I would also appreciate any help on the PR.

Levi-Lesches avatar Dec 06 '23 18:12 Levi-Lesches

What is left and needing doing?

jonpittock avatar Dec 06 '23 19:12 jonpittock

Mostly tests and a few missing cases I didn't finish yet. Unfortunately, it's been so long that I'd need to go back and look in-depth at what was TODO. But in terms of tests -- there are a lot of existing tests that specifically expect private members to be hidden, and this PR would need to duplicate those tests to expect them to be shown when this feature is enabled.

Otherwise, the PR is a good proof-of-concept in that it does produce documentation for most private members.

Levi-Lesches avatar Dec 06 '23 19:12 Levi-Lesches

@Levi-Lesches
does the package support documentation for private classes now ? class _ClassNameUIState extends State<ClassNameUI> i can only see generated documents for public classes

SoyahDev avatar May 20 '24 13:05 SoyahDev