-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Add pretty print to Javascript toStringTree #3229
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
Conversation
None of those instances need an |
Hi, they do for clarity. |
I can't say I agree that that would make anything clearer. I can do it, but it will simply mean that there is unnecessary duplication. |
Instead of
You'd have
|
or: const res = prettyPrint ? another option would be to have toStringTree dispatch to toUglyStringTree/toPrettyStringTree such that the maintainer does not have to think twice about side effects when evolving the formatting |
I suggest not using parentheses form for the prettified tree:
Also, I suggest adding a similar method to all runtimes, not only to JavaScript. See #1502 |
I was just going off the similar PR that was merged for C++: #2505 |
Maybe it makes sense to add a method for prettified output (my suggestion above) and method that outputs resulting JSON that can be easily serialized/deserialized. |
Have you guys come to a consensus? |
If add the feature to JavaScript target it should be added to all other targets as well for consistency. |
yeah that sounds like a big change that I don't want to focus on. Maybe we just make that function available as part of the documentation for JavaScript? |
I don't know but I'm not sure it's a very demanded feature for the current time. Especially considering that tree in any format can be obtained using a utility method in runtime. Also, not sure about lisp-like format. I would think about something serializable at first (JSON). |
The lisp thing is good for showing the structure but I still prefer the GUI visualization using the intellij plug-in (all hail jetbrains!) |
Maybe it's better for showing than JSON. But my suggested format with indents and without parentheses is also good and is shorter (BTW, I took it from Kotlin specification repository). That's why I don't want to rush with accepting a similar request because pretty print formats may differ and may be subjective. On the other hand, JSON format can be converted to almost everything else. |
I'm no longer using antlr at all and have no bandwidth to address this, so will close out. Of course feel free to repurpose my changes if you want them. |
Non-pretty (existing functionality):
Pretty: