[GitHub] [daffodil] tuxji commented on pull request #821: Add support for new tunable "xmlOutputStyle"

[GitBox <gi...@apache.org>]

tuxji commented on PR #821:
URL: https://github.com/apache/daffodil/pull/821#issuecomment-1368009944

   We need to update this pull request's title and description since they no longer match what this pull request actually does.  We collectively decided not to create a tunable XMLOutputStyle, only pass an enum XMLTextEscapeStyle as a new constructor parameter to XMLTextInfosetOutputter.  We decided not to address the question how to let a CLI option change infoset outputter settings in this pull request, even though this pull request's issue, [DAFFODIL-2346](https://issues.apache.org/jira/browse/DAFFODIL-2346), originally requested a CLI option to use <!CDATA[]]> to preserve whitespace in XML infosets' simple elements' text content.
   
   Note that another issue, [DAFFODIL-2234](https://issues.apache.org/jira/browse/DAFFODIL-2234), also wants a CLI option to not pretty print XML or JSON infosets.  We already pass a boolean "pretty" as a constructor parameter to XMLTextInfosetOutputter, but we don't have any way for the CLI to change this constructor parameter's value from true to false in the places where the codebase passes true.
   
   If we still punt the question of how to let the CLI, or TDML Runner, change infoset outputter settings to DAFFODIL-2234 instead of this pull request, then I suggest we edit this pull request's new title and description to:
   
   ```text
   Allow <!CDATA[]]> to preserve whitespace in XML text content
   
   Add a new constructor parameter (an enum XMLTextEscapeStyle with Standard and CDATA values) to XMLTextInfosetOutputter.  When CDATA is passed instead of the default Standard, wrap simple XML elements' text content in CDATA brackets to preserve any whitespace they contain.
   
   DAFFODIL-2346
   ```
   
   Also, I think we should consider adding new XMLTextInfosetOutputter methods which can change infoset outputter settings using "withXXX" calls instead of changing them in constructor parameters.  Calling "withXXX" methods is a more upward compatible and extensible mechanism to change objects' settings since we can simply add new "withXXX" methods in the future.  A "withXXX" method can also eliminate the need to pass a boolean or enum value since the "withXXX" method's name can describe the new setting as well, such as "withPrettyPrint" or "withCDATA".


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org