-
Notifications
You must be signed in to change notification settings - Fork 333
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
Cookbook: Debug Print a Value with ppx_deriving #2917
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @mostafatouny
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @mostafatouny. I just wanted to inform you that your comments don't provide helpful information. This needs to be fixed.
The discussion block does not need project creation instruction. This is covered elsewhere.
removed project initialization. added more info to comments.
Thank you @cuihtlauac and @sabine for your time. I removed the project initialization and added more info to the comments. |
Thanks for the quick update, @mostafatouny. That's nice. I'm under the impression this recipe has more to do with using |
@cuihtlauac there is a connection to debugging. People often want to debug by just printing the values. Then they have to figure out how to print the values. And they see that they need to derive printers for custom types. So we should definitely mention this workflow. If we don't mention a motivating use case like debugging, it might not be clear to people why they would want to use this recipe. The recipe should justify itself by providing a use case. It's not enough to just show how to do it. |
Thanks, @yawaramin. You have a valid point. The key is discovering that you need ppx |
|
||
(* string *) | ||
let () = print_endline "hello world" | ||
let () = print_endline ([%show: string] "hello world") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems redundant. The string literal is already a string.
let () = print_endline ([%show: string] "hello world") | ||
|
||
(* integer *) | ||
let show_int = [%show: int] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is slightly more useful but no clear advantage over string_of_int
imho.
type tree = | ||
| Leaf of float | ||
| Node of float * tree * tree | ||
[@@deriving show] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might want to explain why we are suddenly switching from [%show: ...]
to [@@deriving show]
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It helps to take inspiration from the JSON parsing recipes, to give a better explanation of what @@deriving
does.
And indeed, makes sense to add a single sentence that explains that [%show: t]
generates the code to print values of type t
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sabine would you recommend me background resources to check? maybe I can write a short explanation and enhance the tutorial's accessibility.
I agree. If something is well-documented but end users cannot figure it is the solution to their problem, then it is useless.
We can create Thanks @yawaramin for the code feedback. I removed redundant |
Hello,
I am contributing to Debug Print a Value.
Moreover,
ppx_deriving
withutop -ppx
.