Silence some warnings in escaping #58
Conversation
If the object passed to the template has undefined keys (or methods that return Nil,) then there were warnings produced from the escaping. For an Associative this could potentially be worked around at source, but for an object that has methods that may return Nil ( say an ORM row object,) it's a bit less tractable.
|
Isn't this PR hide potential bugs instead of helping to reveal them in the caller's code? |
|
Well it might, but unfortunately there is no way of distinguishing between deliberate and expected behaviour and bug in this case, if, for instance, one has an object with attributes that are optionally defined then one doesn't particularly want to be warned about it, and changing the interface of the object, which may already be used happily in other places, in order to quiet warnings here is not really ideal ( or even possible.) If there is a bug in the thing that is generating the data passed to the template then that can and should be determined by other means, the template module should just assume that the caller knows what they are doing in this case (all other things being equal.) |
|
DISCLAIMER: To be fair, I completely agree with you the way Cro gives us tons of warnings now is not natural. More so, the warnings are useless, it doesn't tell you a thing like "In your template X at line Y caret Z you use this attribute, but it was Nil, oh noes!", it just says "Wow, I saw a Nil" and that's it, making debugging a rather cumbersome activity I myself dislike, We even have a ticket at #40
Hmm. As an example, let's say you have an object with field 'foo' which can be Nil / type object / whatever "not a real value". IMO for deliberately omitted fields, a nice idea is to do At worst case this behaviour can/should be configurable if people don't want to write the conditional wrapper, but making no warn policy a default sounds like a potentially dangerous thing, it's one thing when you write an HTML template assuming some things to be empty and another thing when you get a bunch of broken HTML attrs due to it (as it happens with e.g. the logs website). No?
Why should it be this way, can you please elaborate considering the |
|
Yeah, I agree the warnings are fine IFF they tell me where in the template / what key / method was used, etc. The way it is now, is more like "computer says no". :-) |
Yep, that works. I'm cool with doing that but it would be nicer if some diagnostic information could be output, right now it is difficult to determine where the undefined value is being used. Also it might be worth documenting that a warning will be issued and how to avoid it. |
A proper solution for the ticket I mentioned should be enough to tick all the boxes. Should I close this PR? |
|
Re silencing warnings: if that were my intent, I could easily prefix I'm looking forward to the day where I can make a warnings collector using macros and RakuAST, which I have now done in my %warnings is BagHash;
CONTROL {
when CX::Warn {
%warnings.add(.message);
.resume;
}
}
$file.spurt: render-template $crot, %params;
for %warnings.sort(*.key) -> (:key($message), :value($seen)) {
note $seen > 1
?? $seen ~ "x $message"
!! $message;
}which greatly reduces the amount of noise on STDERR :-) |
|
No, it's fine I'll close 😄 |
If the object passed to the template has undefined keys (or methods that return Nil,) then there were warnings produced from the escaping. For an Associative this could potentially be worked around at source, but for an object that has methods that may return Nil ( say an ORM row object,) it's a bit less tractable.