-
Notifications
You must be signed in to change notification settings - Fork 46
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
Improve error reporting of ocaml-mdx test #172
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't taken the time to review all this but I think I'd prefer we start using the result
type rather than exceptions.
3dcc179
to
5569715
Compare
Rebased. |
Can you rebase now that #171 has been merged please? |
ada48e0
to
637ef93
Compare
Rebased and cleaned. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks good!
Let's fix the error message a bit and merge!
bin/test/main.ml
Outdated
prerr_endline f; | ||
1 | ||
| Test_block_failure (block, msg) -> | ||
Fmt.epr "Error in block at line %d in %s:@\n%s\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the error message could be slightly improved here by:
- Using
code block
instead of block, I think it sounds a bit vague - Adding the kind of block, OCaml, OCaml toplevel or shell
- printing locations the other way around, file first, line second
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed it. What do you think?
Can you add a changelog entry as well? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thank you! It's a great change!
Can you fix the conflict and I'll merge?
Rebased. Also cleaned the history a bit. |
CHANGES: #### Added - Add `--syntax` option to `rule` subcommand to allow generating rules for cram tests (realworldocaml/mdx#177, @craigfe) - Add a `require-package` label to explicitly declare dune `package` dependencies of a code block (realworldocaml/mdx#149, @Julow) - Add an `unset-` label to unset env variables in shell blocks (realworldocaml/mdx#132, @clecat) #### Changed - Run promotion of markdown files before `.ml` files in generated dune rules (realworldocaml/mdx#140, @clecat) #### Fixed - Remove trailing whitespaces at the end of toplevel or bash evaluation lines (realworldocaml/mdx#166, @clecat) - Improve error reporting of ocaml-mdx test (realworldocaml/mdx#172, @Julow) - Rule: Pass the --section option to `test` (realworldocaml/mdx#176, @Julow) - Remove trailing whitespaces from shell outputs and toplevel evals (realworldocaml/mdx#166, @clecat) - Remove inappropriate empty lines in generated dune rules (realworldocaml/mdx#163, @Julow) - Fix ignored `skip` label in `ocaml-mdx pp` (realworldocaml/mdx#1561, @craigfe) - Fix synchronization of new parts from markdown to `.ml` (realworldocaml/mdx#156, @Julow) - Fix ignored `[@@@Parts ...]` markers within module definitions (realworldocaml/mdx#155, @Julow) - Fix a bug in internal OCaml version comparison that lead to crashes in some cases (realworldocaml/mdx#145, @gpetiot) - Promote to empty `.ml` file when using `to-ml` direction (realworldocaml/mdx#139, @clecat) - Apply `--force-output` to `.ml` file as well (realworldocaml/mdx#137, @clecat) - Fix a bug preventing `.corrected` files to be written in some cases (realworldocaml/mdx#136, @clecat) - Add compatibility with `4.09.0` (realworldocaml/mdx#133, @xclerc) #### Removed - Remove the `infer-timestamp` direction (realworldocaml/mdx#171 @Julow)
CHANGES: #### Added - Add `--syntax` option to `rule` subcommand to allow generating rules for cram tests (realworldocaml/mdx#177, @craigfe) - Add a `require-package` label to explicitly declare dune `package` dependencies of a code block (realworldocaml/mdx#149, @Julow) - Add an `unset-` label to unset env variables in shell blocks (realworldocaml/mdx#132, @clecat) #### Changed - Format rules generated by `ocaml-mdx rule` using `dune format-dune-file` (realworldocaml/mdx#184, @NathanReb) - Run promotion of markdown files before `.ml` files in generated dune rules (realworldocaml/mdx#140, @clecat) #### Fixed - Use module_presence information on Env.summary to prevent fetching absent modules from the toplevel env (realworldocaml/mdx#186, @clecat) - Remove trailing whitespaces at the end of toplevel or bash evaluation lines (realworldocaml/mdx#166, @clecat) - Improve error reporting of ocaml-mdx test (realworldocaml/mdx#172, @Julow) - Rule: Pass the --section option to `test` (realworldocaml/mdx#176, @Julow) - Remove trailing whitespaces from shell outputs and toplevel evals (realworldocaml/mdx#166, @clecat) - Remove inappropriate empty lines in generated dune rules (realworldocaml/mdx#163, @Julow) - Fix ignored `skip` label in `ocaml-mdx pp` (realworldocaml/mdx#1561, @craigfe) - Fix synchronization of new parts from markdown to `.ml` (realworldocaml/mdx#156, @Julow) - Fix ignored `[@@@Parts ...]` markers within module definitions (realworldocaml/mdx#155, @Julow) - Fix a bug in internal OCaml version comparison that lead to crashes in some cases (realworldocaml/mdx#145, @gpetiot) - Promote to empty `.ml` file when using `to-ml` direction (realworldocaml/mdx#139, @clecat) - Apply `--force-output` to `.ml` file as well (realworldocaml/mdx#137, @clecat) - Fix a bug preventing `.corrected` files to be written in some cases (realworldocaml/mdx#136, @clecat) - Add compatibility with `4.09.0` (realworldocaml/mdx#133, @xclerc) #### Removed - Remove the `infer-timestamp` direction (realworldocaml/mdx#171 @Julow)
CHANGES: #### Added - Add a `--output`/`-o` option to the `test` subcommand to allow specifying a different output file to write the corrected to, or to write it to the standard output (realworldocaml/mdx#194, @NathanReb) - Migrate to OCaml 4.08 AST to add support for `let*` bindings (realworldocaml/mdx#190, @gpetiot) - Add `--syntax` option to `rule` subcommand to allow generating rules for cram tests (realworldocaml/mdx#177, @craigfe) - Add a `require-package` label to explicitly declare dune `package` dependencies of a code block (realworldocaml/mdx#149, @Julow) - Add an `unset-` label to unset env variables in shell blocks (realworldocaml/mdx#132, @clecat) #### Changed - Format rules generated by `ocaml-mdx rule` using `dune format-dune-file` (realworldocaml/mdx#184, @NathanReb) - Run promotion of markdown files before `.ml` files in generated dune rules (realworldocaml/mdx#140, @clecat) #### Fixed - Use module_presence information on Env.summary to prevent fetching absent modules from the toplevel env (realworldocaml/mdx#186, @clecat) - Remove trailing whitespaces at the end of toplevel or bash evaluation lines (realworldocaml/mdx#166, @clecat) - Improve error reporting of ocaml-mdx test (realworldocaml/mdx#172, @Julow) - Rule: Pass the --section option to `test` (realworldocaml/mdx#176, @Julow) - Remove trailing whitespaces from shell outputs and toplevel evals (realworldocaml/mdx#166, @clecat) - Remove inappropriate empty lines in generated dune rules (realworldocaml/mdx#163, @Julow) - Fix ignored `skip` label in `ocaml-mdx pp` (realworldocaml/mdx#1561, @craigfe) - Fix synchronization of new parts from markdown to `.ml` (realworldocaml/mdx#156, @Julow) - Fix ignored `[@@@Parts ...]` markers within module definitions (realworldocaml/mdx#155, @Julow) - Fix a bug in internal OCaml version comparison that lead to crashes in some cases (realworldocaml/mdx#145, @gpetiot) - Promote to empty `.ml` file when using `to-ml` direction (realworldocaml/mdx#139, @clecat) - Apply `--force-output` to `.ml` file as well (realworldocaml/mdx#137, @clecat) - Fix a bug preventing `.corrected` files to be written in some cases (realworldocaml/mdx#136, @clecat) - Add compatibility with `4.09.0` (realworldocaml/mdx#133, @xclerc) #### Removed - Remove the `output` subcommand as it was very specific to RealWorldOCaml needs (realworldocaml/mdx#195, @NathanReb) - Remove the `infer-timestamp` direction (realworldocaml/mdx#171 @Julow)
Rebased on top of #171
Expected errors were shown as uncaught exceptions