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

Manual examples #2008

Merged
merged 3 commits into from Sep 6, 2018

Conversation

Projects
None yet
3 participants
@gasche
Copy link
Member

commented Aug 27, 2018

This is a small followup to #1209, completing the Extension-migration work.

cc @Octachron

type fragile =
| Int of int [@warn_on_literal_pattern]
| String of string [@warn_on_literal_pattern]

let f = function
| Int 0 | String "constant" -> () (* trigger warning 52 *)
| Int 0 | String "constant" -> ()

This comment has been minimized.

Copy link
@pmetzger

pmetzger Aug 27, 2018

Member

I don't quite get why the comment was removed?

This comment has been minimized.

Copy link
@Octachron

Octachron Aug 27, 2018

Contributor

Because the warning itself is printed by the example preprocessor at the end of the example. Moreover, the option warning=52 tells to the preprocessor that a warning 52 ought to be emitted, otherwise there was an error in the preprocessing.

This comment has been minimized.

Copy link
@pmetzger

pmetzger Aug 28, 2018

Member

Thanks for the clear explanation.

@Octachron
Copy link
Contributor

left a comment

The PR looks good to me except two minor remarks.


let x = begin[@warning "+9"] ... end in ....
let x = begin[@warning "+9"] ()[@ellipsis] end

This comment has been minimized.

Copy link
@Octachron

Octachron Sep 1, 2018

Contributor

There is a small technical issue here: begin[@warning "+9"] ()[@ellipsis] end is syntactically equivalent to ()[@warning "+9"][@ellipsis], thus everything get elided. A possible fix is to replace ()[@ellipsis] by [()[@ellipsis]] .

type fragile =
| Int of int [@warn_on_literal_pattern]
| String of string [@warn_on_literal_pattern]

let f = function
| Int 0 | String "constant" -> () (* trigger warning 52 *)
| Int 0 | String "constant" -> ()

This comment has been minimized.

Copy link
@Octachron

Octachron Sep 1, 2018

Contributor

I would split the example in two with an Int and a String case: currently, the function f raises two identical warnings 52 due to the lack of warning locations. I fear that potential reader would miss the fact that two patterns are underlined. Splitting f into f and g seems clearer.

@gasche gasche force-pushed the gasche:manual-examples branch from 11ddceb to 062e203 Sep 6, 2018

@gasche

This comment has been minimized.

Copy link
Member Author

commented Sep 6, 2018

Thanks @Octachron for the feedback, I changed the examples accordingly and rebased.

gasche added some commits Aug 27, 2018

caml_tex: also provide error output on Lexing errors
I hit a Lexer error by mistakenly closing an environment with

    \end{caml_example*}{verbatim}

instead of

    \end{caml_example*}

and caml_tex would not quote the wrong input or indicate
at which line the error was, which makes debugging painful.

@Octachron Octachron merged commit 0a05dc8 into ocaml:trunk Sep 6, 2018

1 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
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.