Django

Code

Ticket #3753 (closed: fixed)

Opened 1 year ago

Last modified 1 year ago

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

Reported by: Matt McClanahan <cardinal@dodds.net> Assigned to: adrian
Milestone: Component: Template system
Version: SVN Keywords: Template, debug, TEMPLATE_STRING_IF_INVALID
Cc: Triage Stage: Ready for checkin
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

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

template-invalid-varname.diff (0.6 kB) - added by Matt McClanahan <cardinal@dodds.net> on 03/18/07 02:05:01.
template-invalid-varname-doc.diff (0.6 kB) - added by Matt McClanahan <cardinal@dodds.net> on 03/18/07 02:05:22.
3753-1.diff (4.2 kB) - added by Matt McClanahan <cardinal@dodds.net> on 05/05/07 19:40:03.
3753-2.diff (4.2 kB) - added by Matt McClanahan <cardinal@dodds.net> on 05/06/07 00:57:53.
Rather use URL than email

Change History

03/18/07 02:05:01 changed by Matt McClanahan <cardinal@dodds.net>

  • attachment template-invalid-varname.diff added.

03/18/07 02:05:22 changed by Matt McClanahan <cardinal@dodds.net>

  • attachment template-invalid-varname-doc.diff added.

03/18/07 04:35:17 changed by Simon G. <dev@simon.net.nz>

  • keywords set to Template, debug, TEMPLATE_STRING_IF_INVALID.
  • needs_better_patch changed.
  • stage changed from Unreviewed to Design decision needed.
  • needs_tests changed.
  • needs_docs changed.

04/30/07 22:54:50 changed by mtredinnick

  • needs_better_patch set to 1.
  • needs_tests set to 1.
  • stage changed from Design decision needed to Accepted.

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.

05/05/07 19:39:39 changed by Matt McClanahan <cardinal@dodds.net>

  • needs_better_patch deleted.
  • needs_tests deleted.

As requested, tests added and suggested modifications made.

05/05/07 19:40:03 changed by Matt McClanahan <cardinal@dodds.net>

  • attachment 3753-1.diff added.

05/05/07 23:20:11 changed by Simon G. <dev@simon.net.nz>

  • stage changed from Accepted to Ready for checkin.

05/06/07 00:57:53 changed by Matt McClanahan <cardinal@dodds.net>

  • attachment 3753-2.diff added.

Rather use URL than email

05/07/07 22:33:12 changed by mtredinnick

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).

05/07/07 22:36:16 changed by mtredinnick

  • status changed from new to closed.
  • resolution set to fixed.

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


Add/Change #3753 (Allow TEMPLATE_STRING_IF_INVALID to contain a placeholder for the missing variable's name)




Change Properties
Action