stevedlawrence commented on code in PR #977:
URL: https://github.com/apache/daffodil/pull/977#discussion_r1123718717
##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/dsom/SDE.scala:
##########
@@ -120,10 +121,19 @@ class SchemaDefinitionWarning(
) {
def this(sc: SchemaFileLocation, kind: String, args: Any*) =
- this(Some(sc), None, kind, args: _*)
+ this(None, Some(sc), None, kind, args: _*)
override def isError = false
def modeName = "Schema Definition"
+
+ override def toString = warnID match {
+ case Some(id) =>
+ super.toString + "\n" +
+ s"To suppress this warning, add '${id}' to the " +
+ "daf:suppressSchemaDefinitionWarnings element of a Daffodil
defineConfig " +
+ "used when processing"
Review Comment:
This feels too verbose to output for every single warning. Especially since
there are other ways to define which warnings to suppress, like via the API
setting tunables. I'd suggest we just append the id either to the beginning or
end of the warning message. Coulple of different ideas:
```
Schema Definition Warning [deprecatedBuiltInFormats]: warning message here
```
```
Schema Definition Warning: warning message here (id: deprecatedBuiltInFormas)
```
I don't feel strongly about where it goes, but it should be pretty terse.
Diagnostis are already verbose enough with all the file/line number information.
##########
daffodil-codegen-c/src/main/scala/org/apache/daffodil/codegen/c/DaffodilCCodeGenerator.scala:
##########
@@ -216,7 +216,7 @@ class DaffodilCCodeGenerator(root: Root) extends
DFDL.CodeGenerator {
* Adds a warning message to the diagnostics
*/
private def warning(formatString: String, args: Any*): Unit = {
- val sde = new SchemaDefinitionWarning(None, None, formatString, args: _*)
+ val sde = new SchemaDefinitionWarning(None, None, None, formatString,
args: _*)
Review Comment:
It would be nice if all warnings had a warn id. I assume this is the only
place where we don't have an id? Maybe we can create a single warn ID like
`CodeGenerator` for al code generator warnings? Eventually we may want to
update this so there are unique warnings and/or have the ability to ignore code
generator warnings like we can others.
@tuxji, thoughts?
##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/dsom/SDE.scala:
##########
@@ -120,10 +121,19 @@ class SchemaDefinitionWarning(
) {
def this(sc: SchemaFileLocation, kind: String, args: Any*) =
- this(Some(sc), None, kind, args: _*)
+ this(None, Some(sc), None, kind, args: _*)
override def isError = false
def modeName = "Schema Definition"
+
+ override def toString = warnID match {
Review Comment:
I'm not sure toString is the right thing to override. If a user calls
`getMessage` instead of `toString` they won't get the warnID. I'm not sure
exatly what to override though, we might need to make something in Diagnostic
protected. Or alternatively SDW's could add the warn id to the format string?
--
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]
