Bazaar hookless email

Merge lp://staging/~kfogel/bzr-hookless-email/byte-limit into lp://staging/bzr-hookless-email

  1. byte-limit
  2. Merge into trunk-2a
Proposed by Karl Fogel
Status: Merged
Merged at revision: not available
Proposed branch: lp://staging/~kfogel/bzr-hookless-email/byte-limit
Merge into: lp://staging/bzr-hookless-email
Diff against target: None lines
To merge this branch: bzr merge lp://staging/~kfogel/bzr-hookless-email/byte-limit
Reviewer Review Type Date Requested Status
Wouter van Heyst Approve
Review via email: mp+11233@code.staging.launchpad.net
To post a comment you must log in.
Revision history for this message
Karl Fogel (kfogel) wrote :

Add a '--byte-limit' option, similar to '--line-limit' but with the limit based on the size of the diff in bytes not lines. Ensure that both limits are active, and that the diff-suppression message mentions both when applicable.

This resolves bug #424560.

Revision history for this message
Wouter van Heyst (larstiq) wrote :

Why the default byte limit of ~40k? I mean, it might be a very good choice, but seems a bit arbitray. My initial guess would be 80 * default line limit.

review: Approve
Revision history for this message
Karl Fogel (kfogel) wrote :

> Why the default byte limit of ~40k? I mean, it might be a very good choice,
> but seems a bit arbitray. My initial guess would be 80 * default line limit.

Oh, I just gathered some sample data and looked at how many bytes 1000 lines usually was. It turns out that 40 bytes per line average is normal for source code (I guess it would be more when the source code is in some Unicode encoding that uses more than one byte per char).

Thanks for the review!

Revision history for this message
Wouter van Heyst (larstiq) wrote :

> > Why the default byte limit of ~40k? I mean, it might be a very good choice,
> > but seems a bit arbitray. My initial guess would be 80 * default line
> limit.
>
> Oh, I just gathered some sample data and looked at how many bytes 1000 lines
> usually was. It turns out that 40 bytes per line average is normal for source
> code (I guess it would be more when the source code is in some Unicode
> encoding that uses more than one byte per char).

Research is much better than my guess, your changes have been pushed to trunk :)
Thanks for pinging me!

Revision history for this message
Karl Fogel (kfogel) wrote :

Wouter van Heyst <email address hidden> writes:
>> > Why the default byte limit of ~40k? I mean, it might be a very good choice,
>> > but seems a bit arbitray. My initial guess would be 80 * default line
>> limit.
>>
>> Oh, I just gathered some sample data and looked at how many bytes 1000 lines
>> usually was. It turns out that 40 bytes per line average is normal for source
>> code (I guess it would be more when the source code is in some Unicode
>> encoding that uses more than one byte per char).
>
> Research is much better than my guess, your changes have been pushed
> to trunk :) Thanks for pinging me!

Thank you!

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bzr_hookless_email.py'
2--- bzr_hookless_email.py 2009-04-14 14:01:01 +0000
3+++ bzr_hookless_email.py 2009-08-22 05:48:05 +0000
4@@ -243,7 +243,9 @@
5 msg['Date'] = format_date(revision.timestamp, revision.timezone,
6 date_fmt='%a, %d %b %Y %H:%M:%S')
7
8- if options.line_limit != 0:
9+ # Currently we obey the narrowest restriction available: if
10+ # either -l or -b says to send no diff, then we send no diff.
11+ if options.line_limit != 0 and options.byte_limit != 0:
12 diff = cStringIO.StringIO()
13 diff_name = 'r%d.diff' % rev1
14
15@@ -265,14 +267,31 @@
16 self._branch.repository.unlock()
17
18 show_diff_trees(tree_old, tree_new, diff)
19- numlines = diff.getvalue().count('\n') + 1
20- if numlines <= options.line_limit or options.line_limit < 0:
21- diff = diff.getvalue()
22- else:
23- diff = ("Diff too large for email (%d lines, the limit is %d).\n" %
24- (numlines, options.line_limit))
25-
26- msg.add_inline_attachment(diff, diff_name)
27+ diff_val = diff.getvalue()
28+
29+ # The byte_limit and line_limit interact with each other.
30+ # We observe both limits -- whichever is more restrictive
31+ # wins. But the message announcing the restriction should
32+ # say that both of them are in play, if that is the case,
33+ # so that anyone upping the limit knows to tweak both.
34+ diff_already_suppressed = False
35+ numlines = diff_val.count('\n') + 1
36+ numbytes = len(diff_val)
37+
38+ if options.line_limit > 0:
39+ if numlines > options.line_limit:
40+ diff_already_suppressed = True
41+ diff_val = ("Diff too large for email:\n"
42+ " %d lines (line limit is %d)\n" %
43+ (numlines, options.line_limit))
44+ if options.byte_limit > 0:
45+ if numbytes > options.byte_limit:
46+ if not diff_already_suppressed:
47+ diff_val = "Diff too large for email:\n"
48+ diff_val += (" %d bytes (byte limit is %d)\n" %
49+ (numbytes, options.byte_limit))
50+
51+ msg.add_inline_attachment(diff_val, diff_name)
52
53 return msg
54
55@@ -345,7 +364,11 @@
56
57 p.add_option('-l', '--line-limit', action='store', type='int', metavar='N',
58 help=('do not attach the diff if bigger than N lines '
59- '(default: 1000; -1: unlimited; 0: no diff)'))
60+ '(default: 1000; -1: no line limit; 0: no diff) '))
61+
62+ p.add_option('-b', '--byte-limit', action='store', type='int', metavar='N',
63+ help=('do not attach the diff if bigger than N bytes '
64+ '(default: 40000; -1: no byte limit; 0: no diff) '))
65
66 p.add_option('-d', '--daemon', action='store_true', dest='daemon',
67 help='run as a daemon, watching branches with inotify')
68@@ -356,7 +379,8 @@
69 p.add_option('-o', '--logfile', action='store', metavar='FILE',
70 help='print daemon errors to FILE (default: /dev/null)')
71
72- p.set_defaults(bg_fork=True, logfile='/dev/null', line_limit=1000)
73+ p.set_defaults(bg_fork=True, logfile='/dev/null',
74+ line_limit=1000, byte_limit=40000)
75
76 options, args = p.parse_args()
77

Subscribers

People subscribed via source and target branches

to all changes: