docgen: ingore test types and only document public types by default #2112

mfelsche · 2017-07-30T19:17:30Z

This PR changes the docgen pass (invoked by calling ponyc --docs) to ignore all types that are considered to be test related. That is:

private types with names starting with _Test
TestList and UnitTest implementations
whole packages called builtin_test (static check to filter out stdlib package) or test

A new command-line flag "--docs-public" is introduced to only generate documentation for public types, behaviors, constructors and methods.

This fixes #2092 and #2089

mfelsche · 2017-07-30T19:24:40Z

Open questions

Should the stdlib docs keep the private types (as generated in the release process by calling make docs)?
The numeric primitive types all inherit from _SignedInteger or _UnsignedInteger only. Doc-wise their connection to Integer is lost, as the private types _SignedInteger and UnsigendInteger are not documented when leaving out private types and not links are generated. Listing all known sub-interfaces/-traits and implementing classes would help here. Maybe it is also enough for stdlib docs to exclude the test types and keep the private ones?
I could start writing tests for the docgen, but have no experience with unittests for ASTs in pony. Any help is much appreciated. Tests could help ensure that doc generation stays sober across future ast changes.

SeanTAllen · 2017-07-31T21:23:02Z

src/libponyc/pass/docgen.c

+// Define options for doc generation
+typedef struct docgen_opt_t
+{
+    bool include_private;


this should only be indented 2 spaces.

SeanTAllen · 2017-07-31T21:23:48Z

src/libponyc/pass/docgen.c

-static void doc_type_list(docgen_t* docgen, ast_t* list, const char* preamble,
-  const char* separator, const char* postamble, bool generate_links, bool line_breaks);
+static void doc_type_list(docgen_t* docgen, docgen_opt_t* docgen_opt, ast_t* list,
+        const char* preamble, const char* separator, const char* postamble,


to keep formatting consistent the formatting here should be 2 spaces

SeanTAllen · 2017-07-31T21:23:57Z

src/libponyc/pass/docgen.c

@@ -291,7 +343,8 @@ static const char* doc_get_cap(ast_t* cap)


 // Write the given type to the current type file
-static void doc_type(docgen_t* docgen, ast_t* type, bool generate_links)
+static void doc_type(docgen_t* docgen, docgen_opt_t* docgen_opt,
+                     ast_t* type, bool generate_links)


2 space indent only

SeanTAllen · 2017-07-31T21:24:15Z

src/libponyc/pass/docgen.c

+      // and if the type not private if we exclude private types.
+      const char* type_id_name = ast_name(id);
+      if(generate_links && *type_id_name != '$'
+              && (docgen_opt->include_private || !is_name_private(type_id_name)))


SeanTAllen · 2017-07-31T21:24:38Z

src/libponyc/pass/docgen.c

-  const char* separator, const char* postamble, bool generate_links, bool line_breaks)
+static void doc_type_list(docgen_t* docgen, docgen_opt_t* docgen_opt, ast_t* list,
+        const char* preamble, const char* separator, const char* postamble,
+        bool generate_links, bool line_breaks)


SeanTAllen · 2017-07-31T21:25:22Z

src/libponyc/pass/docgen.c



 // Functions to handle everything else

 // Write the given list of fields to the current type file.
 // The given title text is used as a section header.
 // If the field list is empty nothing is written.
-static void doc_fields(docgen_t* docgen, ast_list_t* fields, const char* title)
+static void doc_fields(docgen_t* docgen, docgen_opt_t* docgen_opt, ast_list_t* fields, const char* title)


this should be wrapped so its no more than 80 columns

SeanTAllen · 2017-07-31T21:25:49Z

src/libponyc/pass/docgen.c

    fprintf(docgen->type_file, "\n\n---\n\n");
  }
 }


 // Write the given list of type parameters to the current type file, with
 // surrounding []. If the given list is empty nothing is written.
-static void doc_type_params(docgen_t* docgen, ast_t* t_params,
+static void doc_type_params(docgen_t* docgen, docgen_opt_t* docgen_opt, ast_t* t_params,


this should be wrapped at 80 columns

SeanTAllen · 2017-07-31T21:26:15Z

src/libponyc/pass/docgen.c

@@ -550,7 +580,8 @@ static void doc_type_params(docgen_t* docgen, ast_t* t_params,

 // Write the given list of parameters to the current type file, with
 // surrounding (). If the given list is empty () is still written.
-static void code_block_doc_params(docgen_t* docgen, ast_t* params)
+static void code_block_doc_params(docgen_t* docgen, docgen_opt_t* docgen_opt,
+        ast_t* params)


only indent 2

SeanTAllen · 2017-07-31T21:26:22Z

src/libponyc/pass/docgen.c

@@ -592,7 +623,8 @@ static void code_block_doc_params(docgen_t* docgen, ast_t* params)
  fprintf(docgen->type_file, ")");
 }

-static void list_doc_params(docgen_t* docgen, ast_t* params)
+static void list_doc_params(docgen_t* docgen, docgen_opt_t* docgen_opt,
+        ast_t* params)


SeanTAllen · 2017-07-31T21:26:27Z

src/libponyc/pass/docgen.c

@@ -636,7 +668,8 @@ static void list_doc_params(docgen_t* docgen, ast_t* params)
 }

 // Write a description of the given method to the current type file
-static void doc_method(docgen_t* docgen, ast_t* method)
+static void doc_method(docgen_t* docgen, docgen_opt_t* docgen_opt, 
+        ast_t* method)


SeanTAllen · 2017-07-31T21:27:19Z

src/libponyc/pass/docgen.c

-  doc_type_params(docgen, tparams, false);
-  doc_type_list(docgen, provides, " is\n  ", ",\n  ", "", false, false);
+  doc_type_params(docgen, docgen_opt, tparams, false);
+  doc_type_list(docgen, docgen_opt, provides, " is\n  ", ",\n  ", "", false, false);


this should be wrapped to 80

SeanTAllen · 2017-07-31T21:27:48Z

src/libponyc/pass/docgen.c

        break;

      case TK_BE:
        doc_list_add_named(&pub_bes, p, 1, true, false);
-        doc_list_add_named(&priv_bes, p, 1, false, true);
+        doc_list_add_named(&priv_bes, p, 1, false, docgen_opt->include_private);


should wrap to 80

SeanTAllen · 2017-07-31T21:27:58Z

src/libponyc/pass/docgen.c

        break;

      case TK_FUN:
        doc_list_add_named(&pub_funs, p, 1, true, false);
-        doc_list_add_named(&priv_funs, p, 1, false, true);
+        doc_list_add_named(&priv_funs, p, 1, false, docgen_opt->include_private);


should wrap to 80

SeanTAllen · 2017-07-31T21:28:40Z

src/libponyc/pass/docgen.c

+                  t,
+                  0,
+                  true,
+                  docgen_opt->include_private


SeanTAllen · 2017-07-31T21:29:14Z

src/libponyc/pass/docgen.c

  docgen->public_types = NULL;
  docgen->private_types = NULL;
 }


 // Document the packages in the given program
-static void doc_packages(docgen_t* docgen, ast_t* ast)
+static void doc_packages(docgen_t* docgen, docgen_opt_t* docgen_opt, ast_t* ast)


should wrap to 80

SeanTAllen · 2017-07-31T21:29:28Z

src/libponyc/pass/docgen.c

+  for(ast_list_t* p = packages.next; p != NULL; p = p->next) {
+    const char* p_name = package_qualified_name(p->ast);
+    if (!is_package_for_testing(p_name)) {
+        doc_package(docgen, docgen_opt, p->ast);


too much indentation

SeanTAllen · 2017-07-31T21:29:42Z

src/libponyc/pass/docgen.c


-  for(ast_list_t* p = packages.next; p != NULL; p = p->next)
-    doc_package(docgen, p->ast);
+  for(ast_list_t* p = packages.next; p != NULL; p = p->next) {


{ goes on next line

SeanTAllen · 2017-07-31T21:29:46Z

src/libponyc/pass/docgen.c

-    doc_package(docgen, p->ast);
+  for(ast_list_t* p = packages.next; p != NULL; p = p->next) {
+    const char* p_name = package_qualified_name(p->ast);
+    if (!is_package_for_testing(p_name)) {


{ goes on next line

SeanTAllen · 2017-07-31T21:29:57Z

src/libponyc/pass/docgen.c

-    doc_package(docgen, p->ast);
+  for(ast_list_t* p = packages.next; p != NULL; p = p->next) {
+    const char* p_name = package_qualified_name(p->ast);
+    if (!is_package_for_testing(p_name)) {


no space between if and (

SeanTAllen · 2017-07-31T21:30:31Z

src/ponyc/main.c

@@ -120,6 +122,8 @@ static void usage()
    "  --runtimebc     Compile with the LLVM bitcode file for the runtime.\n"
    "  --pic           Compile using position independent code.\n"
    "  --docs, -g      Generate code documentation.\n"
+    "  --docs-private  Generate code documentation for private types\n"


what happens if this is used without --docs ?

nothing, at the moment.
The compiler just does its passes (like it would do without).
I will ad a stance about it to the help string and add a warning output that it will be ignored.

Other possibility is to model this switch differently, having --docs-public generating docs for public types only and --docs generating docs for both, just like --docs did before.

Now that i think about it, adding --docs-public looks about right to me. Give me a day to implement the different behavior (and the formatting). :)

SeanTAllen · 2017-07-31T21:31:54Z

@mfelsche thanks for this. there's a lot of code formatting style things to address but they should be quick. everything makes sense to me although, the proof is in the output. i'm assuming you've tested it and get the correct stuff.

once the formatting stuff is taken care of i think we can merge this unless someone else see something off.

thanks for this!

SeanTAllen · 2017-07-31T21:32:15Z

@ponylang/committer we really need to document the c-style as well as the pony.

this includes implementations of TestList and UnitTest as well as classes starting with _Test. This does not exclude packages named test (like builtin/test)

when calling ponyc with --docs option. To enable doc generation for private types, methods and behaviours as well, the flag --docs-private can be used.

otherwise they would show up in generated documentation

Oops.

to produce public-only docs only when used with --docs-public and create docs for private types as well, when using --docs. This leaves the old behavior untouched.

mfelsche · 2017-07-31T23:07:59Z

with 22d9f3c this PR will actually leave the private types inside the stdlib docs.

mfelsche · 2017-07-31T23:08:41Z

The only thing i could add are some tests for properly detecting test and private types during docgen.

SeanTAllen · 2017-08-01T01:04:38Z

I'm not particularly worried about tests at the moment. Let's have @Praetonus or @jemc give this a once over. It looks good to me but I've had a long day so another set of eyes wouldn't hurt.

SeanTAllen · 2017-08-02T19:41:30Z

Thanks @mfelsche!

mfelsche · 2017-08-02T20:21:47Z

٩(ˊᗜˋ*)و

SeanTAllen reviewed Jul 31, 2017

View reviewed changes

src/libponyc/pass/docgen.c Outdated

t,

0,

true,

docgen_opt->include_private

Copy link

Member

SeanTAllen Jul 31, 2017

Choose a reason for hiding this comment

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

formatting

SeanTAllen reviewed Jul 31, 2017

View reviewed changes

mfelsche mentioned this pull request Jul 31, 2017

[stdlib documentation] Only generate docs for public types #2092

Closed

mfelsche added 6 commits August 1, 2017 01:04

dont document types recognizes as test related

648ca50

this includes implementations of TestList and UnitTest as well as classes starting with _Test. This does not exclude packages named test (like builtin/test)

only document public types by default

d350237

when calling ponyc with --docs option. To enable doc generation for private types, methods and behaviours as well, the flag --docs-private can be used.

properly detect TestList actors in docgen

b3c86ea

otherwise they would show up in generated documentation

docgen: ignore builtin_test package and packages named 'test'

1e2f273

don't use private as variable name

d735141

Oops.

change the cli for producing public-only docs and private-as-well docs

22d9f3c

to produce public-only docs only when used with --docs-public and create docs for private types as well, when using --docs. This leaves the old behavior untouched.

fix formatting

3a12d45

mfelsche force-pushed the doc/private branch from 2d2ad35 to 3a12d45 Compare July 31, 2017 23:06

SeanTAllen added the changelog - changed Automatically add "Changed" CHANGELOG entry on merge label Aug 2, 2017

SeanTAllen merged commit c4f9905 into ponylang:master Aug 2, 2017

ponylang-main added a commit that referenced this pull request Aug 2, 2017

Update CHANGELOG for PR #2112 [skip ci]

decbacd

mfelsche deleted the doc/private branch August 2, 2017 20:21

mfelsche mentioned this pull request Aug 2, 2017

[stdlib documentation] Removal of test types #2089

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docgen: ingore test types and only document public types by default #2112

docgen: ingore test types and only document public types by default #2112

mfelsche commented Jul 30, 2017 •

edited

Loading

mfelsche commented Jul 30, 2017

SeanTAllen Jul 31, 2017

SeanTAllen Jul 31, 2017

SeanTAllen Jul 31, 2017

SeanTAllen Jul 31, 2017

SeanTAllen Jul 31, 2017

SeanTAllen Jul 31, 2017

SeanTAllen Jul 31, 2017

SeanTAllen Jul 31, 2017

SeanTAllen Jul 31, 2017

SeanTAllen Jul 31, 2017

SeanTAllen Jul 31, 2017

SeanTAllen Jul 31, 2017

SeanTAllen Jul 31, 2017

SeanTAllen Jul 31, 2017

SeanTAllen Jul 31, 2017

SeanTAllen Jul 31, 2017

SeanTAllen Jul 31, 2017

SeanTAllen Jul 31, 2017

SeanTAllen Jul 31, 2017

SeanTAllen Jul 31, 2017

mfelsche Jul 31, 2017

SeanTAllen commented Jul 31, 2017

SeanTAllen commented Jul 31, 2017

mfelsche commented Jul 31, 2017

mfelsche commented Jul 31, 2017

SeanTAllen commented Aug 1, 2017

SeanTAllen commented Aug 2, 2017

mfelsche commented Aug 2, 2017

docgen: ingore test types and only document public types by default #2112

docgen: ingore test types and only document public types by default #2112

Conversation

mfelsche commented Jul 30, 2017 • edited Loading

mfelsche commented Jul 30, 2017

Open questions

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SeanTAllen commented Jul 31, 2017

SeanTAllen commented Jul 31, 2017

mfelsche commented Jul 31, 2017

mfelsche commented Jul 31, 2017

SeanTAllen commented Aug 1, 2017

SeanTAllen commented Aug 2, 2017

mfelsche commented Aug 2, 2017

mfelsche commented Jul 30, 2017 •

edited

Loading