Multiple pretty printing APIs?

[amomchilov]

By my count, there are 3:

• C pm_prettyprint()

  • prism/templates/src/prettyprint.c.erb

    Line 27 in 16e5efb

    prettyprint_node(pm_buffer_t *output_buffer, const pm_parser_t *parser, const pm_node_t *node, pm_buffer_t *prefix_buffer) {

• Ruby's Prism::Node#inspect

  • prism/templates/lib/prism/inspect_visitor.rb.erb

    Line 4 in 16e5efb

    class InspectVisitor < Visitor

• Java org.prism.Nodes.Node.toString()

  • prism/templates/java/org/prism/Nodes.java.erb

    Line 334 in 16e5efb

    protected String toString(String indent) {

They're similar, but have slightly different formatting. E.g. C uses a +-- instead of a fancy ├──.

Notably, Kevin rewrote the Ruby implementation to use a queue instead of recursion in #2732, but the C and Java implementation still use recursion.

Would it be better to standardize all pretty printing in the C layer, and expose that to Ruby and Java?

[kddnewton]

The Java one is mostly for debugging, so I'm not particularly worried about that one. I think it would be fine for it to be improved/changed to match, but I'm not sure if @enebo or @eregon are necessarily interested.

The C and Ruby ones cannot be shared. The C one is for when you're working with C structs, the Ruby ones are for when you're working with Ruby objects. By the time we've reified all of the C structs into Ruby, we can no longer use the C version since the Ruby objects do not go back into C. At the same time, we can't use the Ruby version from C because the C version is used by --dump=parsetree, so we don't necessarily have the Ruby extension loaded at that point. Furthermore, we can't use non-ASCII characters in that context, so the output has to be slightly different.

So, they're all serving different purposes.

[eregon]

Indeed as @kddnewton said the Java one is mostly for debugging. It could be improved to match better, but I doubt it's worth the complexity,

prism/templates/java/org/prism/Nodes.java.erb

Lines 334 to 383 in 16e5efb

	protected String toString(String indent) {
	StringBuilder builder = new StringBuilder();
	builder.append(this.getClass().getSimpleName());
	if (hasNewLineFlag()) {
	builder.append("[Li]");
	}
	builder.append('\n');
	String nextIndent = indent + " ";
	<%- if node.fields.any?(Prism::Template::NodeListField) or node.fields.any?(Prism::Template::ConstantListField) -%>
	String nextNextIndent = nextIndent + " ";
	<%- end -%>
	<%- [*node.flags, *node.fields.grep_v(Prism::Template::LocationField).grep_v(Prism::Template::OptionalLocationField)].each do |field| -%>
	builder.append(nextIndent);
	builder.append("<%= field.name %>: ");
	<%- case field -%>
	<%- when Prism::Template::NodeField -%>
	builder.append(this.<%= field.name %>.toString(nextIndent));
	<%- when Prism::Template::OptionalNodeField -%>
	builder.append(this.<%= field.name %> == null ? "null\n" : this.<%= field.name %>.toString(nextIndent));
	<%- when Prism::Template::NodeListField -%>
	builder.append('\n');
	for (Node child : this.<%= field.name %>) {
	builder.append(nextNextIndent).append(child.toString(nextNextIndent));
	}
	<%- when Prism::Template::StringField -%>
	builder.append('"' + new String(this.<%= field.name %>, StandardCharsets.UTF_8) + '"');
	builder.append('\n');
	<%- when Prism::Template::ConstantField -%>
	builder.append('"').append(this.<%= field.name %>).append('"');
	builder.append('\n');
	<%- when Prism::Template::OptionalConstantField -%>
	builder.append(this.<%= field.name %> == null ? "null" : "\"" + this.<%= field.name %> + "\"");
	builder.append('\n');
	<%- when Prism::Template::ConstantListField -%>
	builder.append('\n');
	for (<%= string_type %> constant : this.<%= field.name %>) {
	builder.append(nextNextIndent).append('"').append(constant).append('"').append('\n');
	}
	<%- when Prism::Template::Flags -%>
	builder.append(flags);
	builder.append('\n');
	<%- when Prism::Template::UInt8Field, Prism::Template::UInt32Field, Prism::Template::IntegerField, Prism::Template::DoubleField -%>
	builder.append(this.<%= field.name %>);
	builder.append('\n');
	<%- else -%>
	<%- raise field.class.name -%>
	<%- end -%>
	<%- end -%>
	return builder.toString();
	}

is already long and complicated.