POC: print patterns in warnings using rescript printer

[amiralies]

needs exposing printPattern function from res_printer.
there's also an issue with or patterns printing redundant parens which i don't know should be fixed in printer or compiler

[cristianoc]

How does ocaml do it? Does it have its own untype function?
In general, it might make sense to move as much functionality to the printer. So if it knows how to print or's then it can expose that functionality and the compiler use it.

[amiralies]

There's an untype mapper for pattern in untypeast which is much like the one in PR but with different label names.
for printing pretty_val in parmatch.ml flattens Tpat_ors recursively (see pretty_or).
something similar to this flattening is happening in rescript's printer but it seems it flattens one side or sth.

[amiralies]

does paranthesis have meaning in or patterns?
seems like ocamlformat eliminates all of them but reason and rescript printers try to preserve some of them.

[cristianoc]

does paranthesis have meaning in or patterns?
seems like ocamlformat eliminates all of them but reason and rescript printers try to preserve some of them.

The fact that we need to ask the question (and I'd need to go and check to remind myself eg what about type annotations) means the printer knows better and it's not the compiler's error message business.
But, perhaps for the next PR?

[cristianoc]

I think this PR is good to go, as long as one resolves what's going on in CI.

[amiralies]

Thanks.
I need to update failing tests and also add more tests (list patterns for instance).
an small patch should be applied to syntax repo too.

[amiralies]

I've added another prepass before printing in order to avoid extra nested parens when printing Ppat_ors

[amiralies]

This branch needs this patch rescript-lang/syntax#595 for compiling

[cristianoc]

This looks good.
Would you

• rebase on master
• check in the syntax submodule commit (now merged)
• do the work at construction time in the untype function, no extra processing

Then we can merge.

[amiralies]

will revisit this soon

[cristianoc]

rebased on master

[cristianoc]

@amiralies did something go wrong rebasing on master, or what's going on with these tests?

[amiralies]

I didn't update test results

[cristianoc]

I ran the tests and what you see is the current output.

[cristianoc]

Can you do a clean / config / build
looks this there's one modified file that's not checked in.

[cristianoc]

% git diff
diff --git a/jscomp/core/bs_conditional_initial.ml b/jscomp/core/bs_conditional_initial.ml
index 79fdf1e97..7a85592e0 100644
--- a/jscomp/core/bs_conditional_initial.ml
+++ b/jscomp/core/bs_conditional_initial.ml
@@ -34,7 +34,6 @@ let setup_env () =
   Ctype.variant_is_subtype := Matching_polyfill.variant_is_subtype;
   Clflags.dump_location := false;  
   Config.syntax_kind := `rescript;
-  Parmatch.print_res_pat := Pattern_printer.print_pattern;
   
 # 38 "core/bs_conditional_initial.pp.ml"
   Clflags.color := Some Always;
@@ -74,4 +73,4 @@ let setup_env () =
 
 
 let () = 
-  at_exit (fun _ -> Format.pp_print_flush Format.err_formatter ())
+  at_exit (fun _ -> Format.pp_print_flush Format.err_formatter ())

[amiralies]

let me see what's happening

[cristianoc]

Oh right need to change this instead: core/bs_conditional_initial.pp.ml

[cristianoc]

This now behaves as expected.

[cristianoc]

Just one final cleanup.

[cristianoc]

@amiralies see the final cleanup

I think this is ready to merge. OK?

[amiralies]

Yes! it looks better now