Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attempt to fix 197 #198

Merged
merged 2 commits into from Sep 1, 2019

Conversation

@xml-project
Copy link
Contributor

commented Aug 27, 2019

Attempt to fix #197

  • I used "sort-key" because it feels more natural to me then "select". The only "select" in the step library I found is on p:filter and there it really select a portion of a document. If I am the only one arguing against "select" I am perfectly prepared to give in.
  • My proposal keeps option data-type but removes the default value, so there a two ways to enforce a special sort mechanism:
<p:sort sort-key="xs:integer(substring(., 1, 5))" />

or

<p:sort sort-key="substring(., 1, 5)" data-type="numeric" />

As far as I can see, data-type on xsl:sort is only for legacy reason, so if you opt to remove data-type, I am perfectly happy with this.

@xml-project xml-project requested review from ndw, gimsieke and eriksiegel Aug 27, 2019

@eriksiegel
Copy link
Contributor

left a comment

Perfect!

@xml-project

This comment has been minimized.

Copy link
Contributor Author

commented Aug 27, 2019

@ndw Any objections especially concerning implementability?

@ndw

This comment has been minimized.

Copy link
Collaborator

commented Aug 27, 2019

No objections on implementability. I would have expressed a marginal preference for select over sort-key because it's more consistent with xsl:sort, but I'm fine with sort-key.

One proposal: if we're going to keep data-type, let's make it a QName and specify that the values 'text' and 'number' in no-namespace have the legacy behavior we currently get with the string values. This would allow someone to specify:

<p:text-sort data-type='xs:dateTime'/>

which seems better than forcing the sort-key='xs:something(.)' expression for all values except string and number.

This is also consistent with what XSLT 3.0 did with xsl:sort.

@xml-project

This comment has been minimized.

Copy link
Contributor Author

commented Aug 27, 2019

@ndw I thought about this, but the XSLT 3.0 specs says:

If the data-type attribute has any other effective value, then this value must be an EQName denoting an expanded QName with a non-absent namespace, and the effect of the attribute is implementation-defined.

So we need to spell out ourselves the behaviour we prefer. I am not sure, it is worth it. Any suggestions?

@xml-project

This comment has been minimized.

Copy link
Contributor Author

commented Aug 28, 2019

Since I do not know, how to go on, let's do a little survey:

(A) Change 'data-type' as in XSLT 3.0 and spell out, which behaviour is expected.

(B) Leave 'data-type' as it is in the PR.

(c) Remove 'data-type' completely.

Please take a short moment and give me a letter.
Thanks.

@gimsieke

This comment has been minimized.

Copy link
Contributor

commented Aug 28, 2019

Slight preference for (A), with support for the http://www.w3.org/2001/XMLSchema namespace URI (others implementation-defined). If it’s not too expensive implementation-wise.

@ndw

This comment has been minimized.

Copy link
Collaborator

commented Aug 29, 2019

Per 29 Aug 2019 editor's call: remove the 'data-type' attribute.

@xml-project xml-project requested review from eriksiegel, ndw and gimsieke and removed request for ndw and gimsieke Aug 31, 2019

@xml-project xml-project merged commit feed9a9 into xproc:master Sep 1, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@xml-project xml-project deleted the xml-project:fix-197 branch Sep 1, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.