Skip to content

Deprecate some options before 0.14#1293

Merged
Julow merged 6 commits intoocaml-ppx:masterfrom
Julow:deprecate-0-14
Mar 25, 2020
Merged

Deprecate some options before 0.14#1293
Julow merged 6 commits intoocaml-ppx:masterfrom
Julow:deprecate-0-14

Conversation

@Julow
Copy link
Copy Markdown
Collaborator

@Julow Julow commented Mar 18, 2020

This PR deprecate the options doc-comments, escape-chars, escape-strings and extension-sugar.

The first commit improves the deprecation mechanism by passing a custom message and a version string. This is in the same PR because we don't have deprecated options currently to show diffs.

Julow added 2 commits March 18, 2020 15:38
Pass a custom message per option and declare since which version.
Copy link
Copy Markdown
Collaborator

@gpetiot gpetiot left a comment

Choose a reason for hiding this comment

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

Can we also deprecate exp-grouping ?

@Julow
Copy link
Copy Markdown
Collaborator Author

Julow commented Mar 18, 2020

exp-grouping is not working correctly, indeed.
What do you think @emillon ?

@emillon
Copy link
Copy Markdown
Collaborator

emillon commented Mar 18, 2020

It's a bit less conservative to include this one since it's not the default value in the profiles we ship. What's not working correctly with this option?

@Julow
Copy link
Copy Markdown
Collaborator Author

Julow commented Mar 18, 2020

The problem of exp-grouping is that it is applied only to some places and that begin .. end can be removed if they are unecessary.
This option is not a lost cause, we can describe every cases where it's useful and remove cases where it's not (currently there is no general rule and no doc). I expect a lot of maintenance and this will remove most begin .. end anyway.
An option that always put begin .. end when OCamlformat wants would be more useful, I think, to avoid users wondering about inconsistency of a "preserve but only sometimes" option.
Also, it's not used a lot: https://gist.github.com/Julow/110dc94308d6078225e0665e3eccd433#file-output-L137

@emillon emillon added this to the 0.14.0 milestone Mar 20, 2020
Copy link
Copy Markdown
Collaborator

@gpetiot gpetiot left a comment

Choose a reason for hiding this comment

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

Looks good to me, I suggested a small improvement for the warning messages. But if it requires too much engineering it's not necessary (or could be done later) and shouldn't block this PR.

in
C.choice ~names ~all ~doc ~section
let deprecated =
C.deprecated ~since_version:"0.14.0" "There is no equivalent."
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can we have a more precise behavior where the warning (warning only, not the --help manual) depends on the selected value? Like:

  • preserve : this option is not required anymore, character escaping is now preserved by default,
  • others : this feature has been removed, character escaping is now preserved by default.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure if this is necessary.
The option was already not required when set to the default value, is this something we should warn regardless if the option is deprecated ?
The default didn't change because of the deprecation and we can't predict if it will change after we removed the option.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It make sense. Alright let's merge if it's ready :)

@Julow Julow merged commit 85b1fef into ocaml-ppx:master Mar 25, 2020
Julow added a commit to Julow/opam-repository that referenced this pull request Apr 2, 2020
CHANGES:

