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

Provide defaults for manifest and archive ports of p:archive? #239

Closed
xatapult opened this issue Oct 16, 2019 · 13 comments
Assignees

Comments

@xatapult
Copy link
Contributor

@xatapult xatapult commented Oct 16, 2019

The way the p:archive step is defined now means you always have to provide connections for the manifest and archive input ports, even when you don't need them (which happens, quite often I think, especially the archive port). So you'll have to bind them to p:empty now.

Shall we therefore provide these ports with a default p:empty connection?

One problem (probably) (this one is for @ndw): Can the spec formatting pipeline handle default connections on input ports in a step specification?

@ndw

This comment has been minimized.

Copy link
Collaborator

@ndw ndw commented Oct 16, 2019

My first reaction to this is pretty negative. I think it's going to be very confusing. If we provide a default binding, it will only apply if there isn't a default binding. So lots of the time, you'll get whatever happens to be on the DRP even if we specify a default binding. When that isn't appropriate, you'll still have to make the binding explicit.

Or am I misunderstanding something?

@xatapult

This comment has been minimized.

Copy link
Contributor Author

@xatapult xatapult commented Oct 16, 2019

I don't quite get you I'm afraid, I do not understand what this has to do with the DRP.

Here is what I mean: When you invoke p:archive you now always have to provide bindings for the archive and manifest port, even when you don;t need them, which will be quite often. So you always have to write:

<p:archive>
  <p:with-input port="archive">
   <p:empty/>
  </p:with-input>
 <p:with-input port="manifest">
   <p:empty/>
  </p:with-input>
</p:archive>

Especially for the archive port, an empty connection will very often be the case. So you have to write the p:with-input port="archive part over and over again. Not a world class problem but we have means to make life simpler, don't we?

So why not just define the archive and manifest ports of p:archive with a default p:empty connection?

@ndw

This comment has been minimized.

Copy link
Collaborator

@ndw ndw commented Oct 16, 2019

Because I think we understand the semantics differently. Consider:

<p:xslt ...>...</p:xslt>

<p:archive>...</p:archive>

The default bindings for archive and manifest would not apply here. Because the XSLT step provides a default readable port, that's the input on both of those ports.

@xml-project

This comment has been minimized.

Copy link
Contributor

@xml-project xml-project commented Oct 16, 2019

@ndw wrote:

The default bindings for archive and manifest would not apply here. Because the XSLT step provides a default readable port, that's the input on both of those ports.

Would you mind to explain this please, because I do not get it. Ports archive and manifest are non-primary port, so port result of p:xslt to my reading DOES NOT provide the DRP for the ports on manifest. Where is my mistake?

@gimsieke

This comment has been minimized.

Copy link
Contributor

@gimsieke gimsieke commented Oct 16, 2019

Can't we say that unconnected non-primary ports will always connect to p:empty if no explicit or default connection is given? (If sequence=false, p:empty will cause a dynamic error, but this is Ok).

@xml-project

This comment has been minimized.

Copy link
Contributor

@xml-project xml-project commented Oct 16, 2019

Don‘t you think that is to wide a solution because it would apply to every user-defined step ever written. I would tend to Eriks suggestion to do this step wise.
But if I understand @ndw right, we all completely misunderstand the problem.

@ndw

This comment has been minimized.

Copy link
Collaborator

@ndw ndw commented Oct 16, 2019

I think I misunderstood, sorry. I don't know why I keep forgetting that non-primary input ports aren't connected to the DRP by default. (I think that's so confusing, I guess that's why I keep forgetting.)

So, yes, I guess we could make those inputs p:empty by default.

I do not have an informed opinion about whether or not that is a good idea at the moment.

@xatapult

This comment has been minimized.

Copy link
Contributor Author

@xatapult xatapult commented Oct 17, 2019

I agree with @xml-project that defaulting a p:empty for every non-primary port would go a bit too far and will probably have side-effects we don't foresee yet.

So shall I change p:archive to bind these ports p:empty by default?

@xml-project

This comment has been minimized.

Copy link
Contributor

@xml-project xml-project commented Oct 17, 2019

👍 I am not sure whether we have any other step in the standard or optional libraries with non-primary sequence ports. Binding them to p:empty by default might be an option too.

@xatapult

This comment has been minimized.

Copy link
Contributor Author

@xatapult xatapult commented Oct 17, 2019

Consensus or discuss in Cologne?

@xatapult xatapult self-assigned this Oct 17, 2019
@ndw

This comment has been minimized.

Copy link
Collaborator

@ndw ndw commented Oct 17, 2019

FWIW, p:archive, p:insert, p:pack, p:validate-with-nvdl and p:validate-with-xml-schema all have secondary input ports that accept a sequence.

For at least p:insert and p:pack, having the default be empty seems unhelpful.

Given that we've already accepted the semantics for secondary input ports with defaults, I'm not going to object to making the secondary ports on p:archive default to p:empty.

(As to Erik's earlier question about whether or not the spec formatter will do the right thing with the tableaux, I'll fix it if it doesn't.)

@gimsieke

This comment has been minimized.

Copy link
Contributor

@gimsieke gimsieke commented Oct 17, 2019

Consensus to make the secondary ports (only) on p:archive default to p:empty

@xatapult

This comment has been minimized.

Copy link
Contributor Author

@xatapult xatapult commented Oct 17, 2019

(As to Erik's earlier question about whether or not the spec formatter will do the right thing with the tableaux, I'll fix it if it doesn't.)

I tried locally and a p:empty child of a p:option in a step description simply disappears. So what I'm going to do is to add these anyway and add it in prose.

xatapult added a commit to xatapult/3.0-steps that referenced this issue Oct 17, 2019
xatapult added a commit that referenced this issue Oct 18, 2019
#239 provide defaults for non-primary input ports
@xatapult xatapult closed this Oct 18, 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.