Missing line break on the admin component edit page

[Ryan J Ollos <ryan.j.ollos@…>]

The layout of the component edit page is inconsistent relative to other admin edit pages. The component, milestone and version pages are shown below. For consistency, there should be a line break after Owner: on the component edit page.

.

[Ryan J Ollos <ryan.j.ollos@…>]

Patch against r11483 of the trunk.

[Remy Blank]

Thanks for the patch. Note that the owner_field() is used twice in the template, and one requires the <br/> while the other doesn't. So the patch will be slightly more complicated.

[Ryan J Ollos <ryan.j.ollos@…>]

Patch against r11483 of the trunk.

[Ryan J Ollos <ryan.j.ollos@…>]

Patch against r11483 of the trunk.

[Ryan J Ollos <ryan.j.ollos@…>]

Patch against r11483 of the trunk.

[Ryan J Ollos <ryan.j.ollos@…>]

Patch against r11483 of the trunk that sets max line width to 79 characters.

[Ryan J Ollos <ryan.j.ollos@…>]

Thanks, I took another crack at it in t10995-r11483-3.patch​. Component.select returns None if owner is an empty string, so default_owner will only be an empty string when owner_field is called with no arguments.

I think the logic would be less confusing if select returned an empty string for Component.owner (rather than None) and the default argument to owner_field could then be None, but I doubt we want to make changes like that which could have other consequences.

t10995-r11483-4.patch​ also fixes two ​PEP8 violations. Finally, I've wondered why the max 79 character line length coding guideline doesn't seem to be enforced for templates. I modified this template to meet the guideline in t10995-r11483-5.patch​ and it seems nice and readable.

[Remy Blank]

I used a more explicit approach in [11495], by passing an additional br_after_label argument.

We don't particularly limit the line length of templates to 79 characters, as long as the file is readable, so I didn't apply patch -5.

[Ryan J Ollos <ryan.j.ollos@…>]

Replying to rblank:

I used a more explicit approach in [11495], by passing an additional br_after_label argument.

That makes sense; it is consistent with [11164#file7] and much more clear than my patch. The argument could even be changed from br_after_label to multiline for consistency across the template code.