Opened 17 years ago

Closed 17 years ago

#4573 closed (wontfix)

Minor rewrite of linebreaks and linebreaksbr

Reported by: Johan Bergström <bugs@…> Owned by: nobody
Component: Template system Version: dev
Severity: Keywords: linebreaks html
Cc: Triage Stage: Design decision needed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

This relatively small patch modifieslinebreaks and linebreaksbr. They now share the same backend (django.utils.html.linebreaks) which makes linebreaksbr accept additional types of newlines (\r\n etc). This patch also adds support for html (as addition to xhtml) output which are passed in as an argument to the filter:

    <h1>linebreaks/linebreaksbr both accepts "html" or 0 (False) as argument</h1>
    {{ blog.entry|linebreaks:"html" }}

Both filters and util defaults to xhtml output (the current behaviour).

Attachments (6)

django-linebreaks.patch (3.4 KB ) - added by Johan Bergström <bugs@…> 17 years ago.
Includes patch, docs and tests.
django-linebreaks.diff (3.4 KB ) - added by Lfe 17 years ago.
Last patch somehow didn't apply cleanly. Here's the new version (with a proper mime-type)
django-linebreaks-v2.diff (3.4 KB ) - added by Johan Bergström <bugs@…> 17 years ago.
...and here's the slightly updated version. Should save a cpu cycle and fix a nasty bug (is vs ==).
django-linebreaks-v3.diff (3.4 KB ) - added by Johan Bergström <bugs@…> 17 years ago.
Fixed a typo..
django-linebreaks-html-v4.diff (5.6 KB ) - added by Johan Bergström <bugs@…> 17 years ago.
Fourth revision of the patch
django-linebreaks-html-v5.diff (5.4 KB ) - added by Johan Bergström <bugs@…> 17 years ago.
Thank you both for feedback. Here's the same patch with the <br/> behaviour reverted.

Download all attachments as: .zip

Change History (13)

by Johan Bergström <bugs@…>, 17 years ago

Attachment: django-linebreaks.patch added

Includes patch, docs and tests.

by Lfe, 17 years ago

Attachment: django-linebreaks.diff added

Last patch somehow didn't apply cleanly. Here's the new version (with a proper mime-type)

comment:1 by Johan Bergström <bugs@…>, 17 years ago

Safari somehow got my IRC nickname as name when posting the updated patch, sorry for any confusion this may have caused...

by Johan Bergström <bugs@…>, 17 years ago

Attachment: django-linebreaks-v2.diff added

...and here's the slightly updated version. Should save a cpu cycle and fix a nasty bug (is vs ==).

by Johan Bergström <bugs@…>, 17 years ago

Attachment: django-linebreaks-v3.diff added

Fixed a typo..

comment:2 by Chris Beaven, 17 years ago

Triage Stage: UnreviewedDesign decision needed

How about changing

xhtml = True 
if arg == "html": 
    xhtml = False 

to

xhtml = (arg != 'html')

comment:3 by Johan Bergström <bugs@…>, 17 years ago

SmileyChris: My major itch was HTML (as an option to XHTML) support - which should be outlined in my patch. If the committer feels that my code could be rewritter further - you/he/she has my blessing :-)

by Johan Bergström <bugs@…>, 17 years ago

Fourth revision of the patch

comment:4 by Johan Bergström <bugs@…>, 17 years ago

Ok. Here's another stab at this. I've implemented the improvement SmileyChris suggested. This should also apply cleanly after the unicode merge. I modified the test case for linebreaks so it accepts arguments and added extra test cases for linebreaksbr. One thing this patch also does is change the newline output from <br /> to <br/>, since using truncatewords will strip the line break containing a space (bug of feature?). I guess most people write line breaks with spaces, but this patch is slimmer than modifying truncatewords.

comment:5 by Malcolm Tredinnick, 17 years ago

I haven't read the patch yet, so I have no comment on the specifics, however one thing in your comment raises alarms.

The space before the slash in <br /> is required for HTML parsing. The trailing slash is actually invalid in HTML (as opposed to XHTML) and so it is exploiting the required error recovery behaviour in order to work. You can't just remove it or there will be subtle errors in some renderers when used in SGML-based HTML output.

comment:6 by Chris Beaven, 17 years ago

You shouldn't be using truncatewords after that filter - it's not HTML aware (that's why I wrote the truncatewords_html filter) so it's not something you need to worry about.

by Johan Bergström <bugs@…>, 17 years ago

Thank you both for feedback. Here's the same patch with the <br/> behaviour reverted.

comment:7 by Jacob, 17 years ago

Resolution: wontfix
Status: newclosed

While there's nothing technically wrong with this patch, I just don't see a reason to include it since it's just code churn.

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