C. Michael Pilato wrote:
> wein@tigris.org wrote:
> > Author: wein
> > Date: Wed Sep 27 12:56:57 2006
> > New Revision: 21684
> >
> > Log:
> > Add property handling to mailer.py's commit messages.
> >
> > It is now possible to also report changes of properties (including
> > diffs) with a commit message. Therefor several new classes were
> ^^^ "Therefore"
Thanks. Should I change the log message (and am I allowed to)?
> > added to mailer.py. To configure the reporting of property changes
> > four new parameters are added to mailer.conf. Without adding these
> > parameters to an existing configuration file no property changes
> > will be reported.
> >
> > New configuration parameters:
> > show_props configure which property changes to report
> > (valid options are any combination of
> > 'add_path add copy_path modify delete_path
> > delete')
> > ignore_props configure which property names shall be ignored
> > in the list of changed properties
> > generate_propdiffs configure for which property changes diffs
> > shall be created
> > (valid options are any combination of
> > 'add_path add copy_path modify delete_path
> > delete')
> > ignore_propdiffs configure which property names shall be ignored
> > when creating diffs for changed properties
> >
> > * tools/hook-scripts/mailer/mailer.py
> > New classes:
> > PropSelections
> > PropDiffSelections
> > PropDiffGenerator.
> > New functions:
> > TextCommitRenderer._get_prop_list
> > TextCommitRenderer._render_props
> > TextCommitRenderer.__render_props
> > TextCommitRenderer._render_propdiffs
> > TextCommitRenderer.__render_propdiffs.
> > Several appropriate changes to other classes and functions.
> >
> > * tools/hook-scripts/mailer/mailer.conf.example
> > New parameters show_props, ignore_props, generate_propdiffs and
> > ignore_propdiffs.
> >
> > * tools/hook-scripts/mailer/tests/mailer.conf
> > New parameters show_props, ignore_props, generate_propdiffs and
> > ignore_propdiffs.
> >
> > * tools/hook-scripts/mailer/tests/mailer-t1.output
> > Now containing property changes.
> >
> > Modified:
> > trunk/tools/hook-scripts/mailer/mailer.conf.example
> > trunk/tools/hook-scripts/mailer/mailer.py
> > trunk/tools/hook-scripts/mailer/tests/mailer-t1.output
> > trunk/tools/hook-scripts/mailer/tests/mailer.conf
> >
> > Modified: trunk/tools/hook-scripts/mailer/mailer.conf.example
> > URL: http://svn.collab.net/viewvc/svn/trunk/tools/hook-scripts/mailer/mailer.conf.example?pathrev=21684&r1=21683&r2=21684
> > ==============================================================================
> > --- trunk/tools/hook-scripts/mailer/mailer.conf.example (original)
> > +++ trunk/tools/hook-scripts/mailer/mailer.conf.example Wed Sep 27 12:56:57 2006
> > @@ -193,6 +193,52 @@
> > # delete: generates diffs for all removed paths
> > generate_diffs = add copy modify
> >
> > +# Specify which types of property changes mailer.py will report as a
> > +# summary at the top of the message, after the summary of changed paths.
> > +# Valid options are any combination of 'add_path add copy_path modify
> > +# delete_path delete'.
> > +# If the show_props option is empty, no list of changed properties will
> > +# be created.
> > +# Meaning of the possible values:
> > +# add_path: lists all properties added with a newly added paths
> > +# add: lists all added properties not including the ones added
> > +# for newly added paths
> > +# copy_path: lists all properties which were implicitly added by
> > +# making a copy of a path and not changed or removed after
> > +# copying
> > +# modify: lists all modified properties, including the ones that
> > +# were added by making a copy of a path and modified
> > +# afterwards (within the same commit)
> > +# delete_path: lists all properties removed with a path
> > +# delete: lists all removed properties not including the ones for
> > +# removed paths
> > +show_props = add modify delete
>
> That's, uh, purty complex stuff you've got there. You sure all that is
> necessary?
I think yes (see also Alan's post). If you find this too complex we
can change the default to show every change so only people who want to
can explicitly remove the regarding option value from the list.
> > +
> > +# When set to a regular expression all properties matching this regular
> > +# expression will not be included in the summary of changed properties.
> > +# Example (ignore all internal properties, i. e. all properties
> > +# starting with 'svn:'):
> > +# ignore_props = ^svn:
> > +ignore_props = ^svn:
>
> I'm not sure if the example file should actually ignore the svn:
> properties -- ideally, a person should get the typically desired
> behavior with the minimal number of edits to a copy of those sample
> file, and I suspect most folks want to see that svn: properties were
> changed.
Oops, that was a mistake. In fact ignore_props shouldn't be set in the
configuration example. Thanks. I will correct that.
>
> > +
> > +# Specify which types of property changes mailer.py will create
> > +# diffs for. Valid options are any combination of
> > +# 'add_path add copy_path modify delete_path delete'. If the
> > +# generate_propdiffs option is empty, no diffs are created.
> > +# Note that this only affects the display of property diffs - the mentioning
> > +# of property changes in the summary at the top of the message is regulated
> > +# by show_props and listed regardless of this option's value.
> > +# See show_props for an explanation of the possible values.
> > +generate_propdiffs = add modify delete
> > +
> > +# When set to a regular expression all properties matching this regular
> > +# expression will be ignored by the selection made by
> > +# generate_propdiffs.
> > +# Example (ignore all internal properties, i. e. all properties
> > +# starting with 'svn:'):
> > +# ignore_propdiffs = ^svn:
> > +ignore_propdiffs = ^svn:
>
> See comment above regarding "ignore_props".
See comment above regarding "ignore_props".
>
> > # Commit URL construction. This adds a URL to the top of the message
> > # that can lead the reader to a Trac, ViewVC or other view of the
> > # commit as a whole.
> > @@ -232,8 +278,9 @@
> > # match, this variable controls the behaviour for the non-matching
> > # paths. Possible values are:
> > #
> > -# yes: (Default) Show in both summary and diffs.
> > -# summary: Show the changed paths in the summary, but omit the diffs.
> > +# yes: (Default) Show in both summary and diffs (including properties).
> > +# summary: Show the changed paths in the summary, but omit the diffs
> > +# (and the property diffs).
> > # no: Show nothing more than a note saying "and changes in other areas"
> > #
> > show_nonmatching_paths = yes
>
> [...]
>
> > Modified: trunk/tools/hook-scripts/mailer/tests/mailer-t1.output
> > URL: http://svn.collab.net/viewvc/svn/trunk/tools/hook-scripts/mailer/tests/mailer-t1.output?pathrev=21684&r1=21683&r2=21684
> > ==============================================================================
> > --- trunk/tools/hook-scripts/mailer/tests/mailer-t1.output (original)
> > +++ trunk/tools/hook-scripts/mailer/tests/mailer-t1.output Wed Sep 27 12:56:57 2006
>
> [...]
>
> > @@ -204,6 +238,19 @@
> > file1 (props changed)
> > file2 (contents, props changed)
> >
> > +Property Changes:
> > + dir1/
> > + Added:
> > + prop3
> > + file1
> > + Added:
> > + prop1
> > + file2
> > + Added:
> > + prop1
> > + svn:keywords
> > + svn:new_svn_prop
>
> Wow. I *really* don't see the utility of this whole "Property Changes"
> section. Besides being positively atrocious in term of the physical
> layout (level four nesting?!), I just really fail to see the utility of
> having both this and the property diff section.
>
> I would strongly favor ditching this section altogether (and the
> configury associated with it).
An alternative would be to not set the show_props option in the
configuration example.
>
> > +Added property for: dir1/ (from r1, dir1)
> > +==============================================================================
> > +--- /dev/null 00:00:00 1970 (empty, because property is newly added)
> > ++++ dir1|prop3 Sun Sep 9 04:33:20 2001 (r2)
> > +@@ -0,0 +1 @@
> > ++propval3
> > +
>
> Yikes?! Does that actually say "/dev/null"? And contain a malformatted
> date string? And bother to explain "empty, because property is newly
> added"?
I will correct the date string to 'Thu Jan 1 00:00:00 1970' (and I will
also do this for the files content diffs).
> Also, what kind of problems can we expect to be caused by the fact that
> this diff output is something that appears (as far as I can tell) to be
> something 'patch' would actually try to apply? That's bad news.
I actually chose the form used for the file diffs to have a consistent
look for all diffs. Please feel free to present (and commit) any other
layout.
> ~ ~ ~ ~ ~
>
> In summary, I'm all for mailer.py showing diffs of properties, but those
> diffs *must not* appear as something 'patch' will try to understand and
> apply. Further, I'm very much opposed to the propchange summary
> section, which I believe to be visually unwieldy and largely redundant
> with true property diffs. Finally, I think the sample file should
> reflect recommended default settings, even if more interesting things
> are explained yet commented-out.
In summary, I will change the date format for /dev/null and change the
default values in the configuration file like this:
show_props =
ignore_props =
generate_propdiffs = add_path add copy_path modify delete_path delete
ignore_propdiffs =
This will remove the 'Property changes' section form the output and
provide diffs for all property changes, independet from their name or
action.
Topics for further discussion:
- What format shall we use for the property diffs?
- Shall we combine the 'Property Changes' section and the propery diffs
into one section?
Thanks for your comments!
Mathias
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Sep 28 14:30:56 2006