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

Option with name "temp-dir" overrides temp-dir variable #44

Closed
bertfrees opened this issue Oct 18, 2016 · 7 comments
Closed

Option with name "temp-dir" overrides temp-dir variable #44

bertfrees opened this issue Oct 18, 2016 · 7 comments
Assignees
Milestone

Comments

@bertfrees
Copy link
Member

bertfrees commented Oct 18, 2016

Not sure if this is a bug or a feature, but because of it I can't define an option named "temp-dir" before another option that uses "$temp-dir". To be able to define one before the other is important if you want to use nested scenarios.

For example:

<x:scenario label="_">
  <x:call step="sbs:dtbook-to-pef">
    ...
    <x:option name="temp-dir" select="concat($temp-dir,'temp-dir/')"/>
  </x:call>

  <x:scenario label="foo">
    <x:call>
      <x:option name="brf-output-dir" select="concat($temp-dir,'output-dir/foo/')"/>

It does work if I just do:

<x:scenario label="_">
  <x:call step="sbs:dtbook-to-pef">
    ...
  </x:call>

  <x:scenario label="foo">
    <x:call>
      <x:option name="brf-output-dir" select="concat($temp-dir,'output-dir/foo/')"/>
      <x:option name="temp-dir" select="concat($temp-dir,'temp-dir/')"/>

A workaround (but not really what I want) is to set the temp-dir option to "$temp-dir":

<x:option name="temp-dir" select="$temp-dir"/>
@josteinaj
Copy link
Member

I've had the same problem. Hmm, I thought this was fixed in #4 but it seems not

Some testing suggests that this is how calabash handles subsequent p:option declarations. All preceding p:options are in scope for consecutive p:option declarations:

<?xml version="1.0" encoding="UTF-8"?>
<p:declare-step xmlns:p="http://www.w3.org/ns/xproc" xmlns:c="http://www.w3.org/ns/xproc-step" xmlns:px="#" version="1.0">

    <p:serialization port="result" indent="true"/>
    <p:output port="result"/>

    <p:variable name="temp-dir" select="'original temp-dir'"/>

    <p:declare-step type="px:substep">
        <p:output port="result"/>

        <p:option name="temp-dir" select="'new temp-dir'"/>
        <p:option name="output-dir" select="$temp-dir"/>

        <p:in-scope-names name="in-scope-names"/>
        <p:identity>
            <p:input port="source">
                <p:pipe port="result" step="in-scope-names"/>
            </p:input>
        </p:identity>
    </p:declare-step>

    <px:substep/>

</p:declare-step>

Output:

<c:param-set xmlns:c="http://www.w3.org/ns/xproc-step">
   <c:param name="output-dir" namespace="" value="original temp-dir"/>
   <c:param name="temp-dir" namespace="" value="new temp-dir"/>
</c:param-set>

So the simplest solution maybe is to say that this is just how it's supposed to behave? Maybe display a warning if <x:option name="temp-dir"> has following options?

Alternatively I could implement a workaround by resolving all the options prior to inserting them into the <p:with-option ..>s. Maybe that would be best?

@bertfrees
Copy link
Member Author

bertfrees commented Oct 19, 2016

Oh, #4 is exactly the same issue, yes. I am using v1.1.0. It looks to me like your test isn't really testing the issue.

You could say that this is just how it's supposed to behave, although it's a side effect of the current implementation. The specification doesn't give anything away about temp-dir being an option and I like that.

Another solution could be to replace the "$temp-dir" variable for the "param" and "option" elements with a "base-uri" attribute like on the "document" element. Don't know if that makes a difference at all?

@bertfrees
Copy link
Member Author

<x:option name="brf-output-dir" select="concat($temp-dir,'output-dir/foo/')"/>

would become:

<x:option name="brf-output-dir" select="resolve-uri('output-dir/foo/')" base-uri="temp-dir"/>

@josteinaj
Copy link
Member

josteinaj commented Oct 19, 2016

The context for the xpath expressions are set to a document with an xml:base pointing to the xprocspec document, so that resolve-uri resolves relative to the xprocspec document.

For instance, this should resolve properly:

<x:option name="option-name" select="resolve-uri('../resources/book.xml')">

So a context element is inserted when the step invocation is compiled:

<p:with-option name="option-name" select="resolve-uri('../resources/book.xml')">
    <p:inline>
        <context xml:base="file:/home/jostein/daisy-pipeline/pipeline-modules/xprocspec/xprocspec/src/test/xprocspec/tests/document-3.resources/document-3.xprocspec.skip-ci"/>
    </p:inline>
</p:with-option>

I could set the select expression to select="/*/@option-name", and add the value of the option to the context element instead. That way, I could iterate over all the options, and use a <p:add-attribute match="/*" select="xpath expression here"> to evaluate the xpaths instead.

@bertfrees
Copy link
Member Author

bertfrees commented Oct 19, 2016

OK, that could work (I think). What about the suggestion with the base-uri attribute? Doesn't that look a bit cleaner (compared to the current specification)? The only reason for using the "$temp-dir" variable is for resolving URIs anyway (and you'll never need to resolve against both the temp-dir and the base-uri of the xprocspec file).

@josteinaj
Copy link
Member

Ah, I see what you mean now. Sorry I missed that part.

Yeah, I guess that would work. Seems cleaner indeed. 👍

@bertfrees
Copy link
Member Author

See also the documentation of p:with-option (paragraph that starts with "All in-scope bindings for the step...").

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

No branches or pull requests

2 participants