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

Fix 129 #131

Merged
merged 3 commits into from Sep 26, 2017
Merged

Fix 129 #131

merged 3 commits into from Sep 26, 2017

Conversation

ndw
Copy link
Contributor

@ndw ndw commented Sep 2, 2017

Here's my attempt to address #129.

<emphasis>in the XProc namespace</emphasis>), may appear on an element.
If it does, the specified value is used as the expand text value
for the descendants of the element on which it occurs. The
<tag class="attribute"> will not appear in the inline content.</tag>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<tag class="attribute">p:expand-text</tag> will not appear in the inline content.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

<para>(A <tag class="attribute">p:expand-text</tag> attribute on an element in the
XProc namespace is an error. If it occurs on other elements that are not
descendants of <tag>p:inline</tag>, the attribute has no effect.)</para>

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion:
If it occurs below another p:inline within the topmost p:inline, that is, within an inlined pipeline, it will not immediately be used as the expand-text value of the element on which it occurs and it will appear on the inline content.
(It’s better to spell out known fringe cases in adavance. We could alternatively note that people should avoid <tag class="attribute">p:expand-text</tag> within inlined pipelines because what happens then is implementation-dependent.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gimsieke I get (and support) the idea of your suggestion (treating p:inline as a descendant of a p:inline differently), but I can not see how this is reflected by the suggested prose in the specs. Could you please elaborate your idea.

Since the problem with @expand-text we dealing with here, only arises when one wants to construct an XProc pipeline, may be we should mark this and not bother more typical use cases.
Could we solve this problem in a more transparent way if we abandon the idea of @expand-text having a boolean value? When we say it has three values we could cover the special case with @expand-text="pipeline-construction" (for something like that) to explicitly mark the special case.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t like 'true', 'false' and something else because it looks like a boolean but it isn’t. There’s no precedent to this in XProc AFAIK, therefore it may come as a surprise to authors.
Apart from this, let me try to phrase it differently.
“If p:inline contains an XProc pipeline that in turn contains another p:inline, any @p:expand-text within the inner p:inline is treated like an ordinary attribute; that is, neither will its value influence text expansion nor will it be removed from the inlined XProc pipeline.”

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Copy link
Contributor

@gimsieke gimsieke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see the two comments

@xml-project
Copy link
Member

xml-project commented Sep 5, 2017

Ok, I got it and I think you are right. I think the rephrased suggestion is much clearer than the original prose.
We just have to say, what it means that a pipeline "contains on XProc pipeline". But I think that can easily be expressed as "any child of an element with name 'declare-step' or 'pipeline' within the XProc namespace".
If we do it this way, the special treat is invoke by elements name and it's namespace, so we do not need any special value for @expand-text, but can keep a boolean value.
Good suggestion!

Copy link
Member

@xml-project xml-project left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can merge this with Gerrit's additional suggestions, so @expand-text in XProc pipelines as child of a p:inline is preserved and the TVTs are not expanded.

@ndw ndw merged commit 934489b into xproc:master Sep 26, 2017
@ndw ndw deleted the fix-129 branch September 26, 2017 07:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants