Alexey Neyman wrote:
> [Cross-posting to dev@ for patch review]
> I noticed a weird behavior of 'svn diff --old=... --new=...' where
> reversing the old/new targets does not produce the reverse diff, as
> one would expect.
Hi Alexey. Thanks for the report. As you pointed out, I changed something in this area recently, so I'll take a closer look and fix it. I haven't done so yet but should get to it in the next day or two.
[...]
> The reason for this behavior is that the code in diff-cmd.c makes the
> following defaults for the old/new revisions:
>
> if (opt_state->start_revision.kind == svn_opt_revision_unspecified)
> opt_state->start_revision.kind = svn_path_is_url(old_target)
> ? svn_opt_revision_head : svn_opt_revision_base;
>
> if (opt_state->end_revision.kind == svn_opt_revision_unspecified)
> opt_state->end_revision.kind = svn_path_is_url(new_target)
> ? svn_opt_revision_head : svn_opt_revision_working;
>
> These defaults make some sense, if the new target is not specified (it
> defaults to old target in that case). If the new target is specified,
> and is different from from old target, I am not so sure if it makes
> sense. Note that there is a special case if both old/new targets are
> WC paths - in that case, both old and new targets default to
> svn_opt_revision_working. So,
>
> $ svn diff --old=foo --new=bar
>
> will compare MODIFIED version of 'foo' against modified version of
> 'bar' [*], but
>
> $ svn diff --old=foo --new=^/bar
>
> would compare BASE version of 'foo' against head version of 'bar.
> Does it make sense? I would say, if both old and new are specified,
> old target's revision should default to 'working' as well.
>
> Problem is further aggravated since there is no commandline alias
> to request svn_opt_revision_working setting from the command line:
> only 'head', 'prev', 'base' and 'committed' are currently available.
> Hence, it is not possible to request the exact opposite of the patch
> using -rN:M syntax if one of the --old/--new targets is a working
> copy path. Is there a reason why 'working' is not parsed by
> revision_from_word()?
>
> Also note that 'svn help diff' does not describe revision defaults
> for synopsis 2 (and the default is different from synopsis 1, which
> requires old revision to be specified for URLs).
>
> PS. Stephan apparently made 'working' revision as default in his
> check-in (r1442640) at least for the short-hand form, but Julian
> Foad, "optimizing the logic" in r1442659 and r1442676, switched it
> back to be the same as --old/--new logic.
Oops, I didn't notice I was changing the logic. As my log message said, I thought I was just simplifying it.
> PPS. If agreed to suggested change of default, the patch is attached.
>
> [[[
> Make svn diff --old=.. --new=.. default to WORKING revision for old target
> if new target is explicitly specified.
A log message should always indicate *why* the change was made.
>
> * subversion/svn/diff-cmd.c
> (svn_cl__diff) Change defaults as described and simplify the logic.
> ]]]
- Julian
Received on 2013-02-07 17:01:25 CET