#### New features

  + Add an option `--format-invalid-files` to print unparsable parts of the input as verbatim text. This feature is still experimental. (ocaml-ppx/ocamlformat#1026) (Guillaume Petiot)

  + Support multi-indices extended indexing operators (ocaml-ppx/ocamlformat#1279, ocaml-ppx/ocamlformat#1277) (Jules Aguillon, Guillaume Petiot)
    This feature has been added in OCaml 4.10.0

  + Handle OCaml 4.10.0 AST (ocaml-ppx/ocamlformat#1276) (Guillaume Petiot)

  + Preserve functor syntax for consistency (ocaml-ppx/ocamlformat#1312) (Guillaume Petiot)
    Previously both functor syntax: `module M = functor (K : S) -> struct end` and `module M (K : S) = struct end` would be formatted as the latter, the original syntax is now preserved.

#### Changes

  + Add the option `doc-comments-val=before|after` (ocaml-ppx/ocamlformat#1012) (Jules Aguillon)
    This option set the placement of documentation comment on `val` and `external` only.
    It is set to `after` by default.

  + The default for `doc-comments` is changed from `after` to `before` (ocaml-ppx/ocamlformat#1012, ocaml-ppx/ocamlformat#1325) (Jules Aguillon)
    This affects both `conventional` (default) and `ocamlformat` profiles.

  + Some options are now deprecated:
    * `doc-comments` (ocaml-ppx/ocamlformat#1293, ocaml-ppx/ocamlformat#1012)
      This option depends on a flawed heuristic.
      It is replaced by `doc-comments-val` for `val` and `external` declarations.
      There is no equivalent to this option in the general case.
    * `escape-chars`, `escape-strings` and `extension-sugar` (ocaml-ppx/ocamlformat#1293)
      These options are rarely used and their default behavior is considered to be the right behavior.

  + Add space between `row_field` attributes and the label or arguments, to be
    consistent with the non-polymorphic case. (ocaml-ppx/ocamlformat#1299) (Craig Ferguson)

#### Bug fixes

  + Fix missing parentheses around `let open` (ocaml-ppx/ocamlformat#1229) (Jules Aguillon)
    eg. `M.f (M.(x) [@attr])` would be formatted to `M.f M.(x) [@attr]`, which would crash OCamlformat

  + Remove unecessary parentheses with attributes in some structure items:
    * extensions and eval items (ocaml-ppx/ocamlformat#1230) (Jules Aguillon)
      eg. the expression `[%ext (() [@attr])]` or the structure item `(() [@attr]) ;;`
    * `let _ = ...`  constructs (ocaml-ppx/ocamlformat#1244) (Etienne Millon)

  + Fix some bugs related to comments:
    * after a function on the rhs of an infix (ocaml-ppx/ocamlformat#1231) (Jules Aguillon)
      eg. the comment in `(x >>= fun y -> y (* A *))` would be dropped
    * in module unpack (ocaml-ppx/ocamlformat#1309) (Jules Aguillon)
      eg. in the module expression `module M = (val x : S (* A *))`

  + Fix formatting of empty signature payload `[%a:]` (ocaml-ppx/ocamlformat#1236) (Etienne Millon)

  + Fix parenthesizing when accessing field of construct application (ocaml-ppx/ocamlformat#1247) (Guillaume Petiot)

  + Fix formatting of attributes on object overrides `{< >}` (ocaml-ppx/ocamlformat#1238) (Etienne
    Millon)

  + Fix attributes on coercion (ocaml-ppx/ocamlformat#1239) (Etienne Millon)

  + Fix formatting of attributes on packed modules (ocaml-ppx/ocamlformat#1243) (Etienne Millon)

  + Fix parens around binop operations with attributes (ocaml-ppx/ocamlformat#1252, ocaml-ppx/ocamlformat#1306) (Guillaume Petiot, Craig Ferguson)

  + Remove unecessary parentheses in the argument of indexing operators (ocaml-ppx/ocamlformat#1280) (Jules Aguillon)

  + Retain attributes on various AST nodes:
    * field set expressions, e.g. `(a.x <- b) [@A]` (ocaml-ppx/ocamlformat#1284) (Craig Ferguson)
    * instance variable set expressions, e.g. `(a <- b) [@A]` (ocaml-ppx/ocamlformat#1288) (Craig Ferguson)
    * indexing operators, e.g. `(a.(b)) [@A]` (ocaml-ppx/ocamlformat#1300) (Craig Ferguson)
    * sequences, e.g. `(a; b) [@A]` (ocaml-ppx/ocamlformat#1291) (Craig Ferguson)

  + Avoid unnecessary spacing after object types inside records and polymorphic variants,
    e.g. `{foo : < .. > [@A]}` and `{ foo : < .. > }` (ocaml-ppx/ocamlformat#1296) (Craig Ferguson)

  + Fix missing parentheses around tuples with attributes. (ocaml-ppx/ocamlformat#1301) (Craig Ferguson)
    Previously, `f ((0, 0) [@A])` would be formatted to `f (0, 0) [@A]`, crashing OCamlformat.

  + Avoid emitting `>]` when an object type is contained in an extension point
    or attribute payload (ocaml-ppx/ocamlformat#1298) (Craig Ferguson)

  + Fix crash on the expression `(0).*(0)` (ocaml-ppx/ocamlformat#1304) (Jules Aguillon)
    It was formatting to `0.*(0)` which parses as an other expression.

  + Preserve empty doc-comments syntax. (ocaml-ppx/ocamlformat#1311) (Guillaume Petiot)
    Previously `(**)` would be formatted to `(***)`.

  + Do not crash when a comment contains just a newline (ocaml-ppx/ocamlformat#1290) (Etienne Millon)

  + Handle lazy patterns as arguments to `class` (ocaml-ppx/ocamlformat#1289) (Etienne Millon)

  + Preserve cinaps comments containing unparsable code (ocaml-ppx/ocamlformat#1303) (Jules Aguillon)
    Previously, OCamlformat would fallback to the "wrapping" logic, making the comment
    unreadable and crashing in some cases.

  + Fix normalization of attributes, fixing the docstrings in attributes (ocaml-ppx/ocamlformat#1314) (Guillaume Petiot)

  + Add missing parentheses around OR-patterns with attributes (ocaml-ppx/ocamlformat#1317) (Guillaume Petiot)

  + Fix spacing inside parens for symbols when the spacing was handled by the englobing exp (ocaml-ppx/ocamlformat#1316) (Guillaume Petiot)

  + Fix invalid (unparsable) docstrings (ocaml-ppx/ocamlformat#1315) (Guillaume Petiot)
    When parsing a comment raises an error in odoc, it is printed as-is.

  + Fix parenthesizing of optional arguments rebound to non-variables, e.g. `let
    f ?a:(A) = ()` rather than the unparsable `let f ?a:A = ()` (ocaml-ppx/ocamlformat#1305) (Craig Ferguson)
Julow added a commit to Julow/opam-repository that referenced this pull request Apr 14, 2020
CHANGES:

#### Changes

  + The default for `doc-comments` is changed to `after` (ocaml-ppx/ocamlformat#1335) (Jules Aguillon)
    This reverts a change introduced in 0.14.0 (ocaml-ppx/ocamlformat#1012).

  + Revert deprecation of the `doc-comments` option (ocaml-ppx/ocamlformat#1331) (Jules Aguillon)
    This reverts a change introduced in 0.14.0 (ocaml-ppx/ocamlformat#1293).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants