Opened 15 years ago

Closed 15 years ago

#6578 closed (invalid)

Strange code introduced in changeset 7089

Reported by: Grzegorz Lukasik <hauserx@…> Owned by: nobody
Component: Template system Version: dev
Severity: Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


 	293	            try: 
 	294	                if nodelist.contains_nontext: 
 	295	                    raise AttributeError 
 	296	            except AttributeError: 
 	297	                raise TemplateSyntaxError("%r must be the first tag in the template." % node) 

raising exception and then catching it immediately doesn't look right.

Attachments (1)

template_simplify.patch (887 bytes) - added by Grzegorz Lukasik <hauserx@…> 15 years ago.

Download all attachments as: .zip

Change History (3)

Changed 15 years ago by Grzegorz Lukasik <hauserx@…>

Attachment: template_simplify.patch added

comment:1 Changed 15 years ago by Grzegorz Lukasik <hauserx@…>

Has patch: set

comment:2 Changed 15 years ago by Malcolm Tredinnick

Resolution: invalid
Status: newclosed

The current code is correct. We need to catch the attribute error because nodelist might not have an attribute called contains_nontext (read the commit message for why that's the case). Therefore, to avoid code duplication, since we want the same error handling, we just make sure we leap into the exception catcher. This is all in the error path, so it's not a performance penalty that's paid by correct code. It's also fairly idiomatic Python: if a block of code has an exception handler and you need the same error handling, jump into the exception handling portion by raising the right exception. Used throughout the standard library in quite a few places.

So, thanks for the review, but it's correct.

Note: See TracTickets for help on using tickets.
Back to Top