Internal: Future-proof Fmt API in case Fmt.t goes abstract by emillon · Pull Request #1106 · ocaml-ppx/ocamlformat

emillon · 2019-10-28T15:15:57Z

Hi,

This is a set of three independent improvements to Fmt in case Fmt.t becomes abstract in the future, but I think they improve the situation even if that's not the case:

transform list_pn arguments to required options (from optional ones). These arguments are always passed, so optional arguments do not have any advantages. Actually, when Fmt.t is made abstract, this confuses the argument erasure check since the last argument becomes ?next, which can't be erased (before, the last argument was the formatter).
monomorphize ( $ ) and move it to Fmt. It was only used like this in the code base.
add a Fmt.set_options helper. This ensures there is no sequencing with formatters done in Translation_unit itself. This iteration is done with ( $ ) instead, making sure it will continue working if Fmt.t is abstract.

Julow

That's great !
Why not making Fmt.t abstract now ?

src/Fmt.mli

src/Fmt.ml

src/Translation_unit.ml

emillon · 2019-10-28T16:00:11Z

There's more work in other modules to make Fmt.t abstract, I'd like to keep this PR small enough.

gpetiot

Looks good to me, I made the PR name more explicit, don't forget to add a line in the changelog and it's good to go.

emillon · 2019-10-29T09:13:18Z

Can we keep the original PR title? it explains the reasons of this change, not what the individual changes are. I'll keep the details in the commit message.

This is a set of three independent improvements to Fmt in case Fmt.t becomes abstract in the future, but I think they improve the situation even if that's not the case: - transform list_pn arguments to required options (from optional ones). These arguments are always passed, so optional arguments do not have any advantages. Actually, when Fmt.t is made abstract, this confuses the argument erasure check since the last argument becomes ?next, which can't be erased (before, the last argument was the formatter). - monomorphize ( $ ) and move it to Fmt. It was only used like this in the code base. - refactor how the output formatter is set up so that it does not use explicit sequencing. This ensures there is no sequencing with formatters done in Translation_unit itself. This iteration is done with ( $ ) instead, making sure it will continue working if Fmt.t is abstract.

gpetiot · 2019-10-29T09:19:52Z

Can we keep the original PR title? it explains the reasons of this change, not what the individual changes are. I'll keep the details in the commit message.

If the commit message is well detailed sure.

Julow

Looks good to me :)

CHANGES: #### Changes + Set "conventional" as the default profile (ocaml-ppx/ocamlformat#1060) (Guillaume Petiot) This new profile is made to better match the most used style and is encouraged. To continue using the previous default, use `profile = ocamlformat` in your `.ocamlformat`. + CLI: Allow both values of boolean options (ocaml-ppx/ocamlformat#1062) (Jules Aguillon) Now, both `--opt` and --no-opt` are available on the CLI for any boolean option "opt". Previously, only one of them were available depending on the default value. + Auto mode for `break-string-literals` (ocaml-ppx/ocamlformat#1057) (Guillaume Petiot) `wrap`, `newlines` and `newlines-and-wrap` values of `break-string-literals` are removed. `auto` replaces them, it is equivalent to `newlines-and-wrap`. + Dock collection brackets (ocaml-ppx/ocamlformat#1014) (Guillaume Petiot) `after-and-docked` value of `break-separators` is removed and is replaced by a new `dock-collection-brackets` option. + Preserve `begin` and `end` keywords in if-then-else (ocaml-ppx/ocamlformat#978) (Jules Aguillon) Previously, `begin`/`end` keywords around if-then-else branches were turned into parentheses. #### New features + Give a hint when warning 50 is raised (ocaml-ppx/ocamlformat#1111) (Guillaume Petiot) + Add a message when a config value is removed (ocaml-ppx/ocamlformat#1089) (Etienne Millon) Explain what replaces removed options and avoid printing a parsing error. + Implement `sequence-blank-line=preserve-one` for let bindings (ocaml-ppx/ocamlformat#1077) (Jules Aguillon) Preserve a blank line after `let .. in` when `sequence-blank-line` set to `preserve-one`. Previously, only blank lines after `;` could be preserved. + Parse toplevel directives (ocaml-ppx/ocamlformat#1020) (Jules Aguillon) Allow `#directives` in `.ml` files. Previously, files containing a directive needed to be parsed as "use file". The "use file" mode is removed and `--use-file` is now the same as `--impl`. + Don't require `--name`, require kind, forbid `--inplace`, allow `--check`, make `--enable-outside-detected-project` implicit when reading from stdin (ocaml-ppx/ocamlformat#1018) (Guillaume Petiot) + Parse code in docstrings (ocaml-ppx/ocamlformat#941) (Guillaume Petiot) Format OCaml code in cinaps-style comments `(*$ code *)` and code blocks in documentation comments `(** {[ code ]} *)`. + Parse documentation comments with Odoc (ocaml-ppx/ocamlformat#721) (Jules Aguillon) Formatting of documentation comments is more robust and support newer Odoc syntaxes. Internally, Odoc replaces Octavius as the documentation parser. #### Bug fixes + Fix unstabilizing comments on assignments (ocaml-ppx/ocamlformat#1093) (Guillaume Petiot) + Fix the default value documentation for `max-indent` (ocaml-ppx/ocamlformat#1105) (Guillaume Petiot) + Fix closing parenthesis exceeding the margin in function application (ocaml-ppx/ocamlformat#1098) (Jules Aguillon) + Missing break before attributes of `Pmty_with` (ocaml-ppx/ocamlformat#1103) (Josh Berdine) + Fix closing quote exceeding the margin (ocaml-ppx/ocamlformat#1096) (Jules Aguillon) + Fix break before the closing bracket of collections (exceeding the margin) (ocaml-ppx/ocamlformat#1073) (Guillaume Petiot) + Fix precedence of Dot wrt Hash (ocaml-ppx/ocamlformat#1058) (Guillaume Petiot) + Fix break in variant type definition to not exceed the margin (ocaml-ppx/ocamlformat#1064) (Guillaume Petiot) + Fix newlines and indentation in toplevel extension points (ocaml-ppx/ocamlformat#1054) (Guillaume Petiot) + Fix placement of doc comments around extensions (ocaml-ppx/ocamlformat#1052) (Jules Aguillon) + Inline extensions that do not break (ocaml-ppx/ocamlformat#1050) (Guillaume Petiot) + Add missing cut before attributes in type declarations (ocaml-ppx/ocamlformat#1051) (Guillaume Petiot) + Fix alignment of cases (ocaml-ppx/ocamlformat#1046) (Guillaume Petiot) + Fix blank line after comments at the end of lists (ocaml-ppx/ocamlformat#1045) (Guillaume Petiot) + Fix indexing operators precedence (ocaml-ppx/ocamlformat#1039) (Jules Aguillon) + Fix dropped comment after infix op (ocaml-ppx/ocamlformat#1030) (Guillaume Petiot) + No newline if the input is empty (ocaml-ppx/ocamlformat#1031) (Guillaume Petiot) + Fix unstable comments around attributes (ocaml-ppx/ocamlformat#1029) (Guillaume Petiot) + Fix extra blank line in sequence (ocaml-ppx/ocamlformat#1021) (Jules Aguillon) + Check functor arguments when computing placement of doc comments (ocaml-ppx/ocamlformat#1013) (Jules Aguillon) + Fix indentation of labelled args (ocaml-ppx/ocamlformat#1006) (Guillaume Petiot) + Fix linebreak between or-cases with comments when `break-cases=all` (ocaml-ppx/ocamlformat#1002) (Guillaume Petiot) + Fix unstable unattached doc comment in records (ocaml-ppx/ocamlformat#998) (Jules Aguillon) + Fix string literal changed (ocaml-ppx/ocamlformat#995) (Jules Aguillon) + Fix type variable (ocaml-ppx/ocamlformat#996) (Jules Aguillon) + Fix crash on extension sequence (ocaml-ppx/ocamlformat#992) (Guillaume Petiot) + Fix position of expressions regarding of comments in infix-op expressions (ocaml-ppx/ocamlformat#986) (Guillaume Petiot) + Escape special characters in external declaration (ocaml-ppx/ocamlformat#988) (Jules Aguillon) + Fix parens around constrained expr with attrs (ocaml-ppx/ocamlformat#987) (Guillaume Petiot) + Fix the margin, and correctly breaks comments (ocaml-ppx/ocamlformat#957) (Guillaume Petiot) + Fix formatting of custom indexing operators (ocaml-ppx/ocamlformat#975) (Guillaume Petiot) + Fix position of comments of labelled arrow types (ocaml-ppx/ocamlformat#976) (Guillaume Petiot) + No box around inline odoc styles (ocaml-ppx/ocamlformat#971) (Guillaume Petiot) + Fix boxing of collection expressions/patterns (ocaml-ppx/ocamlformat#960) (Guillaume Petiot) + Fix crash on record expr with pack fields (ocaml-ppx/ocamlformat#963) (Jules Aguillon) + Fix letop in subexpr (ocaml-ppx/ocamlformat#956) (hhugo) #### Internal + Take file kind from --name when formatting stdin (ocaml-ppx/ocamlformat#1119) (Jules Aguillon) + Make Fmt.t abstract (ocaml-ppx/ocamlformat#1109) (Jules Aguillon) + Future-proof Fmt API in case Fmt.t goes abstract (ocaml-ppx/ocamlformat#1106) (Etienne Millon) + Future-proof `Fmt` API in case `Fmt.t` goes abstract (ocaml-ppx/ocamlformat#1106) (Etienne Millon) + Optional names for formatting boxes in debug output (ocaml-ppx/ocamlformat#1083) (Guillaume Petiot) + Check ocamlformat error codes in the testsuite (ocaml-ppx/ocamlformat#1084) (Etienne Millon) + Clean `Translation_unit` (ocaml-ppx/ocamlformat#1078) (Guillaume Petiot) + Use dune file generation in test/passing/dune (ocaml-ppx/ocamlformat#1082) (Etienne Millon) + CI: factorize tests and check reason build (ocaml-ppx/ocamlformat#1079) (Guillaume Petiot) + Use short form for action in src/dune (ocaml-ppx/ocamlformat#1076) (Etienne Millon) + Cleanup `sequence_blank_line` (ocaml-ppx/ocamlformat#1075) (Jules Aguillon) + CI: use a script travis-ci.sh to simplify .travis.yml (ocaml-ppx/ocamlformat#1063) (Guillaume Petiot) + Remove utility functions from `Fmt_ast` (ocaml-ppx/ocamlformat#1059) (Guillaume Petiot) + CI: use opam-2.0.5 in Travis (ocaml-ppx/ocamlformat#1044) (Anton Kochkov) + CI: check the build with OCaml 4.07.1 and 4.08.0 (ocaml-ppx/ocamlformat#1036) (Jules Aguillon) + Use the same sets of options for both branches by default in `test_branch.sh` (ocaml-ppx/ocamlformat#1033) (Guillaume Petiot) + Fix `test_branch.sh` and CI checking of CHANGES.md (ocaml-ppx/ocamlformat#1032, ocaml-ppx/ocamlformat#1034) (Jules Aguillon) + Fix flag of git-worktree in `test_branch.sh` and `bisect.sh` (ocaml-ppx/ocamlformat#1027) (Guillaume Petiot) + Remove the `bisect_ppx` dependency and clean the `Makefile` (ocaml-ppx/ocamlformat#1005) (Jules Aguillon) + Use a `CHANGES.md` log file again (ocaml-ppx/ocamlformat#1023) (Guillaume Petiot) + Support OCaml 4.09.0 (add the odoc.1.4.2 dependency) (ocaml-ppx/ocamlformat#1024) (Guillaume Petiot) + Update labels of issue templates (ocaml-ppx/ocamlformat#1017) (Guillaume Petiot) + Update labels in `CONTRIBUTING.md` (ocaml-ppx/ocamlformat#1007) (Guillaume Petiot) + Allow to ignore invalid options (ocaml-ppx/ocamlformat#984) (hhugo) The `--ignore-invalid-option` flag is added to ignore invalid options in `.ocamlformat` files. + Improve the documentation of `--doc-comments` (ocaml-ppx/ocamlformat#982) (Jules Aguillon) + Remove symbolic links and change naming convention of tests (ocaml-ppx/ocamlformat#980) (Guillaume Petiot) + Change the type of `fmt_code` (ocaml-ppx/ocamlformat#974) (Guillaume Petiot) + Simplify `Makefile` (ocaml-ppx/ocamlformat#973) (hhugo) + Dune should not be flagged as a build dep anymore (ocaml-ppx/ocamlformat#954) (Guillaume Petiot)

