|Anonymous | Login | Signup for a new account||2015-11-25 17:18 CET|
|Main | My View | View Issues | Change Log | Roadmap|
|View Issue Details|
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0006274||OCaml||OCamldoc||public||2013-12-16 19:28||2014-05-22 13:24|
|Target Version||Fixed in Version|
|Summary||0006274: Better handling of objects type definition|
|Description||Definition of object types, while syntactically similar to record types, are not "handled" by ocamldoc, i.e. the documentation one associates to fields does not appear in ocamldoc output.|
It would be nice if it did.
I have written a few patches to implement such a feature, they are available for review at https://github.com/trefis/ocaml/tree/ocamldoc [^] .
I'm not sure how much this is related to 0004347 .
I probably have a naïve approach here, however it does seem to work without breaking anything and I am not yet "deeply sad to the point of not using ocamldoc at all" (to quote jm).
You will notice that I introduce some code duplication with the handling of records (although it might be hard to notice, considering how much code duplication there already is) which should be easily factorisable.
However I wanted to gauge people's interest before investing more time on that; and it can always be cleaned afterward.
|Additional Information||The patches are currently on the 4.01.0 branch, but I don't think rebasing them to trunk would pose any problem.|
I just did a review of the patch (medium precision, I would say). It consists of three commits:
1. https://github.com/trefis/ocaml/commit/759a72a479e78c980425c5ce753ecfa048be5339 [^] collects comments occurring after method fields in a definition of an object type
2. https://github.com/trefis/ocaml/commit/68fc982c9985636ed212c22699fa4a6bea8be7f0 [^] collects method fields of object type definitions in the Odoc_type.t_type structure, and then pretty-prints them (in various backends) with more structure than just outputting the source text, rather like record definitions.
3. https://github.com/trefis/ocaml/commit/501f15c017c573b0978f8771e78e4157623f7090 [^] fixes a bug with the implementation approach of commit (2)
I think (commit 1) is reasonable, but I am less convinced by the overall design of (commit 2): a new "type kind" is added to the type Odoc_type.type_kind, which previously precisely mirrored the type_kind of the typedtree (type synonym, variant or record), to store the information needed by the backends.
I think it is wrong to break the correspondence between the typedtree "type kind" and ocamldoc's "type kind" (for example ocamldoc uses type_kind to decide whether to print parameters variance information, and I suspect this is broken by this change). A better design, I think, would be to add the idea that, besides the type_kind, useful printing information may come from the type manifest (the type-expression after the equal sign, if any), and have a function like
val manifest_structure : Types.type_expr -> Odoc_type.ty_manifest option
where ty_manifest would be a new type storing information for the type expressions we decide to print in a special way when they occur on the right of an equal sign -- (only) objects types for now. At printing points you could check both the kind and the manifest, and decide on both (eg. giving priority to the ty_manifest when it exists).
I think most of the current implementation could be reused, the idea is only to preserve the tight correspondence between typedtree's and ocamldoc's type_kind.
Other than that the patch looks sensible, and I don't see any reason *not* to include this extra feature.
Thank you for your comments.
I tried to follow your proposition:
I first reverted the two previous commits, i.e. "2" and "3" in your comment, and then introduced [Odoc_type.type_manifest] as suggested (all this could be squashed before being pushed to the svn if you want a cleaner history).
I tried to make this commit as little intrusive as I could, so there is a lot of code redundancy which I then remove in the two next commits.
The code is available at the same url as before.
edited on: 2014-01-10 12:15
Quick update: gasche commented the previous patches directly on github.
I have since added one last patch to answer his comments.
Everything should be "OK" now.
PS: Thanks to "Ditoran2" for the random pokemon "maps". I hadn't realized mantis could be a suitable replacement to megaupload.
I applied the patched in version/4.02 (rev 19405 and 19406).
@trefis: can you give it a try ?
|2013-12-16 19:28||trefis||New Issue|
|2013-12-16 19:28||trefis||Status||new => assigned|
|2013-12-16 19:28||trefis||Assigned To||=> guesdon|
|2013-12-24 15:09||gasche||Note Added: 0010766|
|2013-12-25 02:18||Ditoran2||File Added: maps.psd|
|2013-12-25 15:15||Ditoran2||File Added: Source.a51|
|2013-12-27 20:42||trefis||Note Added: 0010768|
|2014-01-10 12:12||trefis||Note Added: 0010792|
|2014-01-10 12:15||trefis||Note Edited: 0010792||View Revisions|
|2014-02-19 17:15||doligez||Tag Attached: patch|
|2014-05-22 10:40||gasche||File Deleted: maps.psd|
|2014-05-22 10:40||gasche||File Deleted: Source.a51|
|2014-05-22 13:23||guesdon||Note Added: 0011548|
|2014-05-22 13:24||guesdon||Status||assigned => resolved|
|Copyright © 2000 - 2011 MantisBT Group|