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

Add support for multiple input files #3286

Draft
wants to merge 25 commits into
base: develop
Choose a base branch
from
Draft

Conversation

jelovirt
Copy link
Member

@jelovirt jelovirt commented Apr 20, 2019

Add support for multiple input files per DITA-OT process.

Multiple input files should work the same was as a single input file with additional resource files, except all resource files are treated as processing-role="normal".

Interface changes

In the CLI, add support for multiple --input arguments.

$ dita -i maps/content.ditamap -i maps/keys.ditamap …

For project files, add support for multiple input fields.

contexts:
- input:
  - maps/content.ditamap
  - maps/keys.ditamap

Add args.inputs Ant property:

<pathconvert property="args.inputs">
  <pathelement location="maps/content.ditamap"/>
  <pathelement location="maps/keys.ditamap"/>
</pathconvert>

@jelovirt jelovirt added feature New feature or request priority/medium Medium (or unknown) priority issue preprocess preprocess2 labels Apr 20, 2019
@jelovirt jelovirt self-assigned this Apr 20, 2019
@jelovirt jelovirt force-pushed the feature/multiple-inputs branch 3 times, most recently from da464ee to 860505a Compare April 20, 2019 16:11
@jelovirt
Copy link
Member Author

jelovirt commented Apr 21, 2019

@robander args.input can be either system path or a URI, relative or absolute. I'd prefer to make args.inputs (notice the plural) always be absolute, to make things more simple. If a API in front of the Ant API allows relative paths, then it's up to that layer to resolve those to absolute paths.

However, because Ant doesn't have support for array types, do we want to use properties to be begin with for args.inputs? We could make it a resource collection instead, because Ant resources can be either files or e.g. URLs. But, I'm worried it will be too difficult to use and unnecessarily binds use to Ant.

So, how to handle both URIs and file paths in the same property, because there is no obvious separator character. For paths we can just use the system separators : or ;, but there is no single correct separator for URIs. We can use a space, but then that would not work with paths.

Would it be OK if we defined args.inputs as a list of URIs, separated by e.g. spaces? Either absolute URIs file:///maps/root.ditamap file:///maps/keys.ditamap or with just absolute path parts /maps/root.ditamap /maps/keys.ditamap.

Or have args.inputs and args.inputs.uri, where the former one is system paths separated by system separator and the latter is URIs separated by a space character?

@robander
Copy link
Member

@jelovirt Thinking about how I'm most likely to use this in our larger environment, I think URIs would be OK as long as it can take the absolute path:
/test/inputone.ditamap /test/bad%20name.ditamap

I think over time more and more people interact with the command through editors or build scripts, which should be able to hide or translate the values appropriately. Basically, adding that layer ahead of the build to translate paths in a more friendly way.

I'm also thinking about this (for now) as a more complex usage, where it seems more acceptable to enforce stricter rules about the input - maybe others would see it differently?

@jelovirt jelovirt force-pushed the feature/multiple-inputs branch 2 times, most recently from 9756dc9 to af1d1dc Compare April 29, 2019 16:22
@jelovirt
Copy link
Member Author

This could resolve #1697

@jelovirt jelovirt marked this pull request as ready for review May 13, 2019 16:12
@jelovirt jelovirt requested a review from robander May 13, 2019 16:12
@@ -326,7 +327,9 @@ public Arguments processArgs(final String[] arguments) {
targets.addElement(arg);
}
}
if (!inputs.isEmpty()) {
if (inputs.size() > 1) {
definedProps.put("args.inputs", String.join(" ", inputs));
Copy link
Member

Choose a reason for hiding this comment

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

Is this going to cause issues with inputs that have spaces in the file name or path? Or do those get normalized to %20 before we reach this point?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's up to the client to pass valid URIs. So if the end user user inputs an invalid URI (a URI with a space) to the client, the client will need to convert it to a valid URI (by escaping the space with a %20) when setting args.inputs property.

@jelovirt
Copy link
Member Author

Dropping from 3.4 release due to lack of interest. Maybe in 4.0

@jelovirt jelovirt marked this pull request as draft April 9, 2020 15:06
@jelovirt jelovirt force-pushed the feature/multiple-inputs branch 2 times, most recently from e807467 to e936196 Compare August 8, 2020 07:15
@jelovirt jelovirt force-pushed the feature/multiple-inputs branch 2 times, most recently from c886ade to 836cd1f Compare October 12, 2020 17:39
Signed-off-by: Jarno Elovirta <[email protected]>
Signed-off-by: Jarno Elovirta <[email protected]>
Signed-off-by: Jarno Elovirta <[email protected]>
Signed-off-by: Jarno Elovirta <[email protected]>
Signed-off-by: Jarno Elovirta <[email protected]>
Signed-off-by: Jarno Elovirta <[email protected]>
Signed-off-by: Jarno Elovirta <[email protected]>
Signed-off-by: Jarno Elovirta <[email protected]>
Signed-off-by: Jarno Elovirta <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request preprocess preprocess2 priority/medium Medium (or unknown) priority issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants