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.

Sign up for GitHub

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

Jump to bottom

Improve: improve errors returned to the user. #243

Merged
merged 1 commit into from
Jul 31, 2018
Merged

Improve: improve errors returned to the user. #243

merged 1 commit into from
Jul 31, 2018

Conversation

hhugo
Copy link
Collaborator

@hhugo hhugo commented Jul 30, 2018

Explicitly discriminate errors between

  • invalid source (together with the reason why)
  • and ocamlformat bugs (detail with -g/--debug)

Additionnally,

  • ocamlformat does not stop at the first error and process all inputs.
  • add a -quiet flag
  • remove -warn-error flag
  • [@@@ocamlformat.disable] is only interpreted at toplevel

@hhugo
Copy link
Collaborator Author

hhugo commented Jul 30, 2018

With this PR, one get the following behavior

  1. Multiple invalid input files
./_build/release/src/ocamlformat.exe test/passing/error*.ml --inplace
ocamlformat.exe: ignoring "test/passing/error1.ml" (syntax error)
File "test/passing/error1.ml", line 2, characters 0-0:
Error: Syntax error
ocamlformat.exe: ignoring "test/passing/error2.ml" (syntax error)
File "test/passing/error2.ml", line 1, characters 0-1:
Error: String literal not terminated
ocamlformat.exe: ignoring "test/passing/error3.ml" (misplaced documentation comments - warning 50)
File "test/passing/error3.ml", line 2, characters 0-13:
Warning 50: ambiguous documentation comment
File "test/passing/error3.ml", line 3, characters 8-16:
Warning 50: unattached documentation comment (ignored)
# exit non zero
  1. Multiple invalid input files, quiet mode
./_build/release/src/ocamlformat.exe test/passing/error*.ml --inplace -q
# exit non zero
  1. Multiple files revealing bugs
./_build/release/src/ocamlformat.exe ../js_of_ocaml/lib/dom_html.ml{,i} --inplace
ocamlformat.exe: Cannot process "../js_of_ocaml/lib/dom_html.ml".
  Please report this bug at https://github.com/ocaml-ppx/ocamlformat/issues.
  BUG: generating invalid ocaml syntax.
ocamlformat.exe: Cannot process "../js_of_ocaml/lib/dom_html.mli".
  Please report this bug at https://github.com/ocaml-ppx/ocamlformat/issues.
  BUG: generating invalid ocaml syntax.
# exit non zero

@hhugo
Improve: improve errors returned to the user.
c3991d9 
Explicitly discriminate errors between
- invalid source (together with the reason why)
- and ocamlformat bugs (detail with -g/--debug)

Additionnally,
- ocamlformat does not stop at the first error and process all input.
- add a -quiet flag
- remove -warn-error flag
- [@@@ocamlformat.disable] is only interpreted at toplevel
@hhugo
Copy link
Collaborator Author

hhugo commented Jul 30, 2018

partially address #203

@jberdine jberdine merged commit 23a68ae into ocaml-ppx:master Jul 31, 2018
@hhugo hhugo deleted the errors branch July 31, 2018 00:57
gpetiot pushed a commit to gpetiot/ocamlformat that referenced this pull request Jul 31, 2018
@hhugo @gpetiot
Improve: improve errors returned to the user. (ocaml-ppx#243)
db51212 
Explicitly discriminate errors between
- invalid source (together with the reason why)
- and ocamlformat bugs (detail with -g/--debug)

Additionally,
- ocamlformat does not stop at the first error and processes all input.
- add a -quiet flag
- remove -warn-error flag
- [@@@ocamlformat.disable] is no longer only interpreted at toplevel
jberdine
jberdine reviewed Jul 31, 2018
View reviewed changes
src/Normalize.ml
raise Formatting_disabled
| _ -> ()
in
{Ast_iterator.default_iterator with structure_item; signature_item}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that Ast_iterator is not handled by ocaml-migrate-parsetree, so this change causes compilation failure for ocaml <> 4.07. Perhaps it can be added to OMP: ocaml-ppx/ocaml-migrate-parsetree#47 .

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#252

@hhugo hhugo mentioned this pull request Aug 1, 2018
Closed
@hhugo hhugo mentioned this pull request Mar 11, 2019
Closed
@hhugo hhugo mentioned this pull request May 19, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jberdine jberdine jberdine left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hhugo @jberdine @facebook-github-bot