Improve: parse doc comments with Odoc #721

Julow · 2019-03-27T14:07:22Z

(issue #571)

Work in progress, but the current state is relatively clean and can be reviewed.

Rewrite Fmt_odoc and Normalize to use Odoc.
It adds a lot of regressions and fixes a few bugs.

Odoc 1.4.0 exports its internal libraries, it's no longer needed to vendor it. Other small problems remain.

src/Sugar.mli

src/dune

test/passing/doc_comments.mli

hhugo · 2019-04-01T15:35:51Z

How far are we to have this PR ready ? I'm wondering if we should try to fix Octavious for 4.08 or if this PR will land in reasonable time ?

Julow · 2019-04-19T10:01:49Z

Sorry for the delay.

I think this PR can land in reasonable time from now. I'm not 100% confident it can before the OCaml release.

It is not ready for merging but should be ready for a new review.

I rebased on master.

Since last reviews:

Removed unecessary blank lines between some elements
Don't format comments when Odoc generated warnings
Because Odoc may have lost informations. In practice Odoc try its best to not to.
Use Odoc parser's Ast instead of Model.Comment
It is a bit simpler and the parser is a lot simpler to call.
I made a PR to Odoc: Expose parser internals and add Parser.parse_comment_raw ocaml/odoc#351
This fixes label on headings
Removed terminating break in list elements with heavy syntax
Fixed the formatting of references
This part is ridiculously hard. Odoc could be modified to improve this but it would be a big change.

gpetiot

Some minor comments, I hope we can fix the remaining regressions, otherwise looks good!

src/Fmt_odoc.ml

Julow · 2019-06-18T16:58:36Z

Now that my PRs on Odoc have been merged (ocaml/odoc#351, ocaml/odoc#364), this can be implemented more easily.

Rebased on master and fixed a few bugs to to the rebase and changes in Odoc.
Currently, it is needed to pin Odoc to master.

ocamlformat.opam

ocamlformat_reason.opam

Julow · 2019-06-25T11:40:37Z

Rebased on master. The history is a mess, this should be reviewed file by file.

gpetiot · 2019-06-27T07:45:14Z

The diff with test_branch is:

diff --git a/src/Sugar.mli b/src/Sugar.mli
index 81514eb..db5f9f6 100644
--- a/src/Sugar.mli
+++ b/src/Sugar.mli
@@ -93,9 +93,9 @@ val ite :
     if-then-else expressions, e.g.:
 
     {[
-        if c1 then e1
-        else if c2 then e2
-        else e3
+if c1 then e1
+else if c2 then e2
+else e3
     ]}
 
     will return the following list:
@@ -141,11 +141,9 @@ val polynewtype :
     pattern [pat] with body [exp]. e.g.:
 
     {[
-        let f: 'r 's. 'r 's t = fun (type r) -> fun (type s) -> (e : r s t)
+let f: 'r 's. 'r 's t = fun (type r) -> fun (type s) -> (e : r s t)
     ]}
 
     Can be rewritten as:
 
-    {[
-        let f: type r s. r s t = e
-    ]} *)
+    {[ let f: type r s. r s t = e ]} *)

gpetiot

Looks good to me, good job.
(minor comments on the warning messages when parse-docstrings=false)

Basically the diff produced by tools/test_branch.sh odoc_instead_of_octavius 'parse-docstrings=false' 'parse-docstrings=false' should be empty.

src/Fmt_ast.ml

Julow · 2019-06-27T13:28:31Z

Thanks for the review. I improved the indentation of code blocks. I keep the behavior of fitting single line blocks.
I cleaned up a little and moved the printing of the warning.

What is not yet implemented is:

Preserve list syntax
Preserve whitespaces

I'm not sure if we should and how to preserve whitespaces. This should be done later on its own PR.

gpetiot · 2019-06-27T15:53:15Z

tools/test_branch.sh odoc_instead_of_octavius 'parse-docstrings=true' 'parse-docstrings=true' shows the formatting of some docstrings became unstable (probably due to 87375aa)

Julow · 2019-06-27T16:31:03Z

The unstable docstring errors are from the master build. They should all be fixed by this PR. This commit is clean.

gpetiot · 2019-06-28T04:58:37Z

The unstable docstring errors are from the master build. They should all be fixed by this PR. This commit is clean.

For example ocamlformat test/code/infer/infer/src/biabduction/Prop.mli --no-version-check --parse-docstrings fails with master but also with this PR, so it's not really a regression, but it would be nice if it would be fixed.

But we can fix it in another PR, feel free to merge.

Julow · 2019-06-28T12:04:42Z

I can't reproduce this bug. It fails on master but not on this branch. Do you have an older version of this branch ? (I rebased it 3 days ago)

gpetiot · 2019-06-28T14:02:31Z

I can't reproduce this bug. It fails on master but not on this branch. Do you have an older version of this branch ? (I rebased it 3 days ago)

No I use your rebased version.
Edit: my bad, my repository was not clean.

Requires a fix to Odoc

src/Fmt_odoc.ml

CHANGES: #### Changes + Set "conventional" as the default profile (ocaml-ppx/ocamlformat#1060) (Guillaume Petiot) This new profile is made to better match the most used style and is encouraged. To continue using the previous default, use `profile = ocamlformat` in your `.ocamlformat`. + CLI: Allow both values of boolean options (ocaml-ppx/ocamlformat#1062) (Jules Aguillon) Now, both `--opt` and --no-opt` are available on the CLI for any boolean option "opt". Previously, only one of them were available depending on the default value. + Auto mode for `break-string-literals` (ocaml-ppx/ocamlformat#1057) (Guillaume Petiot) `wrap`, `newlines` and `newlines-and-wrap` values of `break-string-literals` are removed. `auto` replaces them, it is equivalent to `newlines-and-wrap`. + Dock collection brackets (ocaml-ppx/ocamlformat#1014) (Guillaume Petiot) `after-and-docked` value of `break-separators` is removed and is replaced by a new `dock-collection-brackets` option. + Preserve `begin` and `end` keywords in if-then-else (ocaml-ppx/ocamlformat#978) (Jules Aguillon) Previously, `begin`/`end` keywords around if-then-else branches were turned into parentheses. #### New features + Give a hint when warning 50 is raised (ocaml-ppx/ocamlformat#1111) (Guillaume Petiot) + Add a message when a config value is removed (ocaml-ppx/ocamlformat#1089) (Etienne Millon) Explain what replaces removed options and avoid printing a parsing error. + Implement `sequence-blank-line=preserve-one` for let bindings (ocaml-ppx/ocamlformat#1077) (Jules Aguillon) Preserve a blank line after `let .. in` when `sequence-blank-line` set to `preserve-one`. Previously, only blank lines after `;` could be preserved. + Parse toplevel directives (ocaml-ppx/ocamlformat#1020) (Jules Aguillon) Allow `#directives` in `.ml` files. Previously, files containing a directive needed to be parsed as "use file". The "use file" mode is removed and `--use-file` is now the same as `--impl`. + Don't require `--name`, require kind, forbid `--inplace`, allow `--check`, make `--enable-outside-detected-project` implicit when reading from stdin (ocaml-ppx/ocamlformat#1018) (Guillaume Petiot) + Parse code in docstrings (ocaml-ppx/ocamlformat#941) (Guillaume Petiot) Format OCaml code in cinaps-style comments `(*$ code *)` and code blocks in documentation comments `(** {[ code ]} *)`. + Parse documentation comments with Odoc (ocaml-ppx/ocamlformat#721) (Jules Aguillon) Formatting of documentation comments is more robust and support newer Odoc syntaxes. Internally, Odoc replaces Octavius as the documentation parser. #### Bug fixes + Fix unstabilizing comments on assignments (ocaml-ppx/ocamlformat#1093) (Guillaume Petiot) + Fix the default value documentation for `max-indent` (ocaml-ppx/ocamlformat#1105) (Guillaume Petiot) + Fix closing parenthesis exceeding the margin in function application (ocaml-ppx/ocamlformat#1098) (Jules Aguillon) + Missing break before attributes of `Pmty_with` (ocaml-ppx/ocamlformat#1103) (Josh Berdine) + Fix closing quote exceeding the margin (ocaml-ppx/ocamlformat#1096) (Jules Aguillon) + Fix break before the closing bracket of collections (exceeding the margin) (ocaml-ppx/ocamlformat#1073) (Guillaume Petiot) + Fix precedence of Dot wrt Hash (ocaml-ppx/ocamlformat#1058) (Guillaume Petiot) + Fix break in variant type definition to not exceed the margin (ocaml-ppx/ocamlformat#1064) (Guillaume Petiot) + Fix newlines and indentation in toplevel extension points (ocaml-ppx/ocamlformat#1054) (Guillaume Petiot) + Fix placement of doc comments around extensions (ocaml-ppx/ocamlformat#1052) (Jules Aguillon) + Inline extensions that do not break (ocaml-ppx/ocamlformat#1050) (Guillaume Petiot) + Add missing cut before attributes in type declarations (ocaml-ppx/ocamlformat#1051) (Guillaume Petiot) + Fix alignment of cases (ocaml-ppx/ocamlformat#1046) (Guillaume Petiot) + Fix blank line after comments at the end of lists (ocaml-ppx/ocamlformat#1045) (Guillaume Petiot) + Fix indexing operators precedence (ocaml-ppx/ocamlformat#1039) (Jules Aguillon) + Fix dropped comment after infix op (ocaml-ppx/ocamlformat#1030) (Guillaume Petiot) + No newline if the input is empty (ocaml-ppx/ocamlformat#1031) (Guillaume Petiot) + Fix unstable comments around attributes (ocaml-ppx/ocamlformat#1029) (Guillaume Petiot) + Fix extra blank line in sequence (ocaml-ppx/ocamlformat#1021) (Jules Aguillon) + Check functor arguments when computing placement of doc comments (ocaml-ppx/ocamlformat#1013) (Jules Aguillon) + Fix indentation of labelled args (ocaml-ppx/ocamlformat#1006) (Guillaume Petiot) + Fix linebreak between or-cases with comments when `break-cases=all` (ocaml-ppx/ocamlformat#1002) (Guillaume Petiot) + Fix unstable unattached doc comment in records (ocaml-ppx/ocamlformat#998) (Jules Aguillon) + Fix string literal changed (ocaml-ppx/ocamlformat#995) (Jules Aguillon) + Fix type variable (ocaml-ppx/ocamlformat#996) (Jules Aguillon) + Fix crash on extension sequence (ocaml-ppx/ocamlformat#992) (Guillaume Petiot) + Fix position of expressions regarding of comments in infix-op expressions (ocaml-ppx/ocamlformat#986) (Guillaume Petiot) + Escape special characters in external declaration (ocaml-ppx/ocamlformat#988) (Jules Aguillon) + Fix parens around constrained expr with attrs (ocaml-ppx/ocamlformat#987) (Guillaume Petiot) + Fix the margin, and correctly breaks comments (ocaml-ppx/ocamlformat#957) (Guillaume Petiot) + Fix formatting of custom indexing operators (ocaml-ppx/ocamlformat#975) (Guillaume Petiot) + Fix position of comments of labelled arrow types (ocaml-ppx/ocamlformat#976) (Guillaume Petiot) + No box around inline odoc styles (ocaml-ppx/ocamlformat#971) (Guillaume Petiot) + Fix boxing of collection expressions/patterns (ocaml-ppx/ocamlformat#960) (Guillaume Petiot) + Fix crash on record expr with pack fields (ocaml-ppx/ocamlformat#963) (Jules Aguillon) + Fix letop in subexpr (ocaml-ppx/ocamlformat#956) (hhugo) #### Internal + Take file kind from --name when formatting stdin (ocaml-ppx/ocamlformat#1119) (Jules Aguillon) + Make Fmt.t abstract (ocaml-ppx/ocamlformat#1109) (Jules Aguillon) + Future-proof Fmt API in case Fmt.t goes abstract (ocaml-ppx/ocamlformat#1106) (Etienne Millon) + Future-proof `Fmt` API in case `Fmt.t` goes abstract (ocaml-ppx/ocamlformat#1106) (Etienne Millon) + Optional names for formatting boxes in debug output (ocaml-ppx/ocamlformat#1083) (Guillaume Petiot) + Check ocamlformat error codes in the testsuite (ocaml-ppx/ocamlformat#1084) (Etienne Millon) + Clean `Translation_unit` (ocaml-ppx/ocamlformat#1078) (Guillaume Petiot) + Use dune file generation in test/passing/dune (ocaml-ppx/ocamlformat#1082) (Etienne Millon) + CI: factorize tests and check reason build (ocaml-ppx/ocamlformat#1079) (Guillaume Petiot) + Use short form for action in src/dune (ocaml-ppx/ocamlformat#1076) (Etienne Millon) + Cleanup `sequence_blank_line` (ocaml-ppx/ocamlformat#1075) (Jules Aguillon) + CI: use a script travis-ci.sh to simplify .travis.yml (ocaml-ppx/ocamlformat#1063) (Guillaume Petiot) + Remove utility functions from `Fmt_ast` (ocaml-ppx/ocamlformat#1059) (Guillaume Petiot) + CI: use opam-2.0.5 in Travis (ocaml-ppx/ocamlformat#1044) (Anton Kochkov) + CI: check the build with OCaml 4.07.1 and 4.08.0 (ocaml-ppx/ocamlformat#1036) (Jules Aguillon) + Use the same sets of options for both branches by default in `test_branch.sh` (ocaml-ppx/ocamlformat#1033) (Guillaume Petiot) + Fix `test_branch.sh` and CI checking of CHANGES.md (ocaml-ppx/ocamlformat#1032, ocaml-ppx/ocamlformat#1034) (Jules Aguillon) + Fix flag of git-worktree in `test_branch.sh` and `bisect.sh` (ocaml-ppx/ocamlformat#1027) (Guillaume Petiot) + Remove the `bisect_ppx` dependency and clean the `Makefile` (ocaml-ppx/ocamlformat#1005) (Jules Aguillon) + Use a `CHANGES.md` log file again (ocaml-ppx/ocamlformat#1023) (Guillaume Petiot) + Support OCaml 4.09.0 (add the odoc.1.4.2 dependency) (ocaml-ppx/ocamlformat#1024) (Guillaume Petiot) + Update labels of issue templates (ocaml-ppx/ocamlformat#1017) (Guillaume Petiot) + Update labels in `CONTRIBUTING.md` (ocaml-ppx/ocamlformat#1007) (Guillaume Petiot) + Allow to ignore invalid options (ocaml-ppx/ocamlformat#984) (hhugo) The `--ignore-invalid-option` flag is added to ignore invalid options in `.ocamlformat` files. + Improve the documentation of `--doc-comments` (ocaml-ppx/ocamlformat#982) (Jules Aguillon) + Remove symbolic links and change naming convention of tests (ocaml-ppx/ocamlformat#980) (Guillaume Petiot) + Change the type of `fmt_code` (ocaml-ppx/ocamlformat#974) (Guillaume Petiot) + Simplify `Makefile` (ocaml-ppx/ocamlformat#973) (hhugo) + Dune should not be flagged as a build dep anymore (ocaml-ppx/ocamlformat#954) (Guillaume Petiot)

facebook-github-bot added the CLA Signed label Mar 27, 2019

Julow force-pushed the odoc_instead_of_octavius branch from d2be771 to 0c8cec9 Compare March 27, 2019 14:45

jberdine reviewed Mar 28, 2019

View reviewed changes

jberdine mentioned this pull request Mar 30, 2019

--wrap-comment interfering with preformatted bits #741

Closed

gpetiot mentioned this pull request Apr 15, 2019

Formatting of code blocks inside documentation comments #478

Closed

Julow force-pushed the odoc_instead_of_octavius branch from 6c2dd6d to 0abbce3 Compare April 19, 2019 10:00

gpetiot reviewed Apr 19, 2019

View reviewed changes

src/Fmt_odoc.ml Outdated Show resolved Hide resolved

src/Fmt_odoc.ml Outdated Show resolved Hide resolved

src/Fmt_odoc.ml Outdated Show resolved Hide resolved

src/Fmt_odoc.ml Outdated Show resolved Hide resolved

src/Fmt_odoc.ml Outdated Show resolved Hide resolved

Julow force-pushed the odoc_instead_of_octavius branch 2 times, most recently from dc27535 to 962abcf Compare April 29, 2019 09:16

Julow mentioned this pull request May 1, 2019

Expose parser internals and add Parser.parse_comment_raw ocaml/odoc#351

Merged

Julow force-pushed the odoc_instead_of_octavius branch from e519cfc to 004f25f Compare June 18, 2019 16:46

Julow requested review from gpetiot and jberdine June 20, 2019 08:49

avsm reviewed Jun 25, 2019

View reviewed changes

ocamlformat.opam Outdated Show resolved Hide resolved

avsm reviewed Jun 25, 2019

View reviewed changes

ocamlformat_reason.opam Outdated Show resolved Hide resolved

Julow force-pushed the odoc_instead_of_octavius branch from d75093e to c9d73e5 Compare June 25, 2019 11:39

gpetiot mentioned this pull request Jun 26, 2019

Wishlist for the 1.0 release #900

Closed

12 tasks

gpetiot approved these changes Jun 27, 2019

View reviewed changes

gpetiot reviewed Jun 27, 2019

View reviewed changes

src/Fmt_ast.ml Outdated Show resolved Hide resolved

Julow added 17 commits July 30, 2019 15:16

Upgrade to Odoc 1.4.1

d8d0734

Fix warning 9

c99ef00

fits_breaks changed on master

c7fb2c6

Indent code blocks

60ba5ed

Clean up

e08279f

Clean up: Don't shadow Fmt.str

c0ac9b8

Show parsing errors only if parse-docstrings is enable

