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

Updates to p:choose #650

Merged
merged 5 commits into from Nov 30, 2018

Conversation

Projects
None yet
2 participants
@ndw
Contributor

ndw commented Nov 30, 2018

  1. Make it explicit that after a p:when is selected, no subsequent test expressions are evaluated. Fix #647
  2. Make it explicit that p:otherwise is optional and describe what happens if it isn't present.
  3. Simplify the content model of p:choose in the schema.

Right. So it's possible that item 3 will be controversial. The previous content model guaranteed that a p:choose would contain at least one p:when or p:otherwise. However, we have XS0074 that doesn't appeal to the schema so it isn't necessary. I think the new content model is sufficiently simpler that it's worth losing the grammar constraint. But I won't argue if anyone disagrees.

ndw added some commits Nov 30, 2018

@ndw ndw requested review from eriksiegel, gimsieke and xml-project Nov 30, 2018

@ndw

This comment has been minimized.

Contributor

ndw commented Nov 30, 2018

I wanted to fix #645 as well, but it was going to have conflicts with this PR, so I just added the fixes to this PR. Not ideal, but seemed better than waiting for approval of this one before proceeding.

@xml-project

Not sure I like the simplified content model, because (to my reading), now
<p:choose />
is valid.

@xml-project

I agree with this PR, but I think one point we have discussed is still open: Is the p:finally executed even if an error in p:catch occurs. I would argue: Yes. p:finally is ALWAYS executed and the error bubbles up afterward.

@ndw

This comment has been minimized.

Contributor

ndw commented Nov 30, 2018

I agree that p:finally is always executed.

I'll put the content model back. shrug.

Like I said, error XS0074 says that at least one of when or otherwise is required; we don't need to enforce that in the grammar. But the grammar isn't so complicated that I think it matters.

@ndw

This comment has been minimized.

Contributor

ndw commented Nov 30, 2018

Actually, we already say:

Irrespective of whether the initial subpipeline succeeds or fails, if any recovery pipelines are selected, and whether they succeed or fail, the p:finally block is always run after all other processing of the p:try has finished.

But since I'm mucking about, I'll try to clarify that.

@ndw ndw closed this Nov 30, 2018

@ndw ndw reopened this Nov 30, 2018

@ndw

This comment has been minimized.

Contributor

ndw commented Nov 30, 2018

I did not mean to close this one. Fumble-fingered the button, I guess.

@xml-project

This comment has been minimized.

Contributor

xml-project commented Nov 30, 2018

Irrespective of whether the initial subpipeline succeeds or fails, if any recovery pipelines are selected, and whether they succeed or fail, the p:finally block is always run after all other processing of the p:try has finished.

Yes, you are right. Reading this passage in our context it is clear, that p:finally is always executed. Skipped this. Sorry!

@ndw

This comment has been minimized.

Contributor

ndw commented Nov 30, 2018

No worries. I missed it too on a quick reading, so I've pulled it out into a big long list of cases with clear descriptions (I hope) of what happens and what errors must be reported.

@ndw ndw merged commit e976c71 into xproc:master Nov 30, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment