Edgewall Software
Modify

Opened 8 years ago

Closed 8 years ago

Last modified 8 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:
Release Notes:
API 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 8 years ago.
textarea_email_formatting_after_patch.png (21.1 KB ) - added by anonymous 8 years ago.
textarea_email_formatting.patch (948 bytes ) - added by anonymous 8 years ago.
textarea_email_formatting_2.patch (1.4 KB ) - added by andrew.c.martin@… 8 years ago.

Download all attachments as: .zip

Change History (13)

Changed 8 years ago by anonymous

Changed 8 years ago by anonymous

Changed 8 years ago by anonymous

comment:2 Changed 8 years ago by Remy Blank

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 in reply to:  2 Changed 8 years ago by Christian Boos

Replying to rblank:

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

#3914.

Changed 8 years ago by andrew.c.martin@…

comment:4 in reply to:  2 Changed 8 years ago by andrew.c.martin@…

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 Changed 8 years ago by Christian Boos

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 Changed 8 years ago by Remy Blank

Priority: normalhigh

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

comment:7 Changed 8 years ago by Remy Blank

Milestone: 0.130.12.1
Owner: set to Remy Blank

comment:8 Changed 8 years ago by Remy Blank

Resolution: fixed
Status: newclosed

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

comment:9 Changed 8 years ago by Remy Blank

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.
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.