14ef3b9

Add tests for tags without text

edeead6

Remove unecessary space after tags with empty text

0e4ea3b

Fix printing of @see 'file' and @see "title" tags

139f213

Fix doc-comments-tag-only=fit with parsed docstring

f5b5f5a

Promote tests

742b225

Remove trailing spaces in empty lines in code blocks

70a8c99

Fix incorrect location on the first line of doc comments

78d55f0

Add failing tests for codeblocks indentation

07902b7

Requires a fix to Odoc

Add tests

e622d9a

Normalize whitespaces inside code spans

53b7796

Julow force-pushed the odoc_instead_of_octavius branch from 179e0a5 to 53b7796 Compare July 30, 2019 13:16

gpetiot reviewed Jul 31, 2019

View reviewed changes

src/Fmt_odoc.ml Outdated Show resolved Hide resolved

Julow added 2 commits August 1, 2019 15:02

Allow consecutive whitespaces in code spans

ee9d0d5

Revert consecutive whitespaces in code spans and only escape brackets

82b1328

gpetiot merged commit 12e2d15 into ocaml-ppx:master Aug 5, 2019

gpetiot mentioned this pull request Aug 5, 2019

Use the Odoc parser instead of Octavius #571

Closed

Julow mentioned this pull request Nov 4, 2019

[new release] ocamlformat (0.12) ocaml/opam-repository#15208

Merged

jonludlam mentioned this pull request Feb 25, 2020

Convert doc comment to markdown using omd and octavius ocaml/ocaml-lsp#104

Closed

bogdan2412 pushed a commit to bogdan2412/ocamlformat that referenced this pull request Mar 28, 2020

Improve: parse doc comments with Odoc (ocaml-ppx#721)

bac056b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve: parse doc comments with Odoc #721

Improve: parse doc comments with Odoc #721

Julow commented Mar 27, 2019

hhugo commented Apr 1, 2019

Julow commented Apr 19, 2019

gpetiot left a comment

Julow commented Jun 18, 2019

Julow commented Jun 25, 2019

gpetiot commented Jun 27, 2019 •

edited

Loading

gpetiot left a comment •

edited

Loading

Julow commented Jun 27, 2019

gpetiot commented Jun 27, 2019

Julow commented Jun 27, 2019

gpetiot commented Jun 28, 2019

Julow commented Jun 28, 2019

gpetiot commented Jun 28, 2019 •

edited

Loading

Improve: parse doc comments with Odoc #721

Improve: parse doc comments with Odoc #721

Conversation

Julow commented Mar 27, 2019

hhugo commented Apr 1, 2019

Julow commented Apr 19, 2019

gpetiot left a comment

Choose a reason for hiding this comment

Julow commented Jun 18, 2019

Julow commented Jun 25, 2019

gpetiot commented Jun 27, 2019 • edited Loading

gpetiot left a comment • edited Loading

Choose a reason for hiding this comment

Julow commented Jun 27, 2019

gpetiot commented Jun 27, 2019

Julow commented Jun 27, 2019

gpetiot commented Jun 28, 2019

Julow commented Jun 28, 2019

gpetiot commented Jun 28, 2019 • edited Loading

gpetiot commented Jun 27, 2019 •

edited

Loading

gpetiot left a comment •

edited

Loading

gpetiot commented Jun 28, 2019 •

edited

Loading