Roundup Tracker - Issues

Issue 1667729

Title: Multiline properties in the history list
Type: Severity: normal
Components: User Interface Versions:
Status: closed Resolution: abandoned
Dependencies: Superseder:
Assigned To: rouilj Nosy List: ber, htrd, jpend, rouilj
Priority: normal Keywords: patch

Created on 2007-02-24 10:26 by htrd, last changed 2017-04-24 00:28 by rouilj.

File name Uploaded Description Edit Remove
roundup_diffs_in_history.diff htrd, 2007-02-24 10:26
roundup_diffs_in_history-2013-05-16.diff ber, 2013-05-16 20:02
msg2888 Author: [hidden] (htrd) Date: 2007-02-24 10:26
Ive previously submitted a patch to improve the formatting of multiline properties in the emailed change note.... this patch improves the presentation of these properties in the history display.

This patch uses difflib to break the change into added and removed lines. removals are displayed in strikethrough, and additions are given a yellow highlight.

One aspect of this patch could use additional review..... The 'current' dictionary always originally stored the property values *after* being passed through cgi.escape, however this feature needs to calculate the diffs on unescaped strings, therefore the hyperdb.String case now stores unescaped strings in that dictionary. It is obviously critical that input strings do not leak into the ouput html stream without passing through cgi.escape.
msg2889 Author: [hidden] (jpend) Date: 2007-09-11 04:25
The _ndiff_classes dict seems to be defined but not used.

I'm not immediately seeing why you need to pass unescaped strings to _string_history_diff. It seems like it would work just fine with escaped strings. Maybe if you explain my little brain will comprehend :)
msg2890 Author: [hidden] (anonymous) Date: 2007-09-11 08:06
Logged In: NO 

_ndiff_classes is used in _string_history_diff to generate css class names for each line based on whether it is an addition or deletion.

Re: moving the call to cgi.escape to the very end....

Yeah, I think what you suggest is probably safe. At the worst it would mean that difflib would produce a different set of changes (because of the way difflib resynchronises after a change). This relies on the cgi.escape making minimal changes. Would you like me to rework the patch?

- Toby (but not logged into sourceforge from this machine)

msg2891 Author: [hidden] (jpend) Date: 2007-09-11 17:08
If you want to rework it that would be great. It would be nice to keep changes minimal and I don't think cgi.escape will confuse difflib too much.

If not, I'll get around to it eventually :)  
msg4894 Author: [hidden] (ber) Date: 2013-05-16 20:02
just tried to see if the patch still applies to current hg tip.
It does with a few changes, see roundup_diffs_in_history-2013-05-16.diff.

I've ran the demo, but could not directly see the effects in the web interface
history for a regular issues.

Is there still interest in the patch? Can you describe what it does?
Justus proposed a change to make sure cgi.escape is used. So this would need to 
be checked anyway.
msg5623 Author: [hidden] (rouilj) Date: 2016-06-25 21:02
Ping, is anybody still interested in this, if not I'll close as as out
of date or abandoned in a month.
msg5965 Author: [hidden] (rouilj) Date: 2017-04-24 00:28
Closing as abandoned.
Date User Action Args
2017-04-24 00:28:41rouiljsetstatus: open -> closed
resolution: abandoned
messages: + msg5965
2016-06-25 21:02:47rouiljsetassignee: jpend -> rouilj
messages: + msg5623
nosy: + rouilj
2013-05-16 20:02:48bersetfiles: + roundup_diffs_in_history-2013-05-16.diff
nosy: + ber
messages: + msg4894
2007-02-24 10:26:41htrdcreate