[m-rev.] for review: add xmlable typeclass to term_to_xml module

[Ian MacLarty]

On Mon, 25 Jul 2005, Julien Fischer wrote:

>
> On Sat, 23 Jul 2005, Ian MacLarty wrote:
>
> > For review by anyone.
> >
> > Estimated hours taken: 6
> > Branches: main
> >
> > Add a new method to convert terms to XML using a typeclass.
> > The new method is more flexible than the previous method, but DTDs cannot
> > be automatically generated using the new method.
> >
>
> The log message should include a little more information on
> what is more flexible about the new method.
>

Okay:

The new method is more flexible than the previous method in that values can be
converted to arbitrary elements with arbitrary attributes and children
(the previous method mapped a functor to one element with limited attributes).
DTDs cannot be automatically generated using the new method.

> > +% This module provides two mechanisms whereby Mercury terms can be converted to
> > +% XML documents.
>
> I suggest:
>
> 	This module provides two mechanisms for converting Mercury terms
> 	to XML documents.
> ...

Okay.

>
> > +% Method 1
> > +% --------
> > +% The first method requires a type to be an instance of the xmlable typeclass
> > +% before values of the type can be written as XML.
> > +% Members of the xmlable typeclass must implement a to_xml method which
> > +% maps values of the type to XML elements.
>
> That seems redundant.  Members of a typeclass must implement the methods
> by definition.
>

I don't see the problem.  Do you think it reads badly?  It is slightly
redundant, but it gives new information: i.e. what the method names is
and what the method does.

> > +% The XML elements may contain arbitrary children, comments and data.
> > +%
> > +% Method 2
> > +% --------
> > +% The second method is less flexible than the first, but it allows for the
> > +% automatic generation of a DTD.
> > +% In the second method each functor in a term is given a corresponding
>
> Delete 'In the second method'.
>

Okay.

> > +% well-formed element name in the XML document according to a mapping.  Some
> > +% predefined mappings are provided, but user defined mappings may also be used.
> > +%
> > +% Method 1 vs. Method 2
> > +% ---------------------
> > +%
> > +% Method 1 allows values of a specific type to be mapped to arbitrary XML
> > +% elements with arbitrary children and arbitrary attributes.
> > +% In method 2 each functor in a term can be mapped to only one XML element.
> > +% Method 2 also only allows a selected set of attributes.
> > +% In method 2 a DTD can be automatically generated.  In method 1 DTDs cannot
> > +% be automatically generated.
> > +%
> > +% Method 1 is useful for mapping a specific type to XML,
> > +% for example mapping terms which represent mathematical expressions to
> > +% MathML.
> > +% Method 2 is useful for mapping arbitrary terms of any type to XML.
> > +%
> > +% In both methods the XML document can be annotated with a stylesheet
> > +% reference.
>
> Presumably once we have typeclass reflection you could use both methods,
> using the non-typeclass one as a fallback method.
>

I suppose.  Though I can't think of a situation where that would be useful
outside of the debugger.

Perhaps Peter Ross could give an example...

> >  :- import_module type_desc.
> >
> >  %-----------------------------------------------------------------------------%
> > +% Method 1 interface
> > +%
>
> Please use the section heading format in the coding standard.
>
> 	%---------------------------
> 	%
> 	% Method 1 interface
> 	%
>

Okay.

> > +
> > +	% Instances of this typeclass can be converted to XML.
> > +	%
> > +:- typeclass xmlable(T) where [
> > +	func to_xml(T::in) = (xml::out(xml_doc)) is det
> > +].
>
> Is there a reason the method delcaration just isn't:
>
> 	func to_xml(T) = xml
>

Yes.  An XML document must have one element at its root (its root can't be
CDATA for example).  See the xml_doc inst below which you said seemed
redundant.

> I prefer either 'to_xml' or just 'xml' as names for the typeclass.
>

I don't mind to_xml, but xml is too vauge.  Are there any other opinions?

