Reduce Properties, Names, and Defaults duplication in generated classes #328
Conversation
| } | ||
| val containedNodeNames = nodeType.containedNodes match { | ||
| case containedNodes if containedNodes.nonEmpty => | ||
| containedNodes.map(n => camelCaseCaps(n.nodeType.name)).toList |
There was a problem hiding this comment.
should this also have a prefix like above?
e.g. "Contained nodes:" ::
| "Node properties:" :: properties.map(property => camelCaseCaps(property.name)).toList | ||
| case _ => Nil | ||
| } | ||
| val containedNodeNames = nodeType.containedNodes match { |
There was a problem hiding this comment.
there's a lot of spaghetti code in this class, maybe it's worth extracting things like this into something like
def commentForProperty[A](prefix: String, propertyNames: Seq[String]): String = ???and then
val nodePropertiesComments =
commentForProperty("Node properties:", noteType.properties.map(_.property.name) ++
commentForProperty("Contained properties:", containedNodeNames.map(_.nodeType.name)or so..
| results.addOne(file) | ||
| results.addOne(propertiesFile) | ||
| val propertyNamesSource = schema.properties | ||
| .map { property => |
There was a problem hiding this comment.
did scalafmt format it like that? please confirm, I'd prefer to have the .map on the same line...
There was a problem hiding this comment.
similar cases below. likely a separate scalafmt pr
There was a problem hiding this comment.
It is indeed a scalafmt change
| results.addOne(propertiesFile) | ||
| val propertyNamesSource = schema.properties | ||
| .map { property => | ||
| property.comment.map(comment => "/**" ++ comment ++ "*/\n").getOrElse("") ++ s"""val ${camelCaseCaps( |
There was a problem hiding this comment.
this is also formatted in a way that makes it hard to read
| case p if p.hasDefault => | ||
| s"""val ${camelCaseCaps(p.name)} = ${Helpers.defaultValueImpl(p.default.get)}""" | ||
| } | ||
| .mkString("\n\n") |
There was a problem hiding this comment.
| .mkString("\n\n") | |
| .mkString("\n") |
| |val ${camelCaseCaps(containedNode.nodeType.name)}: String = "${containedNode.nodeType.name}"""".stripMargin | ||
| } | ||
| .toSet | ||
| .mkString("\n\n") |
There was a problem hiding this comment.
| .mkString("\n\n") | |
| .mkString("\n") |
| } | ||
|
|
||
| /** Node properties: | ||
| * - DispatchType |
There was a problem hiding this comment.
please also add valueType and comment of the properties, and all information from the contained node as well
johannescoetzee
force-pushed
the
johannes/reduce-property-duplication
branch
from
9ce7ed4 to
20806ce
Compare
|
I've updated the PR with changes that I think address all of the comments on the PR, but please let me know if I missed something. I'm not a huge fan of the format of the generated comments, but this was the best I've found that isn't messed up by scalafmt. Edit: This also breaks CPG due to a double definition |
|
The CPG break is now fixed (I forgot a |
|
we'll configure scalafmt in a followup PR :) |
|
@johannescoetzee if you're happy with the changes I pushed re scaladoc feel free to merge |
|
The new comment format is much nicer :) |
This PR includes a few changes to generated code:
NodeType.Properties,NodeType.PropertyNames, andNodeType.PropertyDefaultsobjects<basepackage>.PropertyDefaultsobject defining constants for all property defaultsPropertyNames.javatoPropertyNames.scalawith scala constants