Opened 17 years ago

Closed 17 years ago

#3753 closed (fixed)

Allow TEMPLATE_STRING_IF_INVALID to contain a placeholder for the missing variable's name

Reported by: Matt McClanahan <cardinal@…> Owned by: Adrian Holovaty
Component: Template system Version: dev
Severity: Keywords: Template, debug, TEMPLATE_STRING_IF_INVALID
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

It'd be handy when debugging to see the name of the undefined variable in the TEMPLATE_STRING_IF_INVALID output. To do that, the patch allows TEMPLATE_STRING_IF_INVALID to contain a single '%s' which will be replaced with the invalid variable's name.

Attachments (4)

template-invalid-varname.diff (629 bytes ) - added by Matt McClanahan <cardinal@…> 17 years ago.
template-invalid-varname-doc.diff (577 bytes ) - added by Matt McClanahan <cardinal@…> 17 years ago.
3753-1.diff (4.2 KB ) - added by Matt McClanahan <cardinal@…> 17 years ago.
3753-2.diff (4.2 KB ) - added by Matt McClanahan <cardinal@…> 17 years ago.
Rather use URL than email

Download all attachments as: .zip

Change History (10)

by Matt McClanahan <cardinal@…>, 17 years ago

by Matt McClanahan <cardinal@…>, 17 years ago

comment:1 by Simon G. <dev@…>, 17 years ago

Keywords: Template debug TEMPLATE_STRING_IF_INVALID added
Triage Stage: UnreviewedDesign decision needed

comment:2 by Malcolm Tredinnick, 17 years ago

Needs tests: set
Patch needs improvement: set
Triage Stage: Design decision neededAccepted

Matt,

This sounds reasonable. A few changes to the existing patch(es) I'd like to see:

  1. Needs a test or two.
  2. Just check if '%s' in TEMPLATE_STRING_IF_INVALID, rather than doing the count() call. If there's more than one format string in that string, it's okay to error out, since that's a developer error that they can easily fix as part of the debugging cycle. So check if '%s' is in the string, then do the substitution if so. Python will raise an error anyway if you include more than one format marker ("not enough arguments for format string"), so you don't need to do anything extra there.
  3. You might want to consider setting a module variable to indicate if the format string is present. Settings values shouldn't ever change (it's a bug if they do), so checking it once and storing the result in a flag for subsequent times will be a performance improvement.

If you could make one combined patch for everything when you update the patch, that would be cool. Also consider including a patch for AUTHORS so that I don't have to try and spell your name correctly.

comment:3 by Matt McClanahan <cardinal@…>, 17 years ago

Needs tests: unset
Patch needs improvement: unset

As requested, tests added and suggested modifications made.

by Matt McClanahan <cardinal@…>, 17 years ago

Attachment: 3753-1.diff added

comment:4 by Simon G. <dev@…>, 17 years ago

Triage Stage: AcceptedReady for checkin

by Matt McClanahan <cardinal@…>, 17 years ago

Attachment: 3753-2.diff added

Rather use URL than email

comment:5 by Malcolm Tredinnick, 17 years ago

For future reference: you should not access settings.* in code that is executed at import time. Doing so means that you cannot import the module in question without haing already configured settings, which requires messy code design in the manually-configured settings case. We should allow people to follow PEP-8 formatted code practices: all imports first, then execute whataver is required (settings configuration, if needed).

I've altered the patch slightly so that settings are only accessed when required (inside Template methods).

comment:6 by Malcolm Tredinnick, 17 years ago

Resolution: fixed
Status: newclosed

(In [5167]) Fixed #3753 -- Allow optional display of invalid variable name in
TEMPLATE_STRING_IF_INVALID. Thanks, Matt McClanahan.

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