…#1106) This is a set of three independent improvements to Fmt in case Fmt.t becomes abstract in the future, but I think they improve the situation even if that's not the case: - transform list_pn arguments to required options (from optional ones). These arguments are always passed, so optional arguments do not have any advantages. Actually, when Fmt.t is made abstract, this confuses the argument erasure check since the last argument becomes ?next, which can't be erased (before, the last argument was the formatter). - monomorphize ( $ ) and move it to Fmt. It was only used like this in the code base. - refactor how the output formatter is set up so that it does not use explicit sequencing. This ensures there is no sequencing with formatters done in Translation_unit itself. This iteration is done with ( $ ) instead, making sure it will continue working if Fmt.t is abstract.

facebook-github-bot added the CLA Signed label Oct 28, 2019

Julow reviewed Oct 28, 2019

View reviewed changes

src/Fmt.mli Outdated Show resolved Hide resolved

src/Fmt.ml Outdated Show resolved Hide resolved

src/Translation_unit.ml Outdated Show resolved Hide resolved

emillon force-pushed the fmt-abstract-prepare branch 2 times, most recently from fab5a74 to b877e67 Compare October 28, 2019 16:14

Julow mentioned this pull request Oct 28, 2019

Make Fmt.t abstract emillon/ocamlformat#1

Closed

gpetiot changed the title ~~Future-proof Fmt API in case Fmt.t goes abstract~~ Internal: add Fmt.set_options, monomorphize $, and require list_pn arguments Oct 28, 2019

gpetiot approved these changes Oct 28, 2019

View reviewed changes

gpetiot requested a review from Julow October 29, 2019 08:56

emillon force-pushed the fmt-abstract-prepare branch from b877e67 to 002acb6 Compare October 29, 2019 09:16

emillon changed the title ~~Internal: add Fmt.set_options, monomorphize $, and require list_pn arguments~~ Internal: Future-proof Fmt API in case Fmt.t goes abstract Oct 29, 2019

Merge branch 'master' into fmt-abstract-prepare

60f4ff9

Julow approved these changes Oct 29, 2019

View reviewed changes

gpetiot merged commit 21f237c into ocaml-ppx:master Oct 29, 2019

Julow mentioned this pull request Oct 29, 2019

Internal: Make Fmt.t abstract #1109

Merged

Julow mentioned this pull request Nov 4, 2019

[new release] ocamlformat (0.12) ocaml/opam-repository#15208

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Internal: Future-proof Fmt API in case Fmt.t goes abstract#1106

Internal: Future-proof Fmt API in case Fmt.t goes abstract#1106
gpetiot merged 2 commits intoocaml-ppx:masterfrom
emillon:fmt-abstract-prepare

emillon commented Oct 28, 2019

Uh oh!

Julow left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

emillon commented Oct 28, 2019

Uh oh!

gpetiot left a comment

Uh oh!

emillon commented Oct 29, 2019

Uh oh!

gpetiot commented Oct 29, 2019

Uh oh!

Julow left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

emillon commented Oct 28, 2019

Uh oh!

Julow left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

emillon commented Oct 28, 2019

Uh oh!

gpetiot left a comment

Choose a reason for hiding this comment

Uh oh!

emillon commented Oct 29, 2019

Uh oh!

gpetiot commented Oct 29, 2019

Uh oh!

Julow left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants