Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.Sign up
Bug fixes for tools/pprintast.ml #5775
Original bug ID: 5775
I have attached some bug fixes for tools/pprintast.ml, currently I use tools/pprintast.ml to bootstrapping camlP4, if the community reach a consensus that we should have a high quality printer for Parsetree to get valid vanilla ocaml code, I would be happy to spend more time to get a beautiful output
Comment author: @gasche
The patch looks reasonable (but why all those commented lines in the Pcl_fun case?). I haven't looked into it in the details, but given that the original code is quite shaky anyway I think this will be an improvement -- and there are a couple glaring bug fixed by the commit.
As a side remark, could you format your diff in the so-called "unified" format (diff -u)? They're much easier to read online because they provide context around the changes. I think using the unified format is pretty much the standard now, and I will upload a unified version of your patch for future potential reviewers.
Comment author: @damiendoligez
We don't (yet) have written guidelines. Off the top of my head:
Step (4) is optional for small changes.
Asking for a code review (as you did) is also a good idea, unless it's a small change and you know what you're doing.