Take expansion's doc when computing a declaration's synopsis #643

Julow · 2021-03-16T14:13:27Z

Fixes #632

Before this patch, no synopsis was attached to declarations like this: (also work on classes, class types and module types)

module M : sig
  (** Expansion doc. *)
end

This is consistent with what ocamldoc does.

The first commit refactors the function generating the synopsis for {!modules:} lists so it can be shared with the generator. Before that, a simpler rule was used that didn't skip headings and other elements:

(** {1 Heading}

    @canonical M

    This is the synopsis. *)

(** - This is the synopsis. *)

These are rules I made up in #597, it's still time to change them and to document them too.

Julow · 2021-03-16T14:16:13Z

src/document/generator.ml

@@ -1533,7 +1537,7 @@ module Make (Syntax : SYNTAX) = struct
      let content = { Include.content; status; summary } in
      let attr = [ "include" ] in
      let anchor = None in
-      let doc = Comment.first_to_ir sg_doc in
+      let doc = Comment.synopsis ~decl_doc:[] ~expansion_doc:(Some sg_doc) in


Unrelated to this PR:
This ignores t.doc and takes only the first paragraph of the expansion doc, the rest is removed. I think we should at least show t.doc in its entirety.
I'll propose something is an other PR.

Issue: #645

dbuenzli · 2021-03-16T14:49:37Z

it's still time to change them and to document them too.

Could you please spell them out ? Because I'm not sure what you mean by skipping.

More precisely, nothing should be skipped. You simply take the first paragraph of the comment attached to module. If there is no such thing there is no synopsis. Do not try to peek past headings this will create a lot of bogus synopses.

For example here there should be no synopsis:

(** {1 Heading}

    @canonical M

    This is the synopsis. *)

dbuenzli · 2021-03-16T14:50:54Z

Here are examples of bogus synopses.

dbuenzli · 2021-03-16T15:01:44Z

If we agree on the current state of discussion between me and @lpw25 in #640 I would suggest the following definitions:

The preamble of a module are all the paragraphs in the comment attached to the module until the first heading (of any level).
The synopsis of a module is ~~the first paragraph of the preamble~~ the first element of the preamble, if it is a paragraph (as per @Julow's clarification).

This not exactly what ocamldoc did but it's what most documented modules out there do (some might need fixing to get a synopsis but not that much) and these definitions are easy to understand and remember (for their good properties see the discussion in #640).

Julow · 2021-03-16T15:15:35Z

Could you please spell them out ?

The temporary rule is: The synopsis is the first paragraph, from top to bottom, ignoring headings and tags but considering list items.

I agree it is surprising and produces bogus synopses. That's why I warned about it.

The synopsis of a module is the first paragraph of the preamble.

I think you actually suggest: the first element of the preamble, if it is a paragraph. (I guess also if it is a list and the first item has a paragraph as first element)
By "element" I mean "block element", which is defined in code as paragraphs, headings, tags, lists, verbatim/code blocks and module lists.

I suggest adding to this rule: Heading's text if the first element is a heading.
I think it's common to start with a heading and I'd find it confusing to get an empty synopsis.

dbuenzli · 2021-03-16T15:44:06Z

The temporary rule is: The synopsis is the first paragraph, from top to bottom, ignoring headings and tags but considering list items.

I don't think it's a good idea.

I think you actually suggest: the first element of the preamble, if it is a paragraph.

Yes. Note that the simple new rule I propose will not produce bogus synopses (except for those that relied on the ill-defined sentence extract). It will however suppress some, namely those who relied on synopses being extracted from headings, like those I suspect.

I suggest adding to this rule: Heading's text if the first element is a heading.

I'd rather not because then it all becomes confusing. The heading is not part of the preamble so we loose the simple and obvious definitions.

Besides it also makes the purpose of headings shady. Headings are here to chunk the module contents. I do not like to overload or change their functionality when they happen to appear at the beginning of a module.

Finally a synopsis is supposed to be a small sentence terminated by a dot. Headings are not supposed to be terminated by a dot.

I think it's common to start with a heading and I'd find it confusing to get an empty synopsis.

I'm not sure it's common. In any case in general you should rather start with a preamble. There's already a heading starting the page which is module M.

The pattern supported by the rules I propose has been well-established by the stdlib for as long as I have been programming in OCaml and as I mentioned here corresponds to what you'll find most of the time out there.

dbuenzli · 2021-03-16T16:28:31Z

One thing I forgot to mention is that by having less ways on how synopses are defined we enforce more page structure regularity across APIs which overall improves the OCaml documentation user experience.

Julow · 2021-03-16T16:49:54Z

I changed the rule according to our discussion.

dbuenzli · 2021-03-16T17:11:36Z

An exception is made if it is a list, the first element of the first
list item is considered.

I'm not sure why you made that exception, is there any documentation pattern you have in mind ? Lists are usually cohesive blocks, extracting out the first paragraph of their first element seems a bit weird.

Julow · 2021-03-16T17:56:37Z

Lists contain paragraphs. I don't have a strong reason, that's how I'd expect it to work. Ocamldoc was taking the first sentence of anything (heading, paragraph, verbatim block), taking more seems more consistent than otherwise.

dbuenzli · 2021-03-16T18:56:37Z

I don't have a strong reason, that's how I'd expect it to work.

Ah ! I don't :-) Especially from a writing perspective I don't see the first paragraph of the first element of a list as being a particularly self-contained and self-identified unit. I'd rather see that as a bug.

Beyond that writing the documentation features, i.e. explaining features for humans, is always useful to judge them (if you can't explain it simply, iterate…) – I did my best to write something for what you propose, maybe you can make it simpler but I still prefer the first definition.

The synopsis of a module is the first block of the preamble, if it is a paragraph.
The synopsis of a module is defined by the first block of the preamble. If the first block is a paragraph, it is the paragraph itself. If the first block is a list, it is the first paragraph of the first item of the list. Otherwise the synopsis is empty.

jonludlam · 2021-03-17T17:55:48Z

Rebase needed please!

dbuenzli · 2021-03-17T18:53:02Z

Also this still implements definition 2. the docs of #644 use definition 1.

Julow · 2021-03-18T15:05:09Z

Does anyone have an opinion on this? (whether we should use the first item of a list if it is the first element)

jonludlam · 2021-03-18T16:17:26Z

I think I'm with @dbuenzli on this one, I wouldn't have expected to pick out the first item in a list.

Allow sharing this function with the generator.

Before this patch, no synopsis was attached to declarations like this: ``` module M : sig (** Expansion doc. *) end ```

The synopsis is now the first element of the preamble, if it is a paragraph. An exception is made if it is a list, the first element of the first list item is considered. The preamble is the comment attached to the definition followed by the first comment of the expansion, up to the first heading.

The previous code was using the expansion doc even if the decl doc wasn't empty and didn't contain a synopsis. This doesn't work with the definition of synopsis in term of preamble.

Julow · 2021-03-19T11:47:47Z

I've removed the exception on lists. I've also fixed a bug where the synopsis could be computed for the expansion doc even if the decl doc was present but didn't contain a synopsis.

jonludlam · 2021-03-25T14:24:07Z

AIUI this is now ready to merge, correct?

Julow · 2021-03-25T15:28:50Z

I believe it is.

Julow commented Mar 16, 2021

View reviewed changes

Julow mentioned this pull request Mar 17, 2021

Document how to write documentation for modules #644

Merged

Julow added 5 commits March 19, 2021 12:41

Move Link.synopsis_from_comment to Comment

0ef2ccd

Allow sharing this function with the generator.

Take expansion's doc when computing a declaration's synopsis

c5a4ba6

Before this patch, no synopsis was attached to declarations like this: ``` module M : sig (** Expansion doc. *) end ```

Fix synopsis not computed from preamble

9b45c25

The previous code was using the expansion doc even if the decl doc wasn't empty and didn't contain a synopsis. This doesn't work with the definition of synopsis in term of preamble.

Doesn't handle lists when computing the synopsis

0292ffe

Julow force-pushed the module-decl-synopsis branch from 9cab356 to 0292ffe Compare March 19, 2021 11:45

Julow mentioned this pull request Mar 19, 2021

Render documentation attached to includes #647

Merged

jonludlam merged commit 069b961 into ocaml:master Mar 25, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Take expansion's doc when computing a declaration's synopsis #643

Take expansion's doc when computing a declaration's synopsis #643

Julow commented Mar 16, 2021

Julow Mar 16, 2021

Julow Mar 17, 2021

dbuenzli commented Mar 16, 2021

dbuenzli commented Mar 16, 2021 •

edited

Loading

dbuenzli commented Mar 16, 2021 •

edited

Loading

Julow commented Mar 16, 2021

dbuenzli commented Mar 16, 2021

dbuenzli commented Mar 16, 2021

Julow commented Mar 16, 2021

dbuenzli commented Mar 16, 2021

Julow commented Mar 16, 2021

dbuenzli commented Mar 16, 2021

jonludlam commented Mar 17, 2021

dbuenzli commented Mar 17, 2021

Julow commented Mar 18, 2021

jonludlam commented Mar 18, 2021

Julow commented Mar 19, 2021

jonludlam commented Mar 25, 2021

Julow commented Mar 25, 2021

Take expansion's doc when computing a declaration's synopsis #643

Take expansion's doc when computing a declaration's synopsis #643

Conversation

Julow commented Mar 16, 2021

Julow Mar 16, 2021

Choose a reason for hiding this comment

Julow Mar 17, 2021

Choose a reason for hiding this comment

dbuenzli commented Mar 16, 2021

dbuenzli commented Mar 16, 2021 • edited Loading

dbuenzli commented Mar 16, 2021 • edited Loading

Julow commented Mar 16, 2021

dbuenzli commented Mar 16, 2021

dbuenzli commented Mar 16, 2021

Julow commented Mar 16, 2021

dbuenzli commented Mar 16, 2021

Julow commented Mar 16, 2021

dbuenzli commented Mar 16, 2021

jonludlam commented Mar 17, 2021

dbuenzli commented Mar 17, 2021

Julow commented Mar 18, 2021

jonludlam commented Mar 18, 2021

Julow commented Mar 19, 2021

jonludlam commented Mar 25, 2021

Julow commented Mar 25, 2021

dbuenzli commented Mar 16, 2021 •

edited

Loading

dbuenzli commented Mar 16, 2021 •

edited

Loading