[m-rev.] for post-commit review: pretty_printer.m

Julien Fischer jfischer at opturion.com
Sun Aug 7 15:40:11 AEST 2016


Hi Zoltan,

On Sat, 6 Aug 2016, Zoltan Somogyi wrote:

> What should be reviewed is not so much the diff, as the new contents
> of pretty_printer.m. I am after feedback mostly on the new
> NOTES_TO_IMPLEMENTORS I added. Acting on those would break
> compatibility with callers (which this diff does NOT do), but the results
> may be worth it.

I think it's ok to break backwards compatibility in the case of this
module.

> +    % NOTE_TO_IMPLEMENTORS: XXX The name of this predicate is wrong.
> +    % NOTE_TO_IMPLEMENTORS: This name should be attached to the predicate
> +    % NOTE_TO_IMPLEMENTORS: that is currently named write_doc/4; THIS predicate
> +    % NOTE_TO_IMPLEMENTORS: should be named something like write_doc_general.
> +    % NOTE_TO_IMPLEMENTORS: Note that "stream" here has two related but NOT
> +    % NOTE_TO_IMPLEMENTORS: IDENTICAL meanings (the stream typeclass, and
> +    % NOTE_TO_IMPLEMENTORS: io.output_stream), and thus using it for this name
> +    % NOTE_TO_IMPLEMENTORS: is more confusing than helpful.

I suggest renaming write_doc_to_stream/9 to put_doc/9, since that name
is consistent with the way predicates that write to string writer
streams are named in other modules (e.g. stream.string_writer).  It
should also be sufficiently distinct from write_doc/4.

Julien.


More information about the reviews mailing list