antlr / stringtemplate4

StringTemplate 4
http://www.stringtemplate.org
Other
955 stars 231 forks source link

Fix: don't apply AttributeRenderer to text elements #279

Closed bannmann closed 2 years ago

bannmann commented 3 years ago

The templates docs say:

A template is a sequence of text and expression elements, optionally interspersed with comments.

However, if one registers an AttributeRenderer<String>, StringTemplate uses it not only for rendering attributes (expression elements), but also for static text emitted by the template (text elements).

This PR adds a unit test uncovering the problem and a (hopefully adequate) bugfix for the interpreter.


Background: for my project, I wrote an AttributeRenderer<String> that defaults to encoding XML/HTML entities even without an explicit format="" option in the attribute expression. The surprising effect was that the static HTML markup around the attribute expressions was encoded as well and therefore no HTML tags being left in the output.

parrt commented 3 years ago

hi. Sounds interesting but I'm worried that with such a large user base now, this could cause all sorts of things to stop working :(

bannmann commented 3 years ago

How about making this an opt-in behavior, then? I would add a method on STGroup to enable this, e.g. setStrictRendering(true). The Interpreter could then either access a boolean field in STGroup or have its own field similar to Interpreter.debug. Alternatively, we could create a subclass StrictInterpreter so that we don't need an if during template execution.

parrt commented 3 years ago

my time is basically nonexistent. Is there a way for you to get what you need by doing some subclassing and overwriting more methods?

bannmann commented 3 years ago

With the current code, subclassing is not a feasible solution unfortunately:

But even if I had a subclass of Interpreter that does what I want, getting STGroup and ST to use it is not easy as there is no central place where the interpreter is instantiated (think InterpreterFactory).

Luckily, making the changes required to allow subclassing turned out to be trivial! From there, implementing this new optional mode that I call "strict rendering" is only a few lines of code, so I did that as well. I also implemented tests for both the existing (unchanged) behavior and the opt-in alternative and added some pointers to the documentation.

I hope you find the time to look through the changes.

parrt commented 3 years ago

Hi. I've taken a quick look and it would be possible to make an option I would say, but I'm not sure I would implement this feature with the mechanism you provided. I think this would take more study which I don't have time for I'm afraid. :(

parrt commented 2 years ago

Hi. sorry for the delay and lack of response but I'm going to close this just to clean up a little bit as I doubt I will have time to think about this. going out with 4.3.2 shortly.