Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#16866 closed Bug (fixed)

Handling empty list of candidates in select_template function

Reported by: Silver_Ghost Owned by: Silver_Ghost
Component: Template system Version: master
Severity: Normal Keywords: select_template
Cc: Silver_Ghost Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

django.template.loader.select_template expects list of template names and select first loadable. If there are some template names in list and no one from them couldn't be loaded then raising TemplateDoesNotExist exception is the right decision. But if there isn't any template names in the list then >>Template<<DoesNotExist is wrong exception, I think.

So maybe checks if list is empty and raise exception with message like "I can't load any template for you because you didn't gave me any possible variants"?

Attachments (2)

handle_empty_list_of_templates.diff (614 bytes) - added by Silver_Ghost 4 years ago.
handling empty list of candidates for select_template
handle_empty_list_of_templates.2.diff (1.3 KB) - added by Silver_Ghost 4 years ago.
Switching to TemplateDoesNotExist exception and adding tests

Download all attachments as: .zip

Change History (6)

Changed 4 years ago by Silver_Ghost

handling empty list of candidates for select_template

comment:1 Changed 4 years ago by carljm

  • Has patch set
  • Needs documentation unset
  • Needs tests set
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

I don't think we need to change the exception class here; TemplateDoesNotExist is an ok exception to raise in this case. Changing it to ValueError could break existing code; it's not worth it.

Accepting this on the basis that it would be nice to provide a clearer message to the TemplateDoesNotExist error, rather than an empty message.

Also, test needed.

Changed 4 years ago by Silver_Ghost

Switching to TemplateDoesNotExist exception and adding tests

comment:2 Changed 4 years ago by Silver_Ghost

  • Needs tests unset
  • Patch needs improvement unset

The patch changed according to ticket:16866#comment:1.

comment:3 Changed 4 years ago by ptone

while the term candidate has apparently been introduced in the class-based-views docs - it is a relatively new term, and one that could confuse people.

Since the documentation refers specifically to a "list of template names" I would just use an error string of "list of template names was empty"

comment:4 Changed 4 years ago by carljm

  • Resolution set to fixed
  • Status changed from new to closed

In [16861]:

Fixed #16866 -- Clearer error message if empty list is passed to select_template. Thanks Silver_Ghost for report and patch.

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