Closed lambdaupb closed 3 years ago
I'm fine with either approach. @pointbazaar has been making a lot of changes lately, maybe he has some thoughts on the matter?
Well i personally never changed these configuration fields in any of my code using j2html. (Assuming we are talking about Config.java).
If there are people who actually use it, it would be beneficial to be able to overwrite them in a more flexible way. My favorite of the 2 approaches is Config on render because most, if not all fields in Config.java deal with the rendering, and not the construction of the tag tree.
.renderFormatted()
could be overloaded for that purpose, accepting some configuration object.
Having the Config on root-Element would be a bad idea in my opinion, because it would be inconsistent.
If you can configure the root element rendering, then it opens up questions like "Can i change the way this div(...)
is rendered?" from assuming that being able to configure one Tag's rendering, it should imply that the other tags support that
aswell. This leads to the root-Element being a unicorn and then lets people wonder which of the other tags are unicorns aswell.
Either of the 2 approaches means more work for people who want to override the rendering config for all the html rendered through j2html in their project, but it resolves the issue with separate dependencies using j2html and overriding each other's configuration, which is more important in my opinion than the work of additional parameter-passing for people using different rendering configurations for their entire project.
Following up from the merge of #179 , configuration can be now be defined when creating an HtmlBuilder using the factory methods on FlatHtml or IndentedHtml. The HtmlBuilder can then be used in rendering.
Config was modified to allow for creating instances, and has a method for creating a Config with default values and a method for creating a Config with the global values in the static fields. You can create modified instances of Config by with the with* methods on an instance. The existing render() and renderFormatted() methods use Config.global() for backwards compatibility.
Having the configuration as a global Singleton is pretty bad.
Migrating to something more composable could be done by something like the following approaches:
Config on root-Element
Use a J2HtmlContext to create the root element if a non-default config is required.
Config on render
Allow passing in the config into the render methods.