#9484 closed defect (fixed)
[PATCH] textarea formatting problems in TracNotification
Reported by: | Owned by: | ||
---|---|---|---|
Priority: | high | Milestone: | 0.12.1 |
Component: | notification | Version: | 0.12dev |
Severity: | normal | Keywords: | patch notification email |
Cc: | Branch: | ||
Release Notes: | |||
API Changes: | |||
Internal Changes: |
Description
When the contents of a custom field of type "textarea" is changed, the email from TracNotification is very oddly formatted. The problem is aggravated when the name of the custom field is very long.
The attached image (textarea_email_formatting_problem.png) highlights the problem.
- The first line of a textarea custom field will in some cases extended beyond the maximum 75 chars that TracNotification attempts to adhere to.
- subsequent lines are needlessly limited in their available width. They are short-changed by 7 chars + len(name of custom field). The longer the field name, the narrower the subsequent lines.
- lines preceded by newlines are given the proper width, but aren't indented.
I've attached a patch that I believe properly resolves this issue. I've also attached a screenshot of the email after the patch is applied (textarea_email_formatting_after_patch.png).
Attachments (4)
Change History (13)
by , 14 years ago
Attachment: | textarea_email_formatting_problem.png added |
---|
by , 14 years ago
Attachment: | textarea_email_formatting_after_patch.png added |
---|
by , 14 years ago
Attachment: | textarea_email_formatting.patch added |
---|
comment:1 by , 14 years ago
follow-ups: 3 4 comment:2 by , 14 years ago
Milestone: | → next-major-0.1X |
---|
Looks nice! How about using the width even better by not indenting by the length of the field name? Something like:
* type: defect => enhancement * a_long_textarea_name: This is a text that is long enough to be broken over several lines and stored as a textarea. This means it can even have newlines. * priority: normal => high
Actually, we already do something special for the description, which is the only standard textarea field. We could just treat all textarea fields the same as the description.
I think there's also a request for displaying changes to textarea fields as diffs, though I can't find the relevant ticket ATM.
comment:3 by , 14 years ago
by , 14 years ago
Attachment: | textarea_email_formatting_2.patch added |
---|
comment:4 by , 14 years ago
Replying to rblank:
Looks nice! How about using the width even better by not indenting by the length of the field name?
I have attached an updated patch that uses the full width when the change entry doesn't fit on one line (attachment:textarea_email_formatting_2.patch). The new patch plays some games with when the '⇒' symbol should be on the same line as a new/old entry and when it should be its own line.
After looking at #3914, I think my second patch fits nicely with that enhancement request. Although in my mind, diff'd text is overkill when both the new and old text can fit on one line. I would only go the diff route in Example #3 below. The patch attached to that ticket only does a unified_diff on the description field. I may add a patch to that ticket which diffs custom text fields as well.
I should point out that both this patch and the previous one also address the indenting and line-length issues in 'text' type custom fields. Additionally, both patches fix a formatting quirk where the bullet for the "cc" change entry is not aligned with every other type of change.
Behavior of attachment:textarea_email_formatting_2.patch:
Example 1: * a_long_textarea_name: short old value => short new value Example 2: * a_long_textarea_name: short old value => very very very very very very very very very very very very very long new value Example 3: * a_long_textarea_name: very very very very very very very very very very very very very long old value => very very very very very very very very very very very very very long new value Example 4: * a_long_textarea_name: very very very very very very very very very very very very very long old value => short new value
comment:5 by , 14 years ago
Milestone: | next-major-0.1X → 0.13 |
---|
Patch and feature looks good, moving to 0.13 (Remy, as you've started reviewing, I leave the owner field alone in case you want to take care of the landing).
Nit-picking about this part of the code (predates the patch):
... len(new) + l ...
is unreadable, is it l
or 1
? We should s/l/length/
or similar, there.
comment:6 by , 14 years ago
Priority: | normal → high |
---|
Excellent patch, thank you very much. I'll take care of applying it.
comment:7 by , 14 years ago
Milestone: | 0.13 → 0.12.1 |
---|---|
Owner: | set to |
comment:8 by , 14 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:9 by , 14 years ago
Owner: | changed from | to
---|
attachment:textarea_email_formatting.patch applies to source:trunk/trac/ticket/notification.py.
This is not spam.