Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#18133 closed New feature (wontfix)

global settings for label_suffix

Reported by: Evil Clay <clay.evil@…> Owned by: nobody
Component: Uncategorized Version: 1.4
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Russell Keith-Magee)

hi, would it be possible to make label_suffix settable in settings file? now, if i want to change the suffix for the whole site, i have to have in my code something like this:

class MyModelForm(forms.ModelForm):

    def __init__(self, data=None, files=None, auto_id='id_%s', prefix=None,
                 initial=None, error_class=ErrorList, label_suffix='',
                 empty_permitted=False, instance=None):
        super(MyModelForm, self).__init__(data, files, auto_id, prefix, initial,
                                            error_class, label_suffix, empty_permitted, instance)
class MyForm(forms.Form):

    def __init__(self, data=None, files=None, auto_id='id_%s', prefix=None,
                 initial=None, error_class=ErrorList, label_suffix='',
                 empty_permitted=False):
        super(MyForm, self).__init__(data, files, auto_id, prefix, initial,
                                            error_class, label_suffix, empty_permitted)

and then inherit all my forms from this classes. But this is rather a way around then a solution. Anyways it doesn't feel pretty.
Thank you.

Change History (10)

comment:1 by Russell Keith-Magee, 12 years ago

Description: modified (diff)
Resolution: invalid
Status: newclosed

Fixed formatting of the ticket description. Please use preview.

Again, as with #18134, you've suggested a solution that modifies API, but you haven't described the problem you're trying to solve, or the effect that the new label_suffix would have.

Closing invalid; feel free to reopen if you care to provide more details.

comment:2 by Evil Clay <clay.evil@…>, 12 years ago

Hi. Again, i'm sorry.

as in #18134 i'm talking about the same area https://docs.djangoproject.com/en/dev/ref/forms/api/#configuring-html-label-tags

Normally, a colon (:) will be appended after any label name when a form is rendered. It's possible to change the colon to another character, or omit it entirely, using the label_suffix parameter:
f = ContactForm(auto_id='id_for_%s', label_suffix="")

So if i want to change to colon to something else for my ENTIRE project i would have to either:

  1. set it in my code on every form instantination (i.e. f = ContactForm(label_suffix="")) which is wicked and dirty and just wrong...(but good if you want to change the suffix only on one page etc..)
  2. override the two classes forms.Form and forms.ModelForm and use them in the project as parents for all my forms see the description above.

I'm in believe that removing the hardcoded label_suffix=":" in:

  1. class BaseModelForm Found at: django.forms.models init method
  2. class BaseForm Found at: django.forms.forms init method

and replace it which something like "label_suffix=settings.LABLE_SUFFIX" would not be that much work.
However i'm not an expert on the internals of django, so it's just IMHO.

comment:3 by Evil Clay <clay.evil@…>, 12 years ago

Resolution: invalid
Status: closedreopened

comment:4 by Carl Meyer, 12 years ago

Resolution: wontfix
Status: reopenedclosed

The solution you demonstrate using a subclass is not a workaround, it's a clear and explicit solution to the problem (though it could perhaps be made more elegant by use of **kwargs). Adding settings introduces implicit action at a distance; in general we want to reduce the number of settings-dependencies in core Django components, not increase it.

comment:5 by Evil Clay <clay.evil@…>, 12 years ago

Hi,

it's your framework so it is your call. But IMHO overriding a framework class that was not primary designed to be overridable (hence is not abstract and is not documented to be ovveridable) is not a clean nor elegant solution. In next version of Django, when you add an important argument to the unity method, it will render this solution useless, or will introduce a problem to my project which may be hard to find. So i will have to keep track of this "solution" in the documentation hence it can't be an elegant solution.

However, thanks for your time anyway.

P.S. If you don't want to introduce settings, you can design some structure that can be overridden by design and will be documented as to be overridden if such a change is needed. I'm a Java programer, so python is not my home language so i cannot yet design this kind of machinery, but python is a very magical language, i'm sure that a better design exists.

comment:6 by Evil Clay <clay.evil@…>, 12 years ago

i could how ever imagine something like this

class BaseForm:

label_suffix = None

init(label_suffix=None)

self.label_suffix = label_suffix

""" safe to override """

def label_suffix(self):

if label_suffix:

return label_suffix

else:

return ":"

def render(self):

return "<label>%s%s</label>" % (label, self.label_suffix())


in this design i would have to only override the method label_suffix(self):

in reply to:  5 comment:7 by Carl Meyer, 12 years ago

Replying to Evil Clay <clay.evil@…>:

it's your framework so it is your call. But IMHO overriding a framework class that was not primary designed to be overridable (hence is not abstract and is not documented to be ovveridable) is not a clean nor elegant solution.

Huh? Form is used only by subclassing, that's what it's for.

In next version of Django, when you add an important argument to the unity method, it will render this solution useless, or will introduce a problem to my project which may be hard to find. So i will have to keep track of this "solution" in the documentation hence it can't be an elegant solution.

That's why you should use **kwargs instead, to avoid this problem.

P.S. If you don't want to introduce settings, you can design some structure that can be overridden by design and will be documented as to be overridden if such a change is needed. I'm a Java programer, so python is not my home language so i cannot yet design this kind of machinery, but python is a very magical language, i'm sure that a better design exists.

Not every little tweak needs its own separate overridable method. An __init__ argument is appropriate for this.

Please take further discussion (if necessary) to a mailing list thread, or usage/Python questions to the appropriate fora. Thanks!

comment:9 by Evil Clay <clay.evil@…>, 12 years ago

im using the solution described about, but you mentioned some improvement - > (though it could perhaps be made more elegant by use of kwargs), so i just want to add this:

  • i'm a beginner in python, as i consider my self a good developer in java, and from my experience, people will copy paste the above solution into their projects without thinking. Maybe it would be wise to either:
  1. add a proper kwargs solution to this ticket (you you write it, i can test it in my current project)
  2. add a proper solution to djnago docs and delete this ticket so people won't copy the init args things which would make their projects vulnerable to future updates.

I'm not asking this for my self, really, i can handle this one project of mine, but i really hate to provide such dirty code. So if you don't have the time to do 1 or 2, then at least delete this ticket so people won't copy this.
Thank you.

in reply to:  9 ; comment:10 by Carl Meyer, 12 years ago

Replying to Evil Clay <clay.evil@…>:

  1. add a proper kwargs solution to this ticket (you you write it, i can test it in my current project)
class MyForm(forms.Form):
    def __init__(self, *args, **kwargs):
        kwargs.setdefault("label_suffix", "")
        super(MyForm, self).__init__(*args, **kwargs)

in reply to:  10 comment:11 by Evil Clay <clay.evil@…>, 12 years ago

Replying to carljm:

Replying to Evil Clay <clay.evil@…>:

  1. add a proper kwargs solution to this ticket (you you write it, i can test it in my current project)
class MyForm(forms.Form):
    def __init__(self, *args, **kwargs):
        kwargs.setdefault("label_suffix", "")
        super(MyForm, self).__init__(*args, **kwargs)

Hi, as promised i did test the code for both inheriting from forms.ModelForm and forms.Form and both work well. Thank you very much for this enlightenment. You're Godlike.

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