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

Feature request: quiet option hides errors in addition to warnings #2078

Open
Khady opened this issue May 19, 2022 · 3 comments
Open

Feature request: quiet option hides errors in addition to warnings #2078

Khady opened this issue May 19, 2022 · 3 comments

Comments

@Khady
Copy link
Contributor

Khady commented May 19, 2022

Describe the bug

We have a problem with the quiet option (which is kind of necessary as long as we are using deprecated options) in case of formatting error as it shows absolutely no details and just exits with code 1. I think that it would be better if errors were always displayed or if quiet could be configured to select what to hide.

When a file can't be compiled because it has an invalid syntax it is a problem but it is reduced by the fact that the ocaml compiler will tell us exactly what happens. When a file is valid ocaml but contains something that ocamlformat is unhappy with it becomes hard to understand the situation.

How to Reproduce

example 1

type public_error =
  [ `InternalError
  | `InvalidInput of string
  | `NotFound of string
  | `InvalidHttpMethod of string
  | session_error
  ]
(** foo bar *)
type private_error =
  [ `Sql_exception
  | `Csv_export
  | `Export_error of string
  ]

without quiet the problem is clear

opam exec -- dune build  @fmt --auto-promote
Entering directory '/home/louis.roche/Code/ahrefs/repo/backend'
File "dune-project", line 3, characters 0-40:
3 | (formatting
4 |  (enabled_for ocaml reason))
Warning: option `break-cases`: value `toplevel` is deprecated since version 0.20.0. It will be removed by version 1.0.0.
Warning: break-struct: This option is deprecated since version 0.20.0. It will be removed by version 1.0.0.
Warning: option `indicate-multiline-delimiters`: value `closing-on-separate-line` is deprecated since version 0.20.0. It will be removed by version 1.0.0.
Warning: nested-match: This option is deprecated since version 0.20.0. It will be removed by version 1.0.0.
Warning: option `module-item-spacing`: value `preserve` is deprecated since version 0.20.0. It will be removed by version 1.0.0.
Warning: option `break-cases`: value `toplevel` is deprecated since version 0.20.0. It will be removed by version 1.0.0.
Warning: break-struct: This option is deprecated since version 0.20.0. It will be removed by version 1.0.0.
Warning: option `indicate-multiline-delimiters`: value `closing-on-separate-line` is deprecated since version 0.20.0. It will be removed by version 1.0.0.
Warning: nested-match: This option is deprecated since version 0.20.0. It will be removed by version 1.0.0.
Warning: option `module-item-spacing`: value `preserve` is deprecated since version 0.20.0. It will be removed by version 1.0.0.
ocamlformat: ignoring "api/src/api.ml" (misplaced documentation comments - warning 50)
File "api/src/api.ml", line 37, characters 0-14:
37 | (** foo bar *)
     ^^^^^^^^^^^^^^
Warning 50 [unexpected-docstring]: ambiguous documentation comment
Hint: (Warning 50) This file contains a documentation comment (** ... *) that the OCaml compiler does not know how to attach to the AST. OCamlformat does not support these cases. You can find more information at: https://github.com/ocaml-ppx/ocamlformat#overview. If you'd like to disable this check and let ocamlformat make a choice (though it might not be consistent with the ocaml compilers and odoc), you can set the --no-comment-check option.

with quiet we are left in the dark

opam exec -- dune build  @fmt --auto-promote
Entering directory '/home/louis.roche/Code/ahrefs/repo/backend'
File "dune-project", line 3, characters 0-40:
3 | (formatting
4 |  (enabled_for ocaml reason))
Command exited with code 1.
make: [Makefile:190: fmt] Error 1 (ignored)
@hhugo
Copy link
Collaborator

hhugo commented May 19, 2022

quiet was implemented this way on purpose. see #243
Maybe another option should be introduced. (quiet-warn / quiet-error)

@Khady
Copy link
Contributor Author

Khady commented May 19, 2022

I can't change the labels, but this issue should then be a feature request.

@Khady Khady changed the title Bug: quiet option hides errors in addition to warnings Feature request: quiet option hides errors in addition to warnings May 19, 2022
@hhugo
Copy link
Collaborator

hhugo commented May 19, 2022

label updated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants