[m-rev.] For review: Convert Mercury terms to XML

[Ian MacLarty]

On Mon, Dec 06, 2004 at 04:49:32PM +1100, Julien Fischer wrote:
> > +
> > +:- type maybe_stylesheet
> > +	--->	with_stylesheet(string, string)	% stylesheet type and href.
> You could perhaps make stylesheet_type and stylesheet_href field names
> here.
> 
Good idea:

+:- type maybe_stylesheet
+	--->	with_stylesheet(
+			stylesheet_type	:: string, 
+			stylesheet_href	:: string)
+	;	no_stylesheet.
+

> > +	% write_xml_doc_cc(Stream, Term, MaybeStyleSheet, MaybeDTD, DTDResult,
> > +	%	!IO).
> > +	% Same as write_xml_doc/5 except write the XML doc to the given
> > +	% output stream.
> > +	%
> > +:- pred write_xml_doc_cc(io.output_stream::in, T::in, maybe_stylesheet::in,
> > +	maybe_dtd::in, dtd_generation_result::out, io::di, io::uo) is cc_multi.
> > +
> > +	% True if the given type doesn't have multiple top level functors.
> > +	%
> > +:- pred ok_to_generate_dtd(type_desc::in) is semidet.
> s/doesn't/does not/
> 
Okay.

> > +	% Write XML elements for the given term and all it's descendents,
> > +	% using IndentLevel as the initial indentation level (each
> > +	% indentation level is one space character).  No <?xml ... ?>
> > +	% header will be written.  Non canonical terms will be handled
> > +	% according to the value of NonCanon.  See the deconstruct
> > +	% library for more information on this argument.
> s/deconstruct library/deconstruct module in the standard library/
> 
Okay.
> > +	%
> > +:- pred write_xml_element(deconstruct.noncanon_handling, int, T, io, io).
> > +:- mode write_xml_element(in(do_not_allow), in, in, di, uo) is det.
> > +:- mode write_xml_element(in(canonicalize), in, in,  di, uo) is det.
> > +:- mode write_xml_element(in(include_details_cc), in, in, di, uo) is cc_multi.
> > +:- mode write_xml_element(in, in, in, di, uo) is cc_multi.
> > +
> Somewhere in the interface you should have a comment mentioning that
> many of theses predicates can throw the xml_internal_error (or whatever
> it is) exception.
> 
If it does ever get thrown then there's a bug somewhere, so are you sure?

> > +%-----------------------------------------------------------------------------%
> > +%
> > +% Some reserved element names.  Reserved element names all start with a
> > +% capital letter so as not to conflict with a mangled element name.
> > +%
> > +
> > +:- func reserved_prefix = string.
> > +
> > +	% A prefix for functors that start with a capital letter or
> > +	% a non-letter.
> > +	%
> > +reserved_prefix = "Tag_".
> > +
> Move the comment so that it is above the function declaration.
> 
Okay.

> > +:- pred common_mercury_functor(string, string).
> > +:- mode common_mercury_functor(in, out) is semidet.
> > +:- mode common_mercury_functor(out, in) is semidet.
> > +
> > +	% These should all start with a capital letter so as not to
> > +	% conflict with a mangled name.
> > +	%
> > +common_mercury_functor("[|]", "List").
> > +common_mercury_functor("[]", "Nil").
> > +common_mercury_functor("{}", "Tuple").
> > +
> > +	% A general element for types whos structure we do not generate
> > +	% DTD rules for.
> > +	%
> s/whos/whose/
> 
Fixed.

> > +%-----------------------------------------------------------------------------%
> > +%
> > +% Mangling functions.
> > +%
> > +% We use the following mangling scheme to create well formed element names
> > +% that do not begin with a capital letter (capitals are used for reserved
> > +% elements).
> > +%
> > +% If the string to be mangled begins with a capital letter then we prefix it
> > +% with another string reserved for this purpose.  Then we replace all
> > +% characters which aren't alpha numeric or underscores with '-' followed by
> > +% the character code.
> > +%
> It would be helpful to give some examples of the mangling scheme in the
> comment here.  Name mangling seems to be one of those things that is
> prone to break easily, so it is important that this bit is documented
> thoroughly.
> 
+%
+% So for example "my-functor!" would become "my-45functor-33" while
+% "MyFunctor" would become "Tag_MyFunctor", presuming we were using
+% "Tag_" as the prefix for strings that started with capital letters.
+%

> > +
> > +get_elements_and_args(TypeDesc, Elements, Functors, MaybeArgTypeLists) :-
> > +	NumFunctors = num_functors(TypeDesc),
> > +	(
> > +		NumFunctors > 0
> > +	->
> > +		FunctorNums = 0`..`(NumFunctors - 1),
> I would leave some space around the `..`
> 
Can do.

> > +:- pred is_array(type_desc::in, type_desc::out) is semidet.
> > +
> > +is_array(TypeDesc, ArgType) :-
> > +	type_ctor_and_args(TypeDesc, TypeCtor, ArgTypes),
> > +	ArgTypes = [ArgType],
> > +	type_ctor_name(TypeCtor) = "array",
> > +	type_ctor_module_name(TypeCtor) = "array".
> > +
> > +:- pragma memo(get_field_names/4).
> > +
> > +:- pred get_field_names(type_desc::in, string::in, int::in,
> > +	list(maybe(string))::out) is det.
> > +
> Is there any reason this couldn't be a function?
> 
No.  I've changed it to a function.

> > +%-----------------------------------------------------------------------------%
> > +%
> > +% The following is done to get around an unimplemented feature where higher
> > +% order terms with more than one mode can't be passed around (so we can't just
> > +% pass write_xml_element_univ to foldl).
> > +%
> I'd put an XXX in front of that comment.
> 
Done.

> > +
> > +:- pred write_xml_element_univ_do_not_allow( int, univ,
> > +	list(maybe(string)), list(maybe(string)), io, io).
> > +:- mode write_xml_element_univ_do_not_allow(in, in, in, out, di, uo)
> > +	is det.
> > +
> You could use predmode syntax here.
> 
Done.

> > +write_xml_element_univ_do_not_allow(IndentLevel, Univ,
> > +		MaybeFieldNames0, MaybeFieldNames, !IO) :-
> > +	write_xml_element_univ(do_not_allow, IndentLevel,
> > +		Univ, MaybeFieldNames0, MaybeFieldNames, !IO).
> > +
> > +:- pred write_xml_element_univ_canonicalize( int, univ,
> > +	list(maybe(string)), list(maybe(string)), io, io).
> > +:- mode write_xml_element_univ_canonicalize(in, in, in, out, di, uo)
> > +	is det.
> > +
> and here
> 
Done.

> > +write_xml_element_univ_canonicalize(IndentLevel, Univ,
> > +		MaybeFieldNames0, MaybeFieldNames, !IO) :-
> > +	write_xml_element_univ(canonicalize, IndentLevel,
> > +		Univ, MaybeFieldNames0, MaybeFieldNames, !IO).
> > +
> > +:- pred write_xml_element_univ_include_details_cc(int, univ,
> > +	list(maybe(string)), list(maybe(string)), io, io).
> > +:- mode write_xml_element_univ_include_details_cc(in, in, in, out, di, uo)
> > +	is cc_multi.
> > +
> and here
> 
Done.

> > +	io.write_string(Element, !IO),
> > +	io.write_string(" (#PCDATA)>\n", !IO),
> > +	write_dtd_field_attlist(Element, !IO).
> > +
> > +	% Write out the DTD entries for all the given types and add the written
> > +	% types to AlreadyDone.  Children types found along the way are added
> > +	% to the 1st argument.  We stop when all the types have had their DTD
> s/1st/first/
Okay.

> > +:- type write_xml_internal_error
> > +	--->	write_xml_internal_error(string, string).
> > +
> Is this name still appropriate now that you've changed the
> module name?  Are you expecting users to (possibly) catch exceptions
> of this type? If so, this type should be exported.
> 
No, if this ever gets thrown then there's a bug somewhere.
I've renamed it to term_to_xml_internal_error.

> > +		io.write_string("(", !IO),
> > +		io.write_list(Elements, "|", io.write_string, !IO),
> > +		io.write_string(")", !IO)
> > +	;
> > +		io.write_list(Elements, "|", io.write_string, !IO)
> > +	).
> > +
> 
> That looks good.  I think you can commit it, although until the
> situation with the tabled functions is sorted out it would be better
> to comment out the memoing pragmas and put an XXX comment with them.
> 

I've done that - thanks for the review.

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
--------------------------------------------------------------------------