use the `caml_example` environment in the Extensions manual by gasche · Pull Request #1209 · ocaml/ocaml

gasche · 2017-06-21T21:21:42Z

This PR, which continues work started in #939 as tracked in MPR#7551, has one well-defined commit that I believe is correct, but I marked it work-in-progress as:

the whole section of the manual is not covered
the parts that I did uncovered some minor details with the {caml_example}{verbatim} environment (from MPR#7551: manual, make final ";;" potentially optional in caml_example #1194) that we may want to discuss and potentially fix before going further

To that end I uploaded a temporary rendering of the PR manual.

If you look at the Recursive modules section for example, you may notice that

The # at the beginning of the code example does not look nice (we are supposed not to be in the toplevel)
The spacing at the end of the code is too big. See also the examples in private row types.
It would be sort of nice to have a way to show signature item examples that are syntax- and type-checked, but there is no urgency.

I think that it would be useful to think (cc @Octachron !) about fixing (1) and (2) before considering merging this or going further with the caml_example-ization of this section.

Octachron · 2017-06-21T21:54:21Z

For the first point, on the html side the # is implemented with a css rule. I would say the best way to remove it would be to add a verbatim (resp. toplevel) class to the caml-example div. This would allow to customize the style of both variant independently. On the latex side, removing the line of # on the first column might require some macro trickery. Note that the rendering of the output might benefit from some tuning too.

On the second point, the generated html code looks fine, this is probably a matter of tuning the margin/padding in the manual css style.

For 3, I think that wrapping the example in a module type definition before sending it to the toplevel might do the trick.

Octachron · 2017-06-21T22:03:09Z

manual/manual/refman/exten.etex

+             let a = A
+             let b n = assert (n > 0); B n
+           end
+\end{caml_example}


May I suggest to add a star here \end{caml_example*} to not betray the illusion that caml_example is a latex environment? (pure nitpicking, I wholeheartedly admit)

Octachron · 2017-06-21T22:04:03Z

manual/manual/refman/exten.etex

-\end{verbatim}
+\begin{caml_example*}{verbatim}
+ type t = [ `A of int | `B of bool ]
+\end{caml_example}


Another lost star above.

(Thanks! Fixed both.)

gasche · 2017-06-23T15:47:14Z

I dislike LaTeX programming, so I don't plan to work on it. On the other hand I'll try to propose a patch for the html/css part along the lines proposed. (I'm interested in feedback on what future we want for this PR. Should we wait for the display issues to be fixed before considering a merge? Should we complete the porting of the chapter? (I would at least support the second option.))

Octachron · 2017-06-23T20:55:28Z

Completing the porting of the current section seems like a good idea: it increases the number of examples available for tuning the rendering afterwards. Moreover, since the fix for the rendering should be mostly independent from the manual changes, I don't see a lot of negatives with merging this PR before the rendering the fix. ((In term of negatives, I imagine that the rendering changes might require some micro-tuning of the manual examples)).

xavierleroy · 2017-09-14T18:13:31Z

Three months later, where are we on this GPR?

gasche · 2017-09-14T20:44:08Z

I haven't worked on it since, and I would rather not merge the change before the rendering gets improved. On the other hand, it's a low-conflict PR so it can stay open for longer. Thanks for the WIP tag, it's appropriate.

Octachron · 2018-05-04T13:12:54Z

@gasche , is there any obstacle (except the current conflict) remaining before merging?

gasche · 2018-05-04T13:14:49Z

I don't remember. I will fix the conflicts and rebase, and then maybe you could think about it, and if applicable accept and merge?

gasche · 2018-05-05T13:47:05Z

I'm looking at it now and it turns out that #1702 would be useful to go further on this PR.

Octachron · 2018-05-05T14:20:00Z

I have rebased #1702 (and moved the change entry to trunk).

gasche · 2018-05-05T21:59:39Z

I have rebased this PR, and added quite a few uses of caml_example. The remaining verbatim environments in exten.etex are for the cases where I don't know how to use caml_example instead, typically code that uses ... for ellipsis -- and thus wouldn't compile.

gasche · 2018-05-06T06:20:22Z

Re dots.: a lot of the manual examples look like

let f (type a) (x : a -> a) = ...

@Octachron, as the manual/tools wizard, do you know if it would be possible to render this out of something like

let f (type a) (x : a -> a) = (assert false)[@ellipsis]

?

Octachron · 2018-05-06T10:05:52Z

It sounds possible. However, in order to have a robust behavior with attributes, the best choice is probably to parse each phrase and extract the location of each ellipsis from the parse tree.

gasche · 2018-05-06T10:24:54Z

This is what is done for expect-tests in testsuite/tools/expect_test.ml.

While we are at it, another random feature request inspired by this PR: if I pass [error] or [warning=...], could I get the error or warning shown even in the {caml_example*} case? On my machine what I observe in the warning case is that the warning position is highlighted (a very nice feature in general), but the warning message is not shown.

Edit (August 2018): the question above has been fixed by #1863.

gasche · 2018-05-07T14:25:11Z

I just rebased this against #1765.

gasche · 2018-05-07T14:28:40Z

manual/manual/refman/exten.etex

+let _ = Hashtbl.add devices "SVG" (module SVG : DEVICE)
+
+module PDF = struct let draw () = ()[@@ellipsis] end
+let _ = Hashtbl.add devices "PDF" (module PDF: DEVICE)


@Octachron I think that you may find this block of code interesting.

The first ellipsis is used in place of a separate {caml_eval} block as I had in my previous version (before the last rebase), and it suggests that maybe longer-term we could think of using [@hide] attributes that are completely removed instead of a separate environment.

The second ellipsis has this form, instead of a signature-item attribute, because of the issue I reported in https://github.com/ocaml/ocaml/pull/1765/files#r186431706. I am trying to come with a fix for the issue, but maybe we could consider trying to get this PR merged before that -- it's been long enough in the baking.

I agree that it would be nice to merge soon. I will do a full review by tomorrow.

that maybe longer-term we could think of using [@hide] attributes that are completely removed instead of a separate environment.

This is definitively doable. At the same time, I fear that mixing hidden and non-hidden code fragment too freely might be confusing for the reader.

Thinking more about it, I think we should wait until we agree on what to do with #1765 before merging.

gasche · 2018-08-19T19:24:00Z

I just rebased this PR on top of #1903, and the ellipsis machinery now works correctly in all examples where it is used.

@Octachron note that I added a patch, currently 48ea928, to simplify the location-handling in the ellipsis code, now that attributes carry their whole location.

Octachron

I have three small remarks but overall the examples look nice.

Octachron · 2018-08-26T20:04:14Z

manual/manual/refman/exten.etex

 let scale l = function
  | Point p -> Point {p with x = l *. p.x; y = l *. p.y}
-  | ....
+  | Other -> Other


Was this change by choice or due to the lack of attribute on case? If it is the second, using
| Other[@ellipsis.start] -> Other[@ellipsis stop] would work after a small fix that I can send as a sub-PR.

In that case I think that showing the full code is just as well. In your sub-PR, can you include a test somewhere? I should also add a regression test for val picture : picture -> unit[@@ellipsis] which I had a lot of trouble not to get rendered as val picture : picture -> ....

I think I will make a test PR for ellipsis attributes in general.

Octachron · 2018-08-26T20:10:56Z

manual/manual/refman/exten.etex

-        module type DEVICE = sig [@@@ellipsis] end
-        let devices : (string, (module DEVICE)) Hashtbl.t = Hashtbl.create 17
+type picture = unit[@ellipsis]
+module type DEVICE = sig val draw : picture -> unit [@@ellipsis] end


I would propose to have a separate ellipsis [@@@ellipsis] to have

module type DEVICe = sig val draw:picture -> unit ... end

rather than just sig ... end since it gives a clearer purpose to picture.

Octachron · 2018-08-26T20:23:13Z

tools/Makefile

 caml-tex: $(CAMLTEX)
 	../runtime/ocamlrun ../ocamlc -nostdlib -I ../stdlib $(LINKFLAGS) \
-	-linkall -o $@ $(CAMLTEX)
+	-linkall -o $@ -no-alias-deps $(CAMLTEX)


I am curious, what is the story behind this -no-alias-deps?

If you remove it, the build fails because Parsetree (to which an alias is introduced in #1903) has no .cmo to link against. This sounds strange, but module M = Foo is broken on all OCaml versions if Foo is mli-only and you don't use no-alias-deps.

gasche · 2018-08-27T05:34:52Z

I rebased on top of trunk, implementing @Octachron's recommendation (to show val draw in the DEVICE signature).

I wanted to test a caml_tex issue by creating a smaller .etex file than exten.etex, and got bitten by the fact that the Makefile hardcode the knowledge that only exten.etex uses caml_example.

…ctions

Co-authored-by: Cuihtlauac ALVARADO <cuihtmlauac@tarides.com>

Octachron reviewed Jun 21, 2017

View reviewed changes

gasche force-pushed the manual-examples branch from 5a50b67 to b87edfd Compare June 21, 2017 22:09

xavierleroy added the work-in-progress label Sep 14, 2017

damiendoligez added this to the 4.07-or-later milestone Sep 25, 2017

Octachron mentioned this pull request Dec 20, 2017

Manual: decouple verbatim and toplevel styles in code examples #1540

Merged

gasche mentioned this pull request May 5, 2018

manual: signature option for caml_example #1702

Merged

gasche force-pushed the manual-examples branch from b87edfd to 46a5f75 Compare May 5, 2018 21:57

gasche changed the title ~~[wip] use the new caml_example environment in the Extensions manual~~ use the caml_example environment in the Extensions manual May 5, 2018

gasche force-pushed the manual-examples branch from 46a5f75 to 132ef5b Compare May 5, 2018 22:01

Octachron mentioned this pull request May 6, 2018

manual: ellipses in examples #1765

Merged

gasche force-pushed the manual-examples branch from 132ef5b to 6195a92 Compare May 7, 2018 14:24

gasche commented May 7, 2018

View reviewed changes

gasche force-pushed the manual-examples branch from 6195a92 to 9cb4c0a Compare May 7, 2018 14:30

damiendoligez removed this from the consider-for-4.07 milestone May 29, 2018

gasche force-pushed the manual-examples branch from 9cb4c0a to 64a0cf4 Compare August 19, 2018 19:19

Octachron approved these changes Aug 26, 2018

View reviewed changes

gasche force-pushed the manual-examples branch from 64a0cf4 to 0c5e51f Compare August 27, 2018 05:34

gasche removed the work-in-progress label Aug 27, 2018

gasche force-pushed the manual-examples branch from 0c5e51f to acc2c70 Compare August 27, 2018 05:36

gasche added 2 commits August 27, 2018 07:42

[minor] manual: simplify the refman/ build system

fffd676

I wanted to test a caml_tex issue by creating a smaller .etex file than exten.etex, and got bitten by the fact that the Makefile hardcode the knowledge that only exten.etex uses caml_example.

caml_etex: use the location of the attribute, now available

d5af6d9

gasche force-pushed the manual-examples branch from acc2c70 to 3e588dc Compare August 27, 2018 06:57

gasche merged commit e687609 into ocaml:trunk Aug 27, 2018

manual: use the caml_example environments in some of the Extension se…

3e588dc

…ctions

gasche mentioned this pull request Aug 27, 2018

Manual examples #2008

Merged

vicuna mentioned this pull request Mar 14, 2019

caml_example pseudo-environment in the language extension chapter #7551

Closed

EmileTrotignon pushed a commit to EmileTrotignon/ocaml that referenced this pull request Jan 12, 2024

Add Signal and Threads RSS feed (ocaml#1209)

6198e4b

Co-authored-by: Cuihtlauac ALVARADO <cuihtmlauac@tarides.com>

Conversation

gasche commented Jun 21, 2017

Uh oh!

Octachron commented Jun 21, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gasche commented Jun 23, 2017

Uh oh!

Octachron commented Jun 23, 2017

Uh oh!

xavierleroy commented Sep 14, 2017

Uh oh!

gasche commented Sep 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Octachron commented May 4, 2018

Uh oh!

gasche commented May 4, 2018

Uh oh!

gasche commented May 5, 2018

Uh oh!

Octachron commented May 5, 2018

Uh oh!

gasche commented May 5, 2018

Uh oh!

gasche commented May 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Octachron commented May 6, 2018

Uh oh!

gasche commented May 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gasche commented May 7, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gasche commented Aug 19, 2018

Uh oh!

Octachron left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gasche commented Aug 27, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gasche commented Sep 14, 2017 •

edited

Loading

gasche commented May 6, 2018 •

edited

Loading

gasche commented May 6, 2018 •

edited

Loading

Octachron left a comment •

edited

Loading