[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1547995 - in /subversion/trunk/subversion/svn: cl-log.h log-cmd.c mergeinfo-cmd.c

From: Ben Reser <breser_at_apache.org>
Date: Thu, 05 Dec 2013 14:09:25 -0800

On 12/5/13 12:30 PM, Ivan Zhakov wrote:
> On 5 December 2013 06:20, <breser_at_apache.org> wrote:
>> Author: breser
>> Date: Thu Dec 5 02:20:41 2013
>> New Revision: 1547995
>>
>> URL: http://svn.apache.org/r1547995
>> Log:
>> Make mergeinfo and log commands share the same log receiver implementation.
>>
> The only problem with this change that 'svn log' has special behavior
> for printing merged revisions and this behavior probably doesn't make
> sense for svn mergeinfo --log. That's why I didn't share
> implementation half a year ago.

How is that a problem? It only activates if a log entry has the has_children
field set to true, which can never happen with mergeinfo --log since
svn_client_mergeinfo_log2() doesn't even have a include_merged_revisions
option. The only complication it really presents is we have to allocate a
apr_array_header_t for merge_stack since the code in the log receiver doesn't
check it. Which actually would be really trivial to fix, I just didn't do it.

My intention for doing this was adding the various options log supports to
mergeinfo --log. I thought about trying to move the support to log and
specifying --show-revs on log. The problem with that is:

1) log takes multiple targets. But mergeinfo needs the source and destination
paths to work. It felt clunky trying to add an argument for the target branch
to log.

2) --show-revs itself as a parameter is poorly named to allow reuse on other
commands.

Pulling in the options that log supports into mergeinfo also feels clunky, but
from a UI perspective it works. The duplicated code to support the options is
really trivial when both commands share the same log receivers.

Adding support for all the features that both will support onto the log
receiver for mergeinfo becomes more and more problematic. As I'm sure you can
guess I'm planning to add --xml as well.
Received on 2013-12-05 23:10:01 CET

This is an archived mail posted to the Subversion Dev mailing list.