Code

Opened 7 years ago

Closed 7 years ago

#3959 closed (wontfix)

[patch] newforms: label as first argument

Reported by: Marty Alchin <gulopine@…> Owned by: nobody
Component: Forms Version: master
Severity: Keywords: newforms label,sprintdec01
Cc: Triage Stage: Design decision needed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

It seems strange to me that model fields can have their verbose_name set as a single positional argument, but newforms labels require the label= prefix unless required and widget are also specified. This patch simply moves label to the front, so that forms may specify field labels as just a single positional argument, just like model field verbose_names.

Attachments (1)

label.diff (2.7 KB) - added by Marty Alchin <gulopine@…> 7 years ago.
Simply moves label to the front of the argument list for fields

Download all attachments as: .zip

Change History (9)

Changed 7 years ago by Marty Alchin <gulopine@…>

Simply moves label to the front of the argument list for fields

comment:1 Changed 7 years ago by SmileyChris

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Design decision needed

It does seem like an ok idea, but backwards-incompatibility concerns.

comment:2 Changed 7 years ago by Marty Alchin <gulopine@…>

I had considered there might be some backwards-incompatibility concerns, but since the existing implementation essentially requires people to use label= anyway, moving it shouldn't affect that. The only true concern would be people supplied required (and possibly widget) as positional arguments, which should probably be discouraged anyway, for readability concerns. That said, however, I understand that this patch comes 5 months after label was implemented, and that convention may already be too well established.

comment:3 Changed 7 years ago by mtredinnick

Probably not worth changing. I'm going to think about it a bit more, but if another developer wants to make a decision, I'm not going to mind. I'm probably -0 on this at the moment.

Writing "label=" is hardly going to kill people. The logic that people shouldn't be using positional arguments currently is flawed, so it does have a backwards-incompatibility impact, however it's not like 1.0 isn't going to have a few of those.

comment:4 Changed 7 years ago by Marty Alchin <gulopine@…>

I'll admit, I'm +0 on this myself. I wish I had delved into newforms early enough to catch this before it hit store shelves, but now that it's out and running, I'm not going to push too hard on the subject.

I do agree that writing label= isn't going to cause any code problems, nor is it likely to induce carpal tunnel. I'll also concede that my presumption about people already using keyword arguments all the time was all too broad, and thus inherently flawed. It's probably true in most cases, but it's far from a guarantee.

The biggest reason I put this together was just consistency. This would fall in line with verbose_name, and thus make Django feel a bit more unified, that's all. So like I said, I'm +0 really, I'm not going to lose sleep over this one.

comment:5 Changed 7 years ago by SmileyChris

I agree that it'd be more consistent. The question is are we perfectionists to the level of breaking backwards compatibility?
(I'm +0 too)

comment:6 Changed 7 years ago by ubernostrum

I'd be -0 on this because of the backwards incompatibility.

comment:7 Changed 7 years ago by thejaswi_puthraya

  • Keywords label,sprintdec01 added; label removed

I guess this ticket can be closed because it is a non-trivial one.

comment:8 Changed 7 years ago by mtredinnick

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

The gain here isn't worth the backwards-incompatibility pain.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.