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

documentation: precedence table for the standard library. #2117

Merged
merged 12 commits into from Nov 6, 2018

Conversation

Projects
None yet
5 participants
@Octachron
Copy link
Contributor

commented Oct 25, 2018

Following the disucssion in #1167, this PR proposes to add a copy of the manual's precedence table inside the standard library documentation page. This makes possible to link easily the description of each operators to this precedence table, replacing the quite arbitrary ordinal precedence level used before.

To add a bit more context, each operator is also linked to the representative of its precedence class and the representative of the 2-neareast precedence classes.

Unfortunately, ocamldoc does not support table by itself. Thus the table is implemented separately for the latex, html and manpage backends. Since the texinfo backend seems faulty for the standard library, I did not include a texinfo version of the table.

The last commit is a small fix that document the fact that #... are left-associative in the manual .

@pmetzger

This comment has been minimized.

Copy link
Member

commented Oct 25, 2018

This is a fine thing. It would be nice in theory if the texinfo stuff could be fixed, but I'll admit that even I use it so rarely that I probably wouldn't notice.

@gasche

This comment has been minimized.

Copy link
Member

commented Oct 25, 2018

Thanks! Do you have an HTML rendering of the new version?

@@ -1223,6 +1264,104 @@ val at_exit : (unit -> unit) -> unit
The functions are called in 'last in, first out' order: the
function most recently added with [at_exit] is called first. *)

(** {1:precedence Appendix: precedence level and associativity of operators }

This comment has been minimized.

Copy link
@gasche

gasche Oct 25, 2018

Member

Maybe have an implementation comment here indicating to remember to update the other table (in the manual), and also the other way around?

This comment has been minimized.

Copy link
@Octachron

Octachron Oct 26, 2018

Author Contributor

I added the two synchronization comments.

@pmetzger

This comment has been minimized.

Copy link
Member

commented Oct 25, 2018

Thanks! Do you have an HTML rendering of the new version?

Built it, yes. I should mention I noticed a couple of oddities (the hyperlinks for (&&) for example come out a different color than the others, and I have no idea why.) Also, in the table itself, you should say "A few other syntactic constructions" rather than "Few other syntactic constructions". Mostly it seems good. I'm a bit distracted right now so I can't do a tight proofread though.

@Octachron

This comment has been minimized.

Copy link
Contributor Author

commented Oct 26, 2018

@gasche , here is the new html rendering .

@pmetzger , typo fixed, thanks! Does all (&&)s render in a different color?

@gasche

This comment has been minimized.

Copy link
Member

commented Oct 26, 2018

It looks weird to have the list of submodules without transition, as if they were part of the appendix. I suppose it was already weird before the appendix was added. Can it be placed before, with a title/section name? Or should there at least be a section for it after the appendix?

@Octachron

This comment has been minimized.

Copy link
Contributor Author

commented Oct 26, 2018

Good point. I moved the appendix to the end of the documentation and added a section for the list of modules of the standard library.

@gasche

gasche approved these changes Oct 26, 2018

Copy link
Member

left a comment

I like the result. Given how easy it is to jump to the table, at least in the HTML version, I'm not completely sure that we really need to recall the levels in the text itself (we could just say (see the {!reference} for associativity and precedence)), but I'm content with the current state.

@dbuenzli

This comment has been minimized.

Copy link
Contributor

commented Oct 26, 2018

I'm not completely sure that we really need to recall the levels in the text itself (we could just say (see the {!reference} for associativity and precedence))

Except for associativity which concerns the operator itself and not its relationship to others, I would tend to agree with @gasche.

I think precedence relationships among operators is better understood by the table than with the text and the idea of classifying by domain doesn't work that well since for example @@ and |> concern all domains.

Also repeatedly seeing the whole table might help people (at least visual ones) to eventually get it in their brain (personally I'm a lost cause).

@Octachron

This comment has been minimized.

Copy link
Contributor Author

commented Oct 29, 2018

I agree that the textual description of precedence is not really useful for the html version, I am less sure for the manpage version where it is not possible to follow the link to the table.

@pmetzger

This comment has been minimized.

Copy link
Member

commented Oct 29, 2018

I am less sure for the manpage version where it is not possible to follow the link to the table.

One option is to have a man page specifically for precedence. Some man page readers (including the one I use in emacs) let you click from one man page to another when there are cross references.

@Octachron

This comment has been minimized.

Copy link
Contributor Author

commented Oct 29, 2018

Indeed, it seems that lifting the table to its own page might be the more portable choice.

@gasche

This comment has been minimized.

Copy link
Member

commented Oct 29, 2018

I'm not sure what are valid manpage names, but one that comes to mind is ocaml-operator-grammar.

@pmetzger

This comment has been minimized.

Copy link
Member

commented Oct 29, 2018

Most Unix systems have an operator(7) man page with C operators and their precedence. An ocaml-operator page might be fine.

@Octachron Octachron force-pushed the Octachron:stdlib_precedence_table_3 branch from bd071b0 to b8ed5af Nov 4, 2018

@Octachron

This comment has been minimized.

Copy link
Contributor Author

commented Nov 4, 2018

I have split the operator table to its own documentation-only module ocaml_operators.mld inside the stdlib directory and replaced the above ... and below ... indication by a simpler reference

see {!Ocaml_operators} for more information

@@ -126,6 +126,8 @@ The table below shows the relative precedences and associativity of
operators and non-closed constructions. The constructions with higher
precedence come first. For infix and prefix symbols, we write
``"*"\ldots'' to mean ``any symbol starting with "*"''.
% Note that this table is duplicated in stdlib/stdlib.mli, these tables should

This comment has been minimized.

Copy link
@gasche

gasche Nov 4, 2018

Member

This should be updated, right?

This comment has been minimized.

Copy link
@Octachron

Octachron Nov 4, 2018

Author Contributor

Indeed.

@gasche

This comment has been minimized.

Copy link
Member

commented Nov 4, 2018

I don't remember everything; is the PR mergeable in the current state (modulo minor fix discussed above)?

@Octachron Octachron force-pushed the Octachron:stdlib_precedence_table_3 branch from b8ed5af to c951d3c Nov 4, 2018

@Octachron

This comment has been minimized.

Copy link
Contributor Author

commented Nov 4, 2018

Some check-typo fiddling was missing, but this should be fixed in the last commit.
Otherwise, if the location of the mld file and the reference message are fine, the PR should be mergeable.

@gasche

This comment has been minimized.

Copy link
Member

commented Nov 4, 2018

Well then I would suggest that you merge when you are satisfied with the results / typo checks / CI results :-) (I already approved.)

@Octachron

This comment has been minimized.

Copy link
Contributor Author

commented Nov 4, 2018

I will merge tomorrow to let some time for potential reaction.

@Octachron

This comment has been minimized.

Copy link
Contributor Author

commented Nov 6, 2018

I have fixed some missing reference in the pdf manual, and some missing cleanup; I will merge once CIs pass.

@Octachron Octachron merged commit 9f01a08 into ocaml:trunk Nov 6, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.