Opened 17 years ago
Closed 17 years ago
#4573 closed (wontfix)
Minor rewrite of linebreaks and linebreaksbr
Reported by: | 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)
Change History (13)
by , 17 years ago
Attachment: | django-linebreaks.patch added |
---|
by , 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 , 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 , 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 ==).
comment:2 by , 17 years ago
Triage Stage: | Unreviewed → Design decision needed |
---|
How about changing
xhtml = True if arg == "html": xhtml = False
to
xhtml = (arg != 'html')
comment:3 by , 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 :-)
comment:4 by , 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 , 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 , 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 , 17 years ago
Attachment: | django-linebreaks-html-v5.diff added |
---|
Thank you both for feedback. Here's the same patch with the <br/>
behaviour reverted.
comment:7 by , 17 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
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.
Includes patch, docs and tests.