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
