Edgewall Software
Modify

Opened 14 years ago

Closed 14 years ago

Last modified 14 years ago

#9484 closed defect (fixed)

[PATCH] textarea formatting problems in TracNotification

Reported by: andrew.c.martin@… Owned by: Andrew C Martin <andrew.c.martin@…>
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)

textarea_email_formatting_problem.png (27.7 KB ) - added by anonymous 14 years ago.
textarea_email_formatting_after_patch.png (21.1 KB ) - added by anonymous 14 years ago.
textarea_email_formatting.patch (948 bytes ) - added by anonymous 14 years ago.
textarea_email_formatting_2.patch (1.4 KB ) - added by andrew.c.martin@… 14 years ago.

Download all attachments as: .zip

Change History (13)

by anonymous, 14 years ago

by anonymous, 14 years ago

comment:2 by Remy Blank, 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.

in reply to:  2 comment:3 by Christian Boos, 14 years ago

Replying to rblank:

I think there's also a request for displaying changes to textarea fields as diffs

#3914.

by andrew.c.martin@…, 14 years ago

in reply to:  2 comment:4 by andrew.c.martin@…, 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 Christian Boos, 14 years ago

Milestone: next-major-0.1X0.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 Remy Blank, 14 years ago

Priority: normalhigh

Excellent patch, thank you very much. I'll take care of applying it.

comment:7 by Remy Blank, 14 years ago

Milestone: 0.130.12.1
Owner: set to Remy Blank

comment:8 by Remy Blank, 14 years ago

Resolution: fixed
Status: newclosed

Awesome patch, works exactly as advertised, applied (with a few cosmetic fixes, see comment:5) in [10050]. Thanks!

comment:9 by Remy Blank, 14 years ago

Owner: changed from Remy Blank to Andrew C Martin <andrew.c.martin@…>

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Andrew C Martin <andrew.c.martin@…>.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Andrew C Martin <andrew.c.martin@…> to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.