mbeckerle commented on code in PR #768:
URL: https://github.com/apache/daffodil/pull/768#discussion_r872486181
##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/processors/EvEncoding.scala:
##########
@@ -93,10 +93,11 @@ abstract class CharsetEvBase(encodingEv: EncodingEvBase,
tci: DPathCompileInfo)
override def compute(state: ParseOrUnparseState) = {
val encString = encodingEv.evaluate(state)
- val cs = CharsetUtils.getCharset(encString)
- if (cs == null) {
- tci.schemaDefinitionError("Unsupported encoding: %s. Supported
encodings: %s", encString, CharsetUtils.supportedEncodingsString)
+ val bcd = BitsCharsetDefinitionRegistry.find(encString).getOrElse(null)
+ if (bcd == null) {
+ tci.schemaDefinitionError("Unsupported encoding: %s. Supported
encodings: %s", encString,
BitsCharsetDefinitionRegistry.supportedEncodingsString)
Review Comment:
I think it's better to be verbose here.
I think there are two errors. One the user didn't get the charset name
right, so it isn't found.
The other is the user does have the name right, but the list of charsets
doesn't contain the charset because they didn't get the classpath right.
The latter error you need to see that the charset you seek isn't in the list
at all. So the verbose list is needed.
So I'm ok with this as is.
##########
daffodil-io/src/main/scala/org/apache/daffodil/processors/charset/CharsetUtils.scala:
##########
@@ -69,6 +71,10 @@ object CharsetUtils {
}
val unicodeReplacementChar = '\uFFFD'
+
+ private def SDE(id: String, args: Any*): Nothing = {
+ throw new Exception(id.format(args: _*))
+ }
Review Comment:
I suggest get rid of SDE here. (remove the method) You don't have enough
context to know whether something is a schema-definition-error or some other
kind of error at this point. These are just charset utils, general purpose
code.
You either need to define a EncodingUnavailableException, or you should just
be returning null/None or such to indicate the lookup failed. I guess I think
the latter is preferable.
There is a method to return a list of the available charsets so returning
null is preferable, and the caller can create a sensible diagnostic message.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
