Skip to content

Implement a encode/decodeEXI command to the CLI#878

Merged
jadams-tresys merged 1 commit intoapache:mainfrom
jadams-tresys:DAFFODIL-2750-EXI_encode_decode
Nov 29, 2022
Merged

Implement a encode/decodeEXI command to the CLI#878
jadams-tresys merged 1 commit intoapache:mainfrom
jadams-tresys:DAFFODIL-2750-EXI_encode_decode

Conversation

@jadams-tresys
Copy link
Contributor

This command simply encodes an existing XML file to EXI using Exificient. It will do a schema aware encoding if a schema is provided. On the other side the decodeEXI command will decode an existing EXI file to a plain text XML file.

This is a useful feature as there isn't an easy existing way to perform these conversions using a schema aware encoding as it requires the use of the DaffodilXMLEntityResolver for anything more than a basic schema.

DAFFODIL-2750

Copy link
Member

@stevedlawrence stevedlawrence left a comment

Choose a reason for hiding this comment

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

+1 Looks pretty straightforward. Just some minor suggestions.

Copy link
Contributor

@tuxji tuxji left a comment

Choose a reason for hiding this comment

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

I would like to request you make Steve's suggested changes and add additional test cases for error messages. I also would like to see #873 merged first since your changes are much smaller and the conflicts will be easier to fix & review in your pull request.

}

case Some(conf.encodeEXI) => {
val encodeOpts = conf.encodeEXI
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm puzzled why we're seeing codecov check warnings that some lines are not covered by tests, when we do have a new test.

@tuxji
Copy link
Contributor

tuxji commented Nov 18, 2022

I'm sure people will try to encode an EXI file and decode an XML file the wrong way around and they should see an appropriate error message.

I remember Mike commented in the JIRA issue that XML and EXI files look so different that we might not even need a --decode option. It may be more work to implement but it might be nice to have file type recognition rather than -d.

Copy link
Member

@stevedlawrence stevedlawrence left a comment

Choose a reason for hiding this comment

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

+1 looks good with minor suggestions and rebased to work with the latest master to get the exi factory from an EXIInfosetHandler.

@jadams-tresys jadams-tresys force-pushed the DAFFODIL-2750-EXI_encode_decode branch from 3ef4ee7 to 41d8cd4 Compare November 28, 2022 19:00
@jadams-tresys
Copy link
Contributor Author

All issues should be addressed, including errors that were being seen on Windows CI machines. @tuxji just need your approval as I removed the expectEOF function.

Copy link
Contributor

@tuxji tuxji left a comment

Choose a reason for hiding this comment

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

+1

This command simply encodes an existing XML file to EXI using
Exificient. It will do a schema aware encoding if a schema is provided.
On the other side the decodeEXI command will decode an existing EXI file
to a plain text XML file.

This is a useful feature as there isn't an easy existing way to perform
these conversions using a schema aware encoding as it requires the use
of the DaffodilXMLEntityResolver for anything more than a basic schema.

DAFFODIL-2750
@jadams-tresys jadams-tresys force-pushed the DAFFODIL-2750-EXI_encode_decode branch from d90c12a to d984fbc Compare November 29, 2022 18:27
@jadams-tresys jadams-tresys merged commit 06701b1 into apache:main Nov 29, 2022
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.

3 participants