Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PR#7478: ocamldoc, avoid module preamble repetition #1037

Merged
merged 3 commits into from
Aug 9, 2017

Conversation

Octachron
Copy link
Member

MPR#7478:

Currently, ocamldoc uses the first documentation comment in a source file as a module preamble. This has the unfortunate consequence that ocamldoc can use documentation comments far from the start of the file as a preamble. Moreover, if this first documentation comment occurs after any module element, this documentation comment will appear twice in the generated documentation: first as a module preamble, then at its original position in the source file:

open A

(** Here, this comment will be used both as a module preamble and a documentation comment for [x] *)
val x :unit

This PR proposes to fix this duplication of documentation comments by using as module preamble, only documentation comments that appear before any module elements:

(** This comment is a module preamble *) 
open A
open A

(** This comment is not a module preamble any more with this PR *)
val x :unit

@Octachron Octachron changed the title PR#7478: ocamldoc, avoid module preambule repetion PR#7478: ocamldoc, avoid module preamble repetition Feb 10, 2017
Changes Outdated
- PR#7478, GPR#1037: ocamldoc, do not use as a module preamble documentation
comments that occur after the first module element
(Florian Angeletti, report by Daniel Bünzli)

Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this qualify as a breaking change (even if a good one)?

Copy link
Member Author

Choose a reason for hiding this comment

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

It does not seem completely clear-cut to me since documentations affected by the change were already somehow broken. Nevertheless, erring on the side of caution and adding a warning about open statements is probably better.

| a :: _ when a.Parsetree.psig_loc.Location.loc_start.Lexing.pos_cnum <
fst info ->
(0,None)
| _ -> info in
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to refactor this to remove the small code duplication with odoc_ast.ml?

Copy link
Member Author

Choose a reason for hiding this comment

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

Possible, certainly. Optimal, I am not so sure, see d7269c0 .

To avoid module preamble repetition, ocamldoc only use as a module
preamble documentation comments that occur before any module elements,
which should also prevent some unexpected module preamble when the first
documentation comments appears in the middle of the source file.
@Octachron Octachron force-pushed the ocamldoc_preamble_duplication branch from 92ff0fc to 64d9cee Compare July 31, 2017 21:10
@Octachron Octachron force-pushed the ocamldoc_preamble_duplication branch from 64d9cee to 3bf7f9f Compare July 31, 2017 21:53
Copy link
Member

@dra27 dra27 left a comment

Choose a reason for hiding this comment

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

Previous review comments addressed - all looks good to me

@gasche
Copy link
Member

gasche commented Aug 9, 2017

Nice work, thanks!

@gasche gasche merged commit 110b1c2 into ocaml:trunk Aug 9, 2017
stedolan pushed a commit to stedolan/ocaml that referenced this pull request Mar 21, 2023
stedolan pushed a commit to stedolan/ocaml that referenced this pull request Mar 21, 2023
Revert "Use Import_info.t in Cmt_format (ocaml#1037)"

This reverts commit 2798499.
EmileTrotignon pushed a commit to EmileTrotignon/ocaml that referenced this pull request Jan 12, 2024
Co-authored-by: Cuihtlauac ALVARADO <cuihtmlauac@tarides.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants