You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Alan Barrett <ap...@cequrux.com> on 2005/11/02 13:00:08 UTC
[PATCH] mailer.py: truncate_diff_lines feature
[[[
Add truncate_diff_lines feature to mailer.py hook script. If this
is set to a non-zero value, then diffs longer than the specified
number of lines will be truncated in email messages.
mailer.py (TextCommitRenderer::generate_content): Obtain
truncate_diff_lines value from cfg, and pass it through in the data sent
to renderer.render().
(TextCommitRenderer::render): Pass data.truncate_diff_lines through to
self._render_diffs().
(TextCommitRenderer::render::_render_diffs): New truncate_diff_lines
arg, and new logic to implement the truncation.
mailer.conf.example: Add commented-out example for truncate_diff_lines.
]]]
=== tools/hook-scripts/mailer.py
==================================================================
--- mailer.py (revision 18531)
+++ mailer.py (revision 18532)
@@ -589,6 +589,12 @@
show_nonmatching_paths = cfg.get('show_nonmatching_paths', group, params) \
or 'yes'
+ try:
+ truncate_diff_lines = int(
+ cfg.get('truncate_diff_lines', group, params))
+ except ValueError:
+ truncate_diff_lines = 0
+
# figure out the lists of changes outside the selected path-space
other_added_data = other_removed_data = other_modified_data = [ ]
if len(paths) != len(changelist) and show_nonmatching_paths != 'no':
@@ -617,6 +623,7 @@
diffs=DiffGenerator(changelist, paths, True, cfg, repos, date, group,
params, diffsels, diffurls, pool),
other_diffs=other_diffs,
+ truncate_diff_lines=truncate_diff_lines,
)
renderer.render(data)
@@ -900,10 +907,10 @@
w('\nLog:\n%s\n' % data.log)
- self._render_diffs(data.diffs)
+ self._render_diffs(data.diffs, data.truncate_diff_lines)
if data.other_diffs:
w('\nDiffs of changes in other areas also in this revision:\n')
- self._render_diffs(data.other_diffs)
+ self._render_diffs(data.other_diffs, data.truncate_diff_lines)
def _render_list(self, header, data_list):
if not data_list:
@@ -934,7 +941,7 @@
w(' - copied%s from r%d, %s%s\n'
% (text, d.base_rev, d.base_path, is_dir))
- def _render_diffs(self, diffs):
+ def _render_diffs(self, diffs, truncate_diff_lines):
w = self.output.write
for diff in diffs:
@@ -971,8 +978,15 @@
w('Binary files. No diff available.\n')
continue
+ nlines = 0
for line in diff.content:
- w(line.raw)
+ if truncate_diff_lines > 0 and nlines >= truncate_diff_lines:
+ w('*** Diff too large. Truncated after %d lines.\n' \
+ % (truncate_diff_lines))
+ break
+ else:
+ w(line.raw)
+ nlines = nlines + 1
class Repository:
=== tools/hook-scripts/mailer.conf.example
==================================================================
--- mailer.conf.example (revision 18531)
+++ mailer.conf.example (revision 18532)
@@ -224,6 +224,12 @@
# Set to 0 to turn off.
#truncate_subject = 200
+# Diff length limit. The generated diff will be truncated after this many
+# lines. The limit applies independently to each file for which a diff
+# is generated.
+# Set to 0 to turn off.
+#truncate_diff_lines = 200
+
# --------------------------------------------------------------------------
[maps]
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] mailer.py: truncate_diff_lines feature
Posted by Joseph Galbraith <ga...@vandyke.com>.
Alan Barrett wrote:
> On Wed, 02 Nov 2005, C. Michael Pilato wrote:
>> Alan Barrett <ap...@cequrux.com> writes:
>>> [[[
>>> Add truncate_diff_lines feature to mailer.py hook script. If this
>>> is set to a non-zero value, then diffs longer than the specified
>>> number of lines will be truncated in email messages.
>> "truncate_diff_lines" sounds like you're chopping characters off the
>> ends of long lines, not chopping lines off the end of a long diff.
>> Suggest "truncate_diff".
>
> Fine.
>
>> I'm somewhat ambivalent about this feature addition, but do have one
>> concern: I'd prefer it if the act of truncating a diff would cause an
>> application of that diff data as a patch to fail in some noticable
>> way. In a sea of 100 diffs in a single email, it'd be really easy to
>> miss a single instance of a truncated one -- what happens to the poor
>> guy that tries to apply that email as a patch and succeeds because the
>> truncation happened to line up with a hunk boundary?
>
> I'll investigate making "patch" fail in a noticeable way.
>
> This feature is intended for use in cases where the email is read by
> people who want to receive a 10-line diff when somebody makes a small
> change, but don't want to receive a 1000-line mail message when somebody
> creates a new 1000-line file, or when somebody makes many changes
> throughout a large file. If you are sending mail to an automated
> process, or to people who will want to treat the mail as a patch to be
> applied, then you shouldn't be using this feature.
Just a confirmation: this would be very useful in our
environment; we don't apply patches from the email,
we use the diffs to increase visibility and get
additional code review.
But a 10000 line diff does not accomplist these goals
because its so big people just delete it instead of
reading it. (We have very few of these and they are
almost always merging changes from trunk to branch.)
Thanks,
Joseph
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] mailer.py: truncate_diff_lines feature
Posted by Julian Foad <ju...@btopenworld.com>.
Branko Čibej wrote:
> Alan Barrett wrote:
>
>> This feature is intended for use in cases where the email is read by
>> people who want to receive a 10-line diff when somebody makes a small
>> change, but don't want to receive a 1000-line mail message when somebody
>> creates a new 1000-line file, or when somebody makes many changes
>> throughout a large file. If you are sending mail to an automated
>> process, or to people who will want to treat the mail as a patch to be
>> applied, then you shouldn't be using this feature.
>
> In that case, I suggest defining some maximum diff size, and if the diff
> exceeds that size to not in include it in the mail at all. Just add a
> note, e.g. "Diff ommitted, because it's longer than N lines.
>
> Truncating the diff at some arbitrary length is totally wrong because
> it's a) lying to the user,
Huh? IIRC, the truncated diff was followed by text saying that it had been
truncated. How is that lying?
> and b) produces a patch that has nothing to
> do with the actual change.
If you are talking about the result of applying it with a "patch" program, then
(a) it is incomplete; it does not have "nothing to do with the actual change",
and (b) steps have been taken to make sure a "patch" program errors on it, so
that's no argument against it.
I have no strong opinion one way or the other. I suggest following whatever
practice we already have in other scripts if any. As a point in favour of
Alan's method, the reader may often find it useful to see just the first few
changes, especially when many similar changes have been made throughout a large
file, or when a new file is added and begins with a description of itself.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] mailer.py: truncate_diff_lines feature
Posted by Branko Čibej <br...@xbc.nu>.
Alan Barrett wrote:
> On Thu, 03 Nov 2005, Branko ?ibej wrote:
>
>> In that case, I suggest defining some maximum diff size, and if the
>> diff exceeds that size to not in include it in the mail at all. Just
>> add a note, e.g. "Diff ommitted, because it's longer than N lines.
>>
>
> I can add that as another feature, if there is demand for it. The
> behaviour that I have already implemented is the one that's more useful
> to me. Two other people have said that this behaviour is useful to them
> too.
>
> How about something like this (also changing the variable name, because
> it would no longer be just for truncation):
>
> max_diff_lines = 0 # no limit (default)
> max_diff_lines = 100 # diffs longer than 100 lines are omitted
> max_diff_lines = -100 # diffs longer than 100 lines are truncated
>
There's no need to be so terse. This is python, after all, so string
parsing is easy.
max_diff_lines = # no value, no limit (default)
max_diff_lines = (\d+)(\s+(truncate|omit))?
So, if you wanted to see at most 100 lines of diff and truncate the
rest, you'd say
max_diff_lines = 100 truncate
if you wanted to omit diffs longer than 100 lines,
max_diff_lines = 100 omit
and
max_diff_lines = 100
would take a suitable default (probably truncate)
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] mailer.py: truncate_diff_lines feature
Posted by Alan Barrett <ap...@cequrux.com>.
On Thu, 03 Nov 2005, Branko ?ibej wrote:
> In that case, I suggest defining some maximum diff size, and if the
> diff exceeds that size to not in include it in the mail at all. Just
> add a note, e.g. "Diff ommitted, because it's longer than N lines.
I can add that as another feature, if there is demand for it. The
behaviour that I have already implemented is the one that's more useful
to me. Two other people have said that this behaviour is useful to them
too.
How about something like this (also changing the variable name, because
it would no longer be just for truncation):
max_diff_lines = 0 # no limit (default)
max_diff_lines = 100 # diffs longer than 100 lines are omitted
max_diff_lines = -100 # diffs longer than 100 lines are truncated
--apb (Alan Barrett)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] mailer.py: truncate_diff_lines feature
Posted by Branko Čibej <br...@xbc.nu>.
Alan Barrett wrote:
> This feature is intended for use in cases where the email is read by
> people who want to receive a 10-line diff when somebody makes a small
> change, but don't want to receive a 1000-line mail message when somebody
> creates a new 1000-line file, or when somebody makes many changes
> throughout a large file. If you are sending mail to an automated
> process, or to people who will want to treat the mail as a patch to be
> applied, then you shouldn't be using this feature.
>
In that case, I suggest defining some maximum diff size, and if the diff
exceeds that size to not in include it in the mail at all. Just add a
note, e.g. "Diff ommitted, because it's longer than N lines.
Truncating the diff at some arbitrary length is totally wrong because
it's a) lying to the user, and b) produces a patch that has nothing to
do with the actual change. Better to simply say that there's no diff,
and let people do the necessary "svn diff" themselves.
IIRC the mailer can produce ViewCVS diff links, so including that link
(if so configured) would be a nice shortcut.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] mailer.py: truncate_diff_lines feature
Posted by Alan Barrett <ap...@cequrux.com>.
On Wed, 02 Nov 2005, C. Michael Pilato wrote:
> Alan Barrett <ap...@cequrux.com> writes:
> > [[[
> > Add truncate_diff_lines feature to mailer.py hook script. If this
> > is set to a non-zero value, then diffs longer than the specified
> > number of lines will be truncated in email messages.
>
> "truncate_diff_lines" sounds like you're chopping characters off the
> ends of long lines, not chopping lines off the end of a long diff.
> Suggest "truncate_diff".
Fine.
> I'm somewhat ambivalent about this feature addition, but do have one
> concern: I'd prefer it if the act of truncating a diff would cause an
> application of that diff data as a patch to fail in some noticable
> way. In a sea of 100 diffs in a single email, it'd be really easy to
> miss a single instance of a truncated one -- what happens to the poor
> guy that tries to apply that email as a patch and succeeds because the
> truncation happened to line up with a hunk boundary?
I'll investigate making "patch" fail in a noticeable way.
This feature is intended for use in cases where the email is read by
people who want to receive a 10-line diff when somebody makes a small
change, but don't want to receive a 1000-line mail message when somebody
creates a new 1000-line file, or when somebody makes many changes
throughout a large file. If you are sending mail to an automated
process, or to people who will want to treat the mail as a patch to be
applied, then you shouldn't be using this feature.
--apb (Alan Barrett)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] mailer.py: truncate_diff_lines feature
Posted by Alan Barrett <ap...@cequrux.com>.
On Wed, 02 Nov 2005, C. Michael Pilato wrote:
> "truncate_diff_lines" sounds like you're chopping characters off the
> ends of long lines, not chopping lines off the end of a long diff.
> Suggest "truncate_diff".
Or "max_diff_lines" or "diff_max_lines"?
--apb (Alan Barrett)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] mailer.py: truncate_diff_lines feature
Posted by André Malo <nd...@perlig.de>.
Well, ehm, perhaps it's time to speak up for me ;-)
* Greg Stein wrote:
> On Thu, Nov 03, 2005 at 10:09:30AM -0800, Garrett Rooney wrote:
> >...
> > I also haven't been paying overly close attention here, but it strikes
> > me that some of this may be reinventing the wheel a bit. I believe
> > that svnmailer (http://opensource.perlig.de/svnmailer/) already has
> > this kind of functionality (splitting diffs across multiple mails,
> > excluding them in favor of viewcvs link if they're too long, etc).
>
> Don't you mean that svnmailer is reinventing? Andre decided to fork
> rather than contribute his work back to Subversion.
The decision has not been made yet (on my side at least). For now I'd
consider it as an external branch.
> I was very interested in some of the work that Andre did with
> svnmailer and went to take a look. Holy crap. There are a lot of
> files, a lot of complicating abstractions, and other stuff in there.
> I'm certainly biased, but I find mailer.py a lot more workable.
That's funny ;-). The main reason to rewrite it from the scratch was, that I
found mailer.py not workable. I simply wanted to extend it for my needs and
found it wasn't that easy, because it's a monolithic block of code which
has to be changed as a whole if you want to extend it. Further it contains
too less documentation and that (not to speak of the bindings which *are*
not pythonic or documented at all, I find myself looking at the subversion
headers and generated C files to figure things out).
Anyway, I know, the notifier modules of svnmailer are a bit messy and I'm
currently refactoring it all (for the 1.1 release). A lot of the base
modules actually belong outside the svnmailer, but it's a bit like shipping
APR with httpd or subversion.
My point is, that the svnmailer is not ready yet to be even considered to be
contributed back. Just wanted to let you know.
nd
--
"Das Verhalten von Gates hatte mir bewiesen, dass ich auf ihn und seine
beiden Gefährten nicht zu zählen brauchte" -- Karl May, "Winnetou III"
Im Westen was neues: <http://pub.perlig.de/books.html#apache2>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] mailer.py: truncate_diff_lines feature
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 11/6/05, Greg Stein <gs...@lyra.org> wrote:
> On Thu, Nov 03, 2005 at 10:09:30AM -0800, Garrett Rooney wrote:
> >...
> > I also haven't been paying overly close attention here, but it strikes
> > me that some of this may be reinventing the wheel a bit. I believe
> > that svnmailer (http://opensource.perlig.de/svnmailer/) already has
> > this kind of functionality (splitting diffs across multiple mails,
> > excluding them in favor of viewcvs link if they're too long, etc).
>
> Don't you mean that svnmailer is reinventing? Andre decided to fork
> rather than contribute his work back to Subversion.
Well, perhaps re-reinventing then ;-)
Honestly, I don't know much about the history of the mailer programs,
just that I'd used svnmailer before and that it had these sort of
features already, that's all I wanted to say.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] mailer.py: truncate_diff_lines feature
Posted by Greg Stein <gs...@lyra.org>.
On Thu, Nov 03, 2005 at 10:09:30AM -0800, Garrett Rooney wrote:
>...
> I also haven't been paying overly close attention here, but it strikes
> me that some of this may be reinventing the wheel a bit. I believe
> that svnmailer (http://opensource.perlig.de/svnmailer/) already has
> this kind of functionality (splitting diffs across multiple mails,
> excluding them in favor of viewcvs link if they're too long, etc).
Don't you mean that svnmailer is reinventing? Andre decided to fork
rather than contribute his work back to Subversion.
I was very interested in some of the work that Andre did with
svnmailer and went to take a look. Holy crap. There are a lot of
files, a lot of complicating abstractions, and other stuff in there.
I'm certainly biased, but I find mailer.py a lot more workable.
And if people want to contribute improvements back to the core
distribution, then how can that be bad?
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] mailer.py: truncate_diff_lines feature
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 03 Nov 2005 09:55:35 -0600, kfogel@collab.net <kf...@collab.net> wrote:
> I came into this thread late, sorry.
>
> It's pointless to include anything less than 100% of a diff. There is
> no way to know whether the most complicated, review-needing part of
> the change is visible or not -- it would just be a roll of the dice
> each time. Instead, just omit diffs that exceed the maximum size,
> replacing them with a "[diff omitted because too large]" message, and
> provide a ViewCVS link (or whatever) for all diffs, both those
> included inline and those omitted.
>
I also haven't been paying overly close attention here, but it strikes
me that some of this may be reinventing the wheel a bit. I believe
that svnmailer (http://opensource.perlig.de/svnmailer/) already has
this kind of functionality (splitting diffs across multiple mails,
excluding them in favor of viewcvs link if they're too long, etc).
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] mailer.py: truncate_diff_lines feature
Posted by kf...@collab.net.
I came into this thread late, sorry.
It's pointless to include anything less than 100% of a diff. There is
no way to know whether the most complicated, review-needing part of
the change is visible or not -- it would just be a roll of the dice
each time. Instead, just omit diffs that exceed the maximum size,
replacing them with a "[diff omitted because too large]" message, and
provide a ViewCVS link (or whatever) for all diffs, both those
included inline and those omitted.
See http://subversion.tigris.org/issues/show_bug.cgi?id=2213 and the
mail threads linked to from it. In particular, see
http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=92636 and
the proposed "--diff-alternative" option. I haven't had time to apply
that patch, unfortunately, but I think it's the right way to go...
Best,
-Karl
--
www.collab.net <> CollabNet | Distributed Development On Demand
Alan Barrett <ap...@cequrux.com> writes:
> On Wed, 02 Nov 2005, C. Michael Pilato wrote:
> > "truncate_diff_lines" sounds like you're chopping characters off the
> > ends of long lines, not chopping lines off the end of a long diff.
> > Suggest "truncate_diff".
>
> I went with "truncate_diffs". A plural word seemed better to me.
>
> > I'm somewhat ambivalent about this feature addition, but do have one
> > concern: I'd prefer it if the act of truncating a diff would cause an
> > application of that diff data as a patch to fail in some noticable
> > way.
>
> Done. I append a new patch.
>
> [[[
> Add truncate_diffs feature to mailer.py hook script. If this
> is set to a non-zero value, then diffs longer than the specified
> number of lines will be truncated in email messages.
>
> * mailer.py (TextCommitRenderer.generate_content): Obtain
> truncate_diffs value from cfg, and pass it through in the data sent
> to renderer.render().
> (TextCommitRenderer.render): Pass data.truncate_diffs through to
> self._render_diffs().
> (TextCommitRenderer.render._render_diffs): New truncate_diffs
> arg, and new logic to implement the truncation.
> * mailer.conf.example: Add commented-out example for truncate_diffs.
> ]]]
> === mailer.py
> ==================================================================
> --- mailer.py (revision 18413)
> +++ mailer.py (revision 18535)
> @@ -589,6 +589,12 @@
> show_nonmatching_paths = cfg.get('show_nonmatching_paths', group, params) \
> or 'yes'
>
> + try:
> + truncate_diffs = int(
> + cfg.get('truncate_diffs', group, params))
> + except ValueError:
> + truncate_diffs = 0
> +
> # figure out the lists of changes outside the selected path-space
> other_added_data = other_removed_data = other_modified_data = [ ]
> if len(paths) != len(changelist) and show_nonmatching_paths != 'no':
> @@ -617,6 +623,7 @@
> diffs=DiffGenerator(changelist, paths, True, cfg, repos, date, group,
> params, diffsels, diffurls, pool),
> other_diffs=other_diffs,
> + truncate_diffs=truncate_diffs,
> )
> renderer.render(data)
>
> @@ -900,10 +907,10 @@
>
> w('\nLog:\n%s\n' % data.log)
>
> - self._render_diffs(data.diffs)
> + self._render_diffs(data.diffs, data.truncate_diffs)
> if data.other_diffs:
> w('\nDiffs of changes in other areas also in this revision:\n')
> - self._render_diffs(data.other_diffs)
> + self._render_diffs(data.other_diffs, data.truncate_diffs)
>
> def _render_list(self, header, data_list):
> if not data_list:
> @@ -934,7 +941,7 @@
> w(' - copied%s from r%d, %s%s\n'
> % (text, d.base_rev, d.base_path, is_dir))
>
> - def _render_diffs(self, diffs):
> + def _render_diffs(self, diffs, truncate_diffs):
> w = self.output.write
>
> for diff in diffs:
> @@ -971,8 +978,29 @@
> w('Binary files. No diff available.\n')
> continue
>
> + nlines = 0
> for line in diff.content:
> - w(line.raw)
> + if truncate_diffs > 0 and nlines >= truncate_diffs:
> + #
> + # Truncate the diff, inform the reader, and ensure that
> + # "patch" will see the truncated diff as an error.
> + #
> + # If somebody accidentally feeds the truncated diff to the
> + # "patch" program, then either the "@@" line or the "***"
> + # line will be seen by "patch" as a fatal error. (Which line
> + # gets reported as an error will depend on exactly where the
> + # truncation occurs, and it doesn't seem possible to construct
> + # a single line that will always be reported as an error
> + # regardless of where it appears.)
> + #
> + w('@@ -0,0 +0,1 @@ Diff truncated after %d lines.\n' \
> + % (truncate_diffs))
> + w('*** Diff truncated after %d lines.\n' \
> + % (truncate_diffs))
> + break
> + else:
> + w(line.raw)
> + nlines = nlines + 1
>
>
> class Repository:
> === mailer.conf.example
> ==================================================================
> --- mailer.conf.example (revision 18413)
> +++ mailer.conf.example (revision 18535)
> @@ -224,6 +224,12 @@
> # Set to 0 to turn off.
> #truncate_subject = 200
>
> +# Diff length limit. The generated diff will be truncated after this many
> +# lines. The limit applies independently to each file for which a diff
> +# is generated.
> +# Set to 0 to turn off.
> +#truncate_diffs = 200
> +
> # --------------------------------------------------------------------------
>
> [maps]
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
>
--
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] mailer.py: truncate_diff_lines feature
Posted by Alan Barrett <ap...@cequrux.com>.
On Wed, 02 Nov 2005, C. Michael Pilato wrote:
> "truncate_diff_lines" sounds like you're chopping characters off the
> ends of long lines, not chopping lines off the end of a long diff.
> Suggest "truncate_diff".
I went with "truncate_diffs". A plural word seemed better to me.
> I'm somewhat ambivalent about this feature addition, but do have one
> concern: I'd prefer it if the act of truncating a diff would cause an
> application of that diff data as a patch to fail in some noticable
> way.
Done. I append a new patch.
[[[
Add truncate_diffs feature to mailer.py hook script. If this
is set to a non-zero value, then diffs longer than the specified
number of lines will be truncated in email messages.
* mailer.py (TextCommitRenderer.generate_content): Obtain
truncate_diffs value from cfg, and pass it through in the data sent
to renderer.render().
(TextCommitRenderer.render): Pass data.truncate_diffs through to
self._render_diffs().
(TextCommitRenderer.render._render_diffs): New truncate_diffs
arg, and new logic to implement the truncation.
* mailer.conf.example: Add commented-out example for truncate_diffs.
]]]
=== mailer.py
==================================================================
--- mailer.py (revision 18413)
+++ mailer.py (revision 18535)
@@ -589,6 +589,12 @@
show_nonmatching_paths = cfg.get('show_nonmatching_paths', group, params) \
or 'yes'
+ try:
+ truncate_diffs = int(
+ cfg.get('truncate_diffs', group, params))
+ except ValueError:
+ truncate_diffs = 0
+
# figure out the lists of changes outside the selected path-space
other_added_data = other_removed_data = other_modified_data = [ ]
if len(paths) != len(changelist) and show_nonmatching_paths != 'no':
@@ -617,6 +623,7 @@
diffs=DiffGenerator(changelist, paths, True, cfg, repos, date, group,
params, diffsels, diffurls, pool),
other_diffs=other_diffs,
+ truncate_diffs=truncate_diffs,
)
renderer.render(data)
@@ -900,10 +907,10 @@
w('\nLog:\n%s\n' % data.log)
- self._render_diffs(data.diffs)
+ self._render_diffs(data.diffs, data.truncate_diffs)
if data.other_diffs:
w('\nDiffs of changes in other areas also in this revision:\n')
- self._render_diffs(data.other_diffs)
+ self._render_diffs(data.other_diffs, data.truncate_diffs)
def _render_list(self, header, data_list):
if not data_list:
@@ -934,7 +941,7 @@
w(' - copied%s from r%d, %s%s\n'
% (text, d.base_rev, d.base_path, is_dir))
- def _render_diffs(self, diffs):
+ def _render_diffs(self, diffs, truncate_diffs):
w = self.output.write
for diff in diffs:
@@ -971,8 +978,29 @@
w('Binary files. No diff available.\n')
continue
+ nlines = 0
for line in diff.content:
- w(line.raw)
+ if truncate_diffs > 0 and nlines >= truncate_diffs:
+ #
+ # Truncate the diff, inform the reader, and ensure that
+ # "patch" will see the truncated diff as an error.
+ #
+ # If somebody accidentally feeds the truncated diff to the
+ # "patch" program, then either the "@@" line or the "***"
+ # line will be seen by "patch" as a fatal error. (Which line
+ # gets reported as an error will depend on exactly where the
+ # truncation occurs, and it doesn't seem possible to construct
+ # a single line that will always be reported as an error
+ # regardless of where it appears.)
+ #
+ w('@@ -0,0 +0,1 @@ Diff truncated after %d lines.\n' \
+ % (truncate_diffs))
+ w('*** Diff truncated after %d lines.\n' \
+ % (truncate_diffs))
+ break
+ else:
+ w(line.raw)
+ nlines = nlines + 1
class Repository:
=== mailer.conf.example
==================================================================
--- mailer.conf.example (revision 18413)
+++ mailer.conf.example (revision 18535)
@@ -224,6 +224,12 @@
# Set to 0 to turn off.
#truncate_subject = 200
+# Diff length limit. The generated diff will be truncated after this many
+# lines. The limit applies independently to each file for which a diff
+# is generated.
+# Set to 0 to turn off.
+#truncate_diffs = 200
+
# --------------------------------------------------------------------------
[maps]
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] mailer.py: truncate_diff_lines feature
Posted by "C. Michael Pilato" <cm...@collab.net>.
Alan Barrett <ap...@cequrux.com> writes:
> [[[
> Add truncate_diff_lines feature to mailer.py hook script. If this
> is set to a non-zero value, then diffs longer than the specified
> number of lines will be truncated in email messages.
"truncate_diff_lines" sounds like you're chopping characters off the
ends of long lines, not chopping lines off the end of a long diff.
Suggest "truncate_diff".
I'm somewhat ambivalent about this feature addition, but do have one
concern: I'd prefer it if the act of truncating a diff would cause an
application of that diff data as a patch to fail in some noticable
way. In a sea of 100 diffs in a single email, it'd be really easy to
miss a single instance of a truncated one -- what happens to the poor
guy that tries to apply that email as a patch and succeeds because the
truncation happened to line up with a hunk boundary?
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org