XProc Editors Draft 2007-07-19: Appendix A.1 Comments

For all the steps: what is/are the base URI(s) of the output(s)? I 
suggest that it defaults to the base URI of the input. For some steps, 
where that feels inappropriate (such as XSLT, perhaps), I suggest adding 
an option that allows the pipeline author to specify it explicitly.

A.1.2 Delete: You don't need to say anything about the XPath context 
here: the match option is a *pattern*, not an expression, so there's no 
context to specify.

A.1.3 Equal: The fail-if-not-equal option hasn't been described. Why 
return "1" or "0" rather than the more human-readable "true" or "false"?

A.1.4 Error: Perhaps this should have an input port on which you can 
provide the document(s) that contributed to the error. Saying "bad 
document" isn't enough if you don't know what the document actually 
looks like.

A.1.5 Escape Markup: There's a stray 'and' at the beginning of the final 
paragraph.

A.1.6 HTTP Request: s/inlcude-content-type/include-content-type. I think 
  a consistent distinction between things that are options on 
<p:http-request> and things that are attributes on <c:http-request> 
should be made. I suggest that it's based on whether the "option" is 
something that's sent with the HTTP request or not. So username, 
password etc. would be sent with the HTTP request, and should therefore 
be attributes on <c:http-request>, whereas detailed and status-only 
aren't sent with the HTTP request (they modify how the step itself 
works) and should therefore be options on <p:http-request>. Similarly, 
override-content-type should be an option on <p:http-request>.

A.1.6.5 c:body: The example has the namespace declaration for the 
xprox-step namespace on the <c:body> element when it should probably (if 
specified at all) go on the <c:http-request> element.

A.1.6 HTTP Request: I wonder whether the <c:body> elements should have 
xml:base attributes on them to provide a suitable base URI (from the 
Content-Location entity-header field).

A.1.8 Insert: There's no need to specify an XPath context here, since 
this step has a match pattern, not a select expression.

A.1.8 Insert: It would seem more useful to me to have insert add an 
element before or after the matched element itself. This would enable 
you to insert an element as the middle child, or in order (if the source 
was already sorted):

   <p:insert>
     <p:input port="insertion">
       <p:inline><item n="3" /></p:inline>
     </p:input>
     <p:option name="match" value="/list/item[@n &lt; 3][last()]" />
     <p:option name="before-or-after" value="before" />
   </p:insert>

A.1.9 Label Elements: Here, you *do* have to specify the XPath context, 
because the select option contains an expression. (But I think it's just 
the default XPath context, so a reference to that will do fine.)

A.1.10 Load: Here (and maybe elsewhere), the codes of the errors that 
might be generated should be specified.

A.1.12 Parameters: I think the "parameters" port should be the primary 
output port. I don't really see why the "result" output port isn't primary?

A.1.13 Rename, A.1.14 Replace, A.1.15 Set Attributes: Again, the 'match' 
option is a pattern, so no need to specify the XPath context.

A.1.17 Split Sequence: We use hyphenated names elsewhere, so shouldn't 
"notmatched" be hyphenated?

A.1.17 Split Sequence: Saying "The XPath context for the test option 
changes over time." makes me think that the phase of the moon might have 
an impact ;) This sentence could be simply removed.

A.1.18 String Replace: The final paragraph (about the XPath context) 
should be removed. No XPath context is required for the match option, 
and the context node for the replace option has already been specified 
(differently), as being the node that's matched. The context position 
and size need to be specified: I think it would make most sense for the 
context position to be the position of the matched node amongst the 
other matched nodes, and the context size to be the number of matched 
nodes, but giving them both the value 1 wouldn't be too debilitating.

A.1.19 Store: Again, we need an error code for when the store fails.

A.1.19 Store: I think the <c:result> element should give the value of 
the URI at which the document has been stored; this isn't necessarily 
the same as the value of the href attribute (in the case where the URI 
was picked up from the base URI of the source).

A.1.21 Unwrap, A.1.22 Wrap: As before, no need to specify XPath context 
for a pattern.

A.1.22 Wrap: You do have to specify the XPath context for the 
group-adjacent option. This could be context position = position of 
matched node and context size = number of matched nodes, but setting 
both to 1 would be acceptable.

A.1.22 Wrap: It says "two matches are considered adjacent if they are 
either siblings or all intervening siblings are whitespace text, 
comments, or processing instruction nodes." I think it should say "...if 
they are either immediate siblings or..."

A.1.22 Wrap: Should there be something about what nodes can be matched 
(specifically that only nodes that can be children can be matched: not 
the document node, nor attributes).

A.1.23 Wrap Sequence: The XPath context for the group-adjacent option 
needs to be specified (use position = position of document, size = 
number of documents). Also, given the grouping, shouldn't the result be 
a sequence?

A.1.25 XSLT: Is the result a sequence or not? The text says so, the 
<p:declare-step> says not. (I thought not, since it's not possible in 
XSLT 1.0 without extensions, but I'm sure the decision was made at some 
point.)

Jeni
-- 
Jeni Tennison
http://www.jenitennison.com

Received on Monday, 23 July 2007 21:44:24 UTC