> > +			% will be replaced by `<', `>', `&', `''
> > +			% and `"' respectively.
> > +	;	data(string)
> > +
> > +			% Data to be enclosed in `<![CDATA[' and `]]>' tags.
> > +			% Any occurances of `]]>' in the data will be
> > +			% converted to `]]>'.
> s/occurances/occurrences/
>

Fixed.

> > +	;	cdata(string)
> > +
> > +			% An XML comment.  The comment should not
> > +			% include the `<!--' and `-->'.  Any occurances of
> s/occurances/occurrences/
>

Fixed.

> > +			% `--' will be replaced by ` - '.
> > +	;	comment(string)
> > +
> > +			% An entity reference.  The string will
> > +			% have `&' prepended and `;' appended before being
> > +			% output.
> > +	;	entity(string)
> > +
> > +			% Raw XML data.  The data will be written out verbatim.
> > +	;	raw(string).
> > +
> > +	% An XML document must have an element at the top-level.
> > +	%
> > +:- inst xml_doc
> > +	--->	elem(
> > +			ground,	% element_name
> > +			ground,	% attributes
> > +			ground	% children
> > +		).
>
> This inst seems redundant.
>

This inst says that to be an XML document a piece of XML must have exactly
one root element.  See the to_xml method above.

>
> > +	% write_xml_doc(Stream, Term, !IO).
> > +	% Same as write_xml_doc/3, but use the given output stream.
> > +	%
> > +:- pred write_xml_doc(io.output_stream::in, T::in, io::di, io::uo) is det
> > +	<= xmlable(T).
> > +
> > +	% write_xml_doc(Term, MaybeStyleSheet, MaybeDTD, !IO).
> > +	% Write Term to the current output stream as an XML document.
> > +	% Term should be an instance of the xmlable typeclass.
>
> s/should/must/ (although that sentence is redundant; the predicate
> declaration says as much.)
>

Okay.  Removed the sentence.

> > +	% MaybeStyleSheet and MaybeDTD specify whether or not a stylesheet
> > +	% reference and/or a DTD should be included.
> > +	% Using this predicate only external DTDs can be included, i.e.
> > +	% a DTD cannot be automatically generated and embedded
> > +	% (that fearure is available only for method 2 -- see below).
> > +	%
> /fearure/feature/
>

I had already got that one :-)

> > +
> > +	% Same as write_xml_header/3, but use the given output stream.
> > +	%
> > +:- pred write_xml_header(io.output_stream::in, maybe(string)::in, io::di,
> > +	io::uo) is det.
> > +
> > +%-----------------------------------------------------------------------------%
> > +% Method 2 interface
> > +%
>
> Fix the section heading here.
>

Done.

> > +			% The original functor name as returned by
> > +			% deconstruct.deconstruct/5.
> > +	--->	functor
> > +			% The field name if the functor appears in a
> > +			% named field (If the field is not named then this
> > +			% attribute is omitted.
>
> You've left out the closing parenthesis there.
>

Oops.

> > +write_xml_doc(X, !IO) :-
> > +	write_xml_doc(X, no_stylesheet, no_dtd, !IO).
> > +
>
> I suggest s/X/XML/ or s/X/Doc/ there and below.
>

X is not XML or a Doc.  It's just some Mercury value.
Have renamed it to Term.

> >
> > +	% The following type is used to decide if an entity should be
> > +	% formated (i.e. be indented and have a newline at the end).
>
> s/formated/formatted/
>

Fixed.

> > +	% We do not format sibling entities if there is anything besides
> > +	% elements, Cdata or comments in the siblings, since then whitespaces
> > +	% are more likely to be significant.
>
> I suggest:
>
> 	We do not format sibiling entities if they contain anything
> 	besides ....

That's not right.  The siblings themselves can contain anything, but the set of
siblings must not contain anything besides elements, Cdata or comments.
Reworded to:
	% We do not format an entity if any of its siblings is anything
	% besides an element, a CDATA entity or a comment, since then
	% whitespaces are more likely to be significant.

Thanks for that.

Do you still have more comments?

Ian.

--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------