Skip Menu |
 

This queue is for tickets about the activitymail CPAN distribution.

Report information
The Basics
Id: 16431
Status: rejected
Priority: 0/
Queue: activitymail

People
Owner: Nobody in particular
Requestors: morfran [...] gmail.com
Cc:
AdminCc:

Bug Information
Severity: Wishlist
Broken in: 1.23
Fixed in: (no value)

Attachments


MIME-Version: 1.0
X-Mailer: MIME-tools 5.418 (Entity 5.418)
Subject: Patch to allow truncation of long diff output
Content-Type: multipart/mixed; boundary="----------=_1134347307-17831-0"
Content-Length: 0
Content-Type: text/plain
Content-Disposition: inline
Content-Transfer-Encoding: binary
X-RT-Original-Encoding: iso-8859-1
Content-Length: 442
Download (untitled) / with headers
text/plain 442b
Really large diff attachments were making our mail clients drag so we added a -T (truncate diffs) option to activitymail locally to only give a limited amount (50 lines) of diff output where the diff is larger than a size given to the -T option. Full diffs are given as usual if -T isn't specified. I've attached a patch, it would be nice to get this added to the main distribution if you think that it would be of use to other people also.
Content-Type: text/x-patch; name="truncate.patch"
Content-Disposition: inline; filename="truncate.patch"
Content-Transfer-Encoding: binary
X-RT-Original-Encoding: iso-8859-1
Content-Length: 3552
Download truncate.patch
text/x-diff 3.4k
--- activitymail.orig Sat Dec 10 08:41:24 2005 +++ activitymail Mon Dec 12 13:24:57 2005 @@ -37,9 +37,9 @@ use vars qw($opt_f $opt_r $opt_l $opt_m $opt_t $opt_n $opt_i $opt_p $opt_c $opt_s $opt_h $opt_d $opt_a $opt_D $opt_o $opt_e $opt_u $opt_g $opt_w $opt_H $opt_B $opt_j $opt_M $opt_S $opt_v $opt_V $opt_N - $opt_I $opt_E $opt_q $opt_Q $opt_P $opt_U + $opt_I $opt_E $opt_q $opt_Q $opt_P $opt_U $opt_T ); - getopts('f:lr:m:t:nipcs:dhaDo:e:u:gw:HB:j:M:SvVN:I:E:qQP:U'); + getopts('f:lr:m:t:nipcs:dhaDo:e:u:gw:HB:j:M:SvVN:I:E:qQP:UT:'); } ############################################################################## @@ -434,23 +434,33 @@ # to be passed to diff are okay when the whole argument is in # quotation marks. $fn =~ s/\s+/_/g if $opt_U; + my $fdiff; if ($r1 eq 'NONE') { # It's a new file. if (-e $file) { # Compare to /dev/null. - $diffs .= `$opt_j $opt_o -L '$opt_N' '$opt_N' -L '$fn' '$file'`; + $fdiff = `$opt_j $opt_o -L '$opt_N' '$opt_N' -L '$fn' '$file'`; } else { # Otherwise, read the file from a non-changing update and pipe # that to diff. - $diffs .= `$opt_e -fn update -r $r2 -p '$file' | $opt_j $opt_o -L '$opt_N' '$opt_N' -L '$fn' -`; + $fdiff = `$opt_e -fn update -r $r2 -p '$file' | $opt_j $opt_o -L '$opt_N' '$opt_N' -L '$fn' -`; } } elsif ($r2 eq 'NONE') { # The file has been deleted. Read it from a non-changing update # and pipe it to diff. - $diffs .= `$opt_e -fn update -r $r1 -p '$file' | $opt_j $opt_o -L '$fn' - -L '$opt_N' '$opt_N'`; + $fdiff = `$opt_e -fn update -r $r1 -p '$file' | $opt_j $opt_o -L '$fn' - -L '$opt_N' '$opt_N'`; } else { # We actually have CVS diff the two versions. - $diffs .= `$opt_e -f diff -kk -L '$fn' -L '$fn' $opt_o -r $r1 -r $r2 '$file'`; + $fdiff = `$opt_e -f diff -kk -L '$fn' -L '$fn' $opt_o -r $r1 -r $r2 '$file'`; + } + + if (defined($opt_T) && length($fdiff) > $opt_T) { + $fdiff =~ s/^((?:[^\n]*\n){50}).*$/$1/sm; + $fdiff .= "\n[...]\n\ndiff output truncated"; + $fdiff .= " (exceeded maximum length of $opt_T characters)\n"; + $diffs .= $fdiff; + } else { + $diffs .= $fdiff; } } return $diffs; @@ -957,6 +967,7 @@ -u <email> User email address from which email should be sent. -g Use \$USER environment variable to group commits. -M <size> Maximum size of emailed messages in kilobytes. + -T <size> Maximum size of diff output for any file. -V Include revision numbers in the email message. -H Use HTML for generated emails. -w <url> Include links to specified CVSWeb url in email. @@ -1183,6 +1194,12 @@ instead. This option is useful if your repository contains large binary files not prevented from be diffed by C<-B>, or when adding many files at once. In those cases, failing to use this option may result in broken mail clients. + +=item -T <size> + +Max length for diff output to be attached to email messages, in characters. If +diff output greater than this size would be attached then the first 10 lines are +returned with a message explaining that the diff output has been truncated. =item -V
MIME-Version: 1.0
X-Mailer: MIME-tools 5.418 (Entity 5.418)
Content-Disposition: inline
Message-Id: <rt-3.5.HEAD-28145-1144107572-1450.16431-0-0 [...] rt.cpan.org>
Content-Type: text/plain; charset="utf8"
Content-Transfer-Encoding: binary
X-RT-Original-Encoding: utf-8
X-RT-Original-Encoding: utf-8
Content-Length: 944
Download (untitled) / with headers
text/plain 944b
On Sun Dec 11 19:28:27 2005, guest wrote: Show quoted text
> Really large diff attachments were making our mail clients drag so > we added a -T (truncate diffs) option to activitymail locally to only > give a limited amount (50 lines) of diff output where the diff is > larger than a size given to the -T option. > > Full diffs are given as usual if -T isn't specified. > > I've attached a patch, it would be nice to get this added to the main > distribution if you think that it would be of use to other people > also.
I'm not sure how this is necessary. If you have large diffs, then have them sent as attachments, instead. Any mail client worth its salt will show a message with attachments very quickly, even if the attachments are very large. I might see a use for making it so that the total size of the diff output is limited. This would be similar to the --max-diff-length option recently added to SVN::Notify. Would that work for you? Best, David
MIME-Version: 1.0
In-Reply-To: <rt-3.5.HEAD-28145-1144107572-1450.16431-0-0 [...] rt.cpan.org>
X-Mailer: MIME-tools 5.418 (Entity 5.418)
Content-Disposition: inline
References: <rt-3.5.HEAD-28145-1144107572-1450.16431-0-0 [...] rt.cpan.org>
Message-Id: <rt-3.5.HEAD-28152-1144112888-1148.16431-0-0 [...] rt.cpan.org>
Content-Type: text/plain; charset="utf8"
Content-Transfer-Encoding: binary
From: morfran [...] gmail.com
X-RT-Original-Encoding: utf-8
X-RT-Original-Encoding: utf-8
Content-Length: 1304
Download (untitled) / with headers
text/plain 1.2k
On Mon Apr 03 19:39:32 2006, DWHEELER wrote: Show quoted text
> I'm not sure how this is necessary. If you have large diffs, then have > them sent as attachments, instead. Any mail client worth its salt will > show a message with attachments very quickly, even if the attachments > are very large.
Well, the patch works for me - the diffs are being sent as attachments and were before I implemented the change. I'm using Thunderbird to read mail at work and it just crawls with really large diffs. I'm accessing mail via IMAP and it might be partly due to that that I'm having problems. I doubt that I would have the same trouble using Mutt (which I use for my personal mail), but that's not a change that I want to make at work since things are going just fine with Thunderbird. Show quoted text
> I might see a use for making it so that the total size of the diff > output is limited. This would be similar to the --max-diff-length option > recently added to SVN::Notify. Would that work for you?
Sounds like the same thing but with the truncated size of the diff output being set by the parameter rather than always giving 50 lines. If so, yes, that sounds like a good solution and preferable to the way I have it since that way you get more of the diff when truncation happens. Thanks for taking a look at this. Cheers, Matt.
X-Scanned-BY: AMaViS-ng at bestpractical
MIME-Version: 1.0 (Apple Message framework v749.3)
X-Spam-Status: No, hits=-2.6 required=8.0 tests=BAYES_00
In-Reply-To: <rt-3.5.HEAD-28152-1144112888-1148.16431-5-0 [...] rt.cpan.org>
X-Mailer: Apple Mail (2.749.3)
Received-SPF: neutral (x1.develooper.com: local policy)
References: <RT-Ticket-16431 [...] rt.cpan.org> <rt-3.5.HEAD-28145-1144107572-1450.16431-5-0 [...] rt.cpan.org> <rt-3.5.HEAD-28152-1144112888-1148.16431-5-0 [...] rt.cpan.org>
Content-Type: text/plain; charset="utf-8"; delsp="yes"; format="flowed"
X-RT-Original-Encoding: US-ASCII
Received: from localhost (localhost.localdomain [127.0.0.1]) by diesel.bestpractical.com (Postfix) with ESMTP id C520E4D832A for <cpan-bug+activitymail [...] diesel.bestpractical.com>; Mon, 3 Apr 2006 21:14:59 -0400 (EDT)
Received: from la.mx.develooper.com (x1.develooper.com [63.251.223.170]) by diesel.bestpractical.com (Postfix) with SMTP id 63A974D8329 for <bug-activitymail [...] rt.cpan.org>; Mon, 3 Apr 2006 21:14:59 -0400 (EDT)
Received: (qmail 22635 invoked by alias); 4 Apr 2006 01:14:52 -0000
Received: from mail1.sea5.speakeasy.net (HELO mail1.sea5.speakeasy.net) (69.17.117.3) by la.mx.develooper.com (qpsmtpd/0.28) with ESMTP; Mon, 03 Apr 2006 18:14:49 -0700
Received: (qmail 16345 invoked from network); 4 Apr 2006 01:14:43 -0000
Received: from dsl093-038-250.pdx1.dsl.speakeasy.net (HELO [192.168.1.20]) (davidw [...] [66.93.38.250]) (envelope-sender <david [...] kineticode.com>) by mail1.sea5.speakeasy.net (qmail-ldap-1.03) with RC4-SHA encrypted SMTP for <bug-activitymail [...] rt.cpan.org>; 4 Apr 2006 01:14:43 -0000
Delivered-To: cpan-bug+activitymail [...] diesel.bestpractical.com
Subject: Re: [rt.cpan.org #16431] Patch to allow truncation of long diff output
Return-Path: <david [...] kineticode.com>
X-Spam-Check-BY: la.mx.develooper.com
X-Original-To: cpan-bug+activitymail [...] diesel.bestpractical.com
Date: Mon, 3 Apr 2006 18:14:41 -0700
Message-Id: <FEB6B033-377C-4221-B7BC-37AF8F2D39E3 [...] kineticode.com>
To: bug-activitymail [...] rt.cpan.org
Content-Transfer-Encoding: 7bit
From: David Wheeler <david [...] kineticode.com>
X-RT-Original-Encoding: utf-8
RT-Message-ID: <rt-3.5.HEAD-28162-1144113304-919.16431-0-0 [...] rt.cpan.org>
Content-Length: 1503
Download (untitled) / with headers
text/plain 1.4k
On Apr 3, 2006, at 18:08, Guest via RT wrote: Show quoted text
> Well, the patch works for me - the diffs are being sent as > attachments and were before I implemented the change. I'm using > Thunderbird to read mail at work and it just crawls with really large > diffs. I'm accessing mail via IMAP and it might be partly due to that > that I'm having problems. I doubt that I would have the same trouble > using Mutt (which I use for my personal mail), but that's not a change > that I want to make at work since things are going just fine with > Thunderbird.
Sounds like a Thunderbird bug. I use IMAP with Mail.app, and I can read messages with large attachments just fine. Sometimes it takes a while to download messages with large diffs, but I'm willing to wait. That's due to the overhead of IMAP and my network connection, neither of which is that significant. Show quoted text
> Sounds like the same thing but with the truncated size of the diff > output being set by the parameter rather than always giving 50 lines. > If so, yes, that sounds like a good solution and preferable to the way > I have it since that way you get more of the diff when truncation > happens.
Well, it means that it truncates it for the total diff size, rather than for each file changed. But really, if the diffs are that big, and you're willing to have them truncated, why have them at all? Personally, I like to see *every single change* committed to a repository that I'm working on. But maybe that's just me. Cheers, David
MIME-Version: 1.0
In-Reply-To: <rt-3.5.HEAD-28162-1144113304-919.16431-0-0 [...] rt.cpan.org>
X-Mailer: MIME-tools 5.418 (Entity 5.418)
Content-Disposition: inline
References: <RT-Ticket-16431 [...] rt.cpan.org> <rt-3.5.HEAD-28145-1144107572-1450.16431-5-0 [...] rt.cpan.org> <rt-3.5.HEAD-28152-1144112888-1148.16431-5-0 [...] rt.cpan.org> <FEB6B033-377C-4221-B7BC-37AF8F2D39E3 [...] kineticode.com> <rt-3.5.HEAD-28162-1144113304-919.16431-0-0 [...] rt.cpan.org>
Message-Id: <rt-3.5.HEAD-28152-1144113814-1096.16431-0-0 [...] rt.cpan.org>
Content-Type: text/plain; charset="utf8"
Content-Transfer-Encoding: binary
From: morfran [...] gmail.com
X-RT-Original-Encoding: utf-8
X-RT-Original-Encoding: utf-8
Content-Length: 970
Download (untitled) / with headers
text/plain 970b
On Mon Apr 03 21:15:04 2006, david@kineticode.com wrote: Show quoted text
> Well, it means that it truncates it for the total diff size, rather > than for each file changed. But really, if the diffs are that big, > and you're willing to have them truncated, why have them at all? > Personally, I like to see *every single change* committed to a > repository that I'm working on. But maybe that's just me.
I see what you mean. Generally I want to see every single change too. The patch was written in response to a small number of files in our CVS tree that produced huge diffs every time something changed and which would cause Thunderbird to choke. If that happened at the same time as something that I was actually interested in, then it made it very difficult to go and read the bits I was interested in - so the per-file diff truncation is useful to me. I'll continue to solve my problem in this way by patching future activitymail versions, if necessary. Cheers, Matt.


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.