"Mathias Weinert" <mathias.weinert@gfa-net.de> wrote:
> Hi there,
>
> I was just starting to test mailer.py and want to provide some
> patches for it (and some questions :-) ). I am using it under
> a cygwin environment (with WinXP SP2), using subversion
> 1.3.0-rc4.
Until now I only got a response regarding number 1 which led
to a longer discussion about where to check the path name.
So I want to ask again if there is anyone willing to commit
the patches, tell me why not or tell me what to do better.
I already have some more patches (error correction, smaller
changes plus two new options regarding changed properties)
for this script but I don't think that it makes sense to post
them now. It's surely better to first apply or recject these
ones before starting any confusion about the patches, their
line numbers etc.
My first mail (see below) contains the following patches:
- "No error for repos path with trailing slash" (hunk 6)
- Renaming of PROPCHANGE in the command line help (hunk 1, 5)
- typo error in the handling of diffs of changes in other
areas (hunk 2)
- Unnecessaary printing of 'Diffs of changes in other areas
also in this revision:' (hunk 3, 4)
plus the two questions about 'propchange2 with wrong action'
and default handling (or not-handling).
Thanks for any answer
Mathias
>
> 1. The script is not very robust against wrong command line
> arguments. You get for example an assertion if you call it with
> a repos path with a trailing slash (like bash completion
> provides it):
> assertion "is_canonical (base, blen)" failed: file "subversion/libsvn_subr/path.c", line 114
> Aborted (core dumped)
>
> But after thinking a while about this I think this might be
> no problem as the script will mostly be used by subversion hooks
> which will always provide the right parameters. On the other hand
> everybody testing the script before implementing it in a hook
> will possibly run into such problems.
> In order to prevent this assertion I changed one line in the
> script: See hunk 6 in patch at bottom of this mail
>
>
> 2. It took a while for me to accept that propchange and
> propchange2 are meant to handle revision properties ;-). So why
> not calling these commands revpropchange and revpropchange2? As
> it's probably too late to change the command names now I would
> at least apply the following patch for the command line help:
> See hunks 1 and 5 in patch at bottom of this mail
>
>
> 3. I always run into an error when calling the command 'commit'
> when there are changes in other areas and show_nonmatching_paths
> is set to yes. I always get:
>
> [...]
> Diffs of changes in other areas also in this revision:
> Traceback (most recent call last):
> File "/d/Subversion/subversion-1.3.0-rc4/tools/hook-scripts/mailer/mailer.py", line 1280, in ?
> sys.argv[3:3+expected_args])
> File "/usr/local/lib/svn-python/svn/core.py", line 217, in run_app
> return apply(func, (_core.application_pool,) + args, kw)
> File "/d/Subversion/subversion-1.3.0-rc4/tools/hook-scripts/mailer/mailer.py", line 75, in main
> messenger.generate()
> File "/d/Subversion/subversion-1.3.0-rc4/tools/hook-scripts/mailer/mailer.py", line 364, in generate
> group, params, paths, subpool)
> File "/d/Subversion/subversion-1.3.0-rc4/tools/hook-scripts/mailer/mailer.py", line 621, in generate_content
> renderer.render(data)
> File "/d/Subversion/subversion-1.3.0-rc4/tools/hook-scripts/mailer/mailer.py", line 906, in render
> self._render_diffs(data.other_diffs)
> File "/d/Subversion/subversion-1.3.0-rc4/tools/hook-scripts/mailer/mailer.py", line 941, in _render_diffs
> if not diff.diff and not diff.diff_url:
> AttributeError: DiffGenerator instance has no attribute 'diff'
>
> The cause of this is a typo bug at the generation of other
> diffs: See hunk 2 in patch at bottom of this mail (looks like
> a copy-and-paste-error)
>
>
> 4. Just a small thing when calling the command 'commit' when
> there are changes in other areas and show_nonmatching_paths is
> set to yes. In this case the line 'Diffs of changes in other
> areas also in this revision:' is always printed even if there
> are no changes that match the generate_diffs option. To avoid
> this I moved the writing of this line into _render_diffs (I am
> not a Python programer and I am sure there must be a more
> elegant solution to this): See hunks 3 and 4 in patch at bottom
> of this mail
>
>
> 5. If you call mailer.py propchange2 with the wrong action it
> is waiting for user input. I don't know what this is good for
> but am interested in it. Could you please explain me this?
>
>
> 6. A question of understanding the configuration file: Let's
> say I am only interested in getting mails about changed .html
> files. So I create a group with 'for_paths = .*\.html'. When
> doing this all .html files are processed with this group and
> all other paths are processed with default. Is there a way
> to explicitly say "do nothing with all other paths" -
> especially if I have some other groups (.c files, .java
> files, ...)? Or do I have to ensure that the default section
> doesn't contain a to_addr for example?
>
>
> Thanks in advance - and keep on developing this wonderful
> piece of software (subversion as a total, for sure ;-) )!
>
> Mathias
original patch and commit message again:
[[[
* tools/hook-scripts/mailer/mailer.py
- now also accepting repos name with trailing slash
- change of command line help parameter names: REVPROPNAME instead of PROPNAME
- typo bug removed from generate_content()
- TextCommitRenderer.render() now only showing text regarding diffs in other areas if these diffs are shown theirselves
]]]
--- tools/hook-scripts/mailer/mailer.py.orig 2005-06-24 19:50:11.000000000 +0200
+++ tools/hook-scripts/mailer/mailer.py 2005-12-06 17:28:28.977720100 +0100
@@ -8,8 +8,8 @@
# $LastChangedRevision: 15162 $
#
# USAGE: mailer.py commit REPOS REVISION [CONFIG-FILE]
-# mailer.py propchange REPOS REVISION AUTHOR PROPNAME [CONFIG-FILE]
-# mailer.py propchange2 REPOS REVISION AUTHOR PROPNAME ACTION \
+# mailer.py propchange REPOS REVISION AUTHOR REVPROPNAME [CONFIG-FILE]
+# mailer.py propchange2 REPOS REVISION AUTHOR REVPROPNAME ACTION \
# [CONFIG-FILE]
# mailer.py lock REPOS AUTHOR [CONFIG-FILE]
# mailer.py unlock REPOS AUTHOR [CONFIG-FILE]
@@ -598,7 +598,7 @@
if len(paths) != len(changelist) and show_nonmatching_paths == 'yes':
other_diffs = DiffGenerator(changelist, paths, False, cfg, repos, date,
- group, params, diffsels, diffurls, pool),
+ group, params, diffsels, diffurls, pool)
else:
other_diffs = [ ]
@@ -900,10 +900,11 @@
w('\nLog:\n%s\n' % data.log)
- self._render_diffs(data.diffs)
+ self._render_diffs(data.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,
+ '\nDiffs of changes in other areas also '
+ 'in this revision:\n')
def _render_list(self, header, data_list):
if not data_list:
@@ -934,12 +935,16 @@
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, first_diff_text):
w = self.output.write
+ is_first_diff = True
for diff in diffs:
if not diff.diff and not diff.diff_url:
continue
+ if is_first_diff:
+ w('%s' % first_diff_text)
+ is_first_diff = False
if diff.kind == 'D':
w('\nDeleted: %s\n' % diff.base_path)
if diff.diff_url:
@@ -1222,8 +1227,8 @@
scriptname = os.path.basename(sys.argv[0])
sys.stderr.write(
"""USAGE: %s commit REPOS REVISION [CONFIG-FILE]
- %s propchange REPOS REVISION AUTHOR PROPNAME [CONFIG-FILE]
- %s propchange2 REPOS REVISION AUTHOR PROPNAME ACTION [CONFIG-FILE]
+ %s propchange REPOS REVISION AUTHOR REVPROPNAME [CONFIG-FILE]
+ %s propchange2 REPOS REVISION AUTHOR REVPROPNAME ACTION [CONFIG-FILE]
%s lock REPOS AUTHOR [CONFIG-FILE]
%s unlock REPOS AUTHOR [CONFIG-FILE]
@@ -1253,7 +1258,7 @@
usage()
cmd = sys.argv[1]
- repos_dir = sys.argv[2]
+ repos_dir = os.path.normpath(sys.argv[2])
try:
expected_args = cmd_list[cmd]
except KeyError:
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Dec 12 15:43:40 2005