Branko ?ibej wrote:
>
> blair@tigris.org wrote:
>
> >Author: blair
> >Date: Tue, 02 Jul 2002 01:14:53 -0500
> >New Revision: 2390
> >
> >Modified:
> > trunk/tools/hook-scripts/commit-email.pl
> >Log:
> >* tools/hook-scripts/commit-email.pl:
> >
> [snip]
>
> >+# Use the reference to the first project to populate.
> >+my $current_project = $project_settings_list[0];
> >+
> >+while (@ARGV) {
> >+ my $arg = shift @ARGV;
> >+ if (my ($opt) = $arg =~ /^-([hlmrs])/) {
> >+ unless (@ARGV) {
> >+ die "$0: command line option `$arg' is missing a value.\n";
> >+ }
> >+ my $value = shift @ARGV;
> >+ SWITCH: {
> >+ $current_project->{hostname} = $value, last SWITCH if $opt eq 'h';
> >+ $current_project->{log_file} = $value, last SWITCH if $opt eq 'l';
> >+ $current_project->{reply_to} = $value, last SWITCH if $opt eq 'r';
> >+ $current_project->{subject_prefix} = $value, last SWITCH if $opt eq 's';
> >+
> >+ # Here handle -match.
> >+ unless ($opt eq 'm') {
> >+ die "$0: internal error: should only handle -m here.\n";
> >+ }
> >+ $current_project = dclone($blank_settings);
> >+ $current_project->{match_regex} = $value;
> >+ push(@project_settings_list, $current_project);
> >
>
> I think there's a bug here. If $current_project->{email_addresses} is
> empty, this'll push a useless entry on the @project_settings_list. And
> this will always happen if you start with an -m option, because you
> initialized the list with emtpy email_addresses.
It won't be useless. The user can specify the -l to write a log
file and not send any emails out. The code later on checks if
a project has any email addresses and/or a log file and does the
correct thing.
Right. The code won't care.
Even if the first command line option is a -m which will create a new
element in @project_settings_list, later on, the code checks if there
are any email addresses or if a log file has been associated with a
particular project.
>
> I think the logic should be different: create a new list entry when
> _any_ option is seen, if the current entries email_addresses list is not
> empty:
>
> if (my ($opt) = $arg =~ /^-([hlmrs])/) {
> unless (@ARGV) {
> die "$0: command line option `$arg' is missing a value.\n";
> }
> if (scalar $current_project->{email_addresses}) {
> $current_project = dclone($blank_settings);
> push(@project_settings_list, $current_project);
> }
> my $value = shift @ARGV;
> SWITCH: {
> $current_project->{hostname} = $value, last SWITCH if $opt eq 'h';
> $current_project->{log_file} = $value, last SWITCH if $opt eq 'l';
> $current_project->{reply_to} = $value, last SWITCH if $opt eq 'r';
> $current_project->{subject_prefix} = $value, last SWITCH if $opt eq 's';
> $current_project->{match_regex} = $value, last SWITCH if $opt eq 'm';
>
> die "$0: internal error: don't know how to handle -$opt here.\n";
> }
The previous versions and this version of the script supported having
no email addresses, so using them as a separator between projects won't
work. This command line wouldn't work with this code flow:
% commit-email.pl -m ^proj1 -l /var/log/svn_prog1
-m ^proj2 -l /var/log/svn_proj2
It'll overwrite proj1's match_regex and logfile with proj2's.
Best,
Blair
--
Blair Zajac <blair@orcaware.com>
Web and OS performance plots - http://www.orcaware.com/orca/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Jul 2 17:55:29 2002