You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Julian Foad <ju...@btopenworld.com> on 2006/03/14 00:30:08 UTC
Diff --old --new: too many peg revision specifiers
We've got a nasty relic of a meaning of "-r" in "diff --old --new" syntax.
> $ svn diff --help
> diff (di): Display the differences between two revisions or paths.
> usage: 1. diff [-c M | -r N[:M]] [TARGET[@REV]...]
> 2. diff [-c M | -r N[:M]] --old=OLD-TGT[@OLDREV] [--new=NEW-TGT[@NEWREV]] \
> [PATH...]
> 3. diff OLD-URL[@OLDREV] NEW-URL[@NEWREV]
> [...]
> 2. [...]
> -r N makes OLDREV default to N,
> -r N:M makes OLDREV default to N and NEWREV default to M
In syntax (1), "@REV" is a peg and "-r N" is an operative revision. Fine.
In syntax (2), "@OLDREV" and "@NEWREV" are pegs, and "-r N[:M]" provides
default values for the pegs. I think that's horrible. It's inconsistent with
every other command. It's redundant: if both "-r" and "@" revisions are
specified, the code silently ignores the "-r" value.
Furthermore, the "-c" option is even sillier, because it is intended to specify
"the change" that occured in an object or in the repository, whereas in this
syntax it is being used to provide the two revision numbers in which to locate
two separate objects.
I would like to delete the "-c" and "-r" options from syntax 2. It's an
incompatible UI change, but I'd call it a bug fix. The help would be:
2. diff --old=OLD-TGT[@OLDREV] [--new=NEW-TGT[@NEWREV]] \
[PATH...]
This would be more in line with the corresponding forms of merge command:
usage: 1. merge sourceURL1[@N] sourceURL2[@M] [WCPATH]
2. merge sourceWCPATH1@N sourceWCPATH2@M [WCPATH]
3. merge [-c M | -r N:M] SOURCE[@REV] [WCPATH]
However, we have a duty to maintain compatibility with the older UI. In v1.0,
which was before it supported peg revs, the syntax was:
diff [-r N[:M]] [--old OLD-TGT] [--new NEW-TGT] [PATH...]
"-r" provided what we now call the peg revs.
The peg-rev notation was added in r9707, round about v1.1, changing the help to:
2. diff [-r N[:M]] --old=OLD-TGT[@OLDREV] [--new=NEW-TGT[@NEWREV]] \
[PATH...]
That is when "-r" was made to provide "defaults" for the peg revs, in order to
maintain compatibility with the v1.0 UI syntax.
At the very least I will get rid of "-c" in syntax 2, because it is just adding
to the confusion and there is still time to stop it before it is released in v1.4.
I would also like to:
* error or warn if both "-r" and an "@" are specified, because that was
never intended
* deprecate "-r" by changing the help to say so
but those would both be incompatible UI changes. Would either of them be
desirable on balance?
In short, I'm trying to find a migration path from having this horrible meaning
of "-r" to eventually either not having "-r" or having it specify operative
revisions as it does in other commands and particularly in other forms of "svn
diff".
Does anyone see any better steps along that road?
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Diff --old --new: too many peg revision specifiers
Posted by Julian Foad <ju...@btopenworld.com>.
Julian Foad wrote:
> kfogel@collab.net wrote:
>> Julian Foad <ju...@btopenworld.com> writes:
>>
>>> Patch attached. OK so far?
>>
>> Looks like a sensible step to me, & the patch looks good (I haven't
>> applied it, but reviewed it in-place).
r18908.
>>> Next step: I think we shouldn't drop support for "-r" before v2, but I
>>> think it is OK for us to deprecate an aspect of the User Interface in
>>> a minor release just as we deprecate APIs. I propose to remove the
>>> mention of "-r" from the main description, and add a note stating it
>>> is accepted for backward compatibility, and describing its meaning.
>>
>> +1
This isn't as straightforward as it seemed, for two reasons:
1. This "old & new" form of "diff" is designed for both comparing two separate
paths and, when "--new" is omitted, for comparing two revisions of "a single
path" (whatever that means) and the latter form requires "-r X:Y".
2. Our semantics for what we broadly call "peg revisions" are in a bit of a
mess in some regards (where we allow "@REV" on a WC-path, in particular).
I need to do some more work on this.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Diff --old --new: too many peg revision specifiers
Posted by Julian Foad <ju...@btopenworld.com>.
kfogel@collab.net wrote:
> Julian Foad <ju...@btopenworld.com> writes:
>
>>Patch attached. OK so far?
>
> Looks like a sensible step to me, & the patch looks good (I haven't
> applied it, but reviewed it in-place).
>
> Note that no backporting to 1.4.x is required, since it's just trunk.
Oh, good - silly me, I'd seen Dave Anderson's announcement that he was
branching 1.3.1 a few days ago and had interpreted it as "1.4" in my head.
>>Next step: I think we shouldn't drop support for "-r" before v2, but I
>>think it is OK for us to deprecate an aspect of the User Interface in
>>a minor release just as we deprecate APIs. I propose to remove the
>>mention of "-r" from the main description, and add a note stating it
>>is accepted for backward compatibility, and describing its meaning.
>
> +1
I'll prepare a patch.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Diff --old --new: too many peg revision specifiers
Posted by kf...@collab.net.
Julian Foad <ju...@btopenworld.com> writes:
> Excellent.
>
> The essential first patch to avoid proliferating this problem is to
> remove "-c" from the help message for this syntax, and preferably
> disallow it at the option-parsing stage (which is a bit clumsy, since
> extra logic is required in order to distinguish it from "-r"). The
> "-c" hasn't been released yet, so we just need to make sure this gets
> back-ported into 1.4.0 and there'll be no compatibility worries in
> that regard.
>
> Patch attached. OK so far?
Looks like a sensible step to me, & the patch looks good (I haven't
applied it, but reviewed it in-place).
Note that no backporting to 1.4.x is required, since it's just trunk.
> Next step: I think we shouldn't drop support for "-r" before v2, but I
> think it is OK for us to deprecate an aspect of the User Interface in
> a minor release just as we deprecate APIs. I propose to remove the
> mention of "-r" from the main description, and add a note stating it
> is accepted for backward compatibility, and describing its meaning.
+1
-Karl
--
www.collab.net <> CollabNet | Distributed Development On Demand
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Diff --old --new: too many peg revision specifiers
Posted by Julian Foad <ju...@btopenworld.com>.
Greg Hudson wrote:
> On Tue, 2006-03-14 at 18:19 +0000, Julian Foad wrote:
>
>>If we support "-r N[:M] --old...", as with other commands it should mean
>>"operate on revisions N and M, or the revision range from N to M, of the
>>object", but this is neither what it currently means nor a good fit for the
>>syntax which specified TWO separate objects (--old and --new), so that's why I
>>say we should (eventually) drop the "-r" option from this syntax altogether.
>
> I was surprised to learn that svn diff --old= --new= allowed the -r
> option, and although it was probably my fault, I agree that we should
> get rid of it whenever it's practical to do so.
Excellent.
The essential first patch to avoid proliferating this problem is to remove "-c"
from the help message for this syntax, and preferably disallow it at the
option-parsing stage (which is a bit clumsy, since extra logic is required in
order to distinguish it from "-r"). The "-c" hasn't been released yet, so we
just need to make sure this gets back-ported into 1.4.0 and there'll be no
compatibility worries in that regard.
Patch attached. OK so far?
Next step: I think we shouldn't drop support for "-r" before v2, but I think it
is OK for us to deprecate an aspect of the User Interface in a minor release
just as we deprecate APIs. I propose to remove the mention of "-r" from the
main description, and add a note stating it is accepted for backward
compatibility, and describing its meaning.
- Julian
Re: Diff --old --new: too many peg revision specifiers
Posted by Greg Hudson <gh...@MIT.EDU>.
On Tue, 2006-03-14 at 18:19 +0000, Julian Foad wrote:
> If we support "-r N[:M] --old...", as with other commands it should mean
> "operate on revisions N and M, or the revision range from N to M, of the
> object", but this is neither what it currently means nor a good fit for the
> syntax which specified TWO separate objects (--old and --new), so that's why I
> say we should (eventually) drop the "-r" option from this syntax altogether.
I was surprised to learn that svn diff --old= --new= allowed the -r
option, and although it was probably my fault, I agree that we should
get rid of it whenever it's practical to do so.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Diff --old --new: too many peg revision specifiers
Posted by Julian Foad <ju...@btopenworld.com>.
Justin Erenkrantz wrote:
> On 3/13/06, Julian Foad <ju...@btopenworld.com> wrote:
>
>>We've got a nasty relic of a meaning of "-r" in "diff --old --new" syntax.
>
>
> I think independently I ran into what you mean. The -r syntax doesn't
> do anything close to what it should do.
>
>
>>The peg-rev notation was added in r9707, round about v1.1, changing the help to:
>>
>> 2. diff [-r N[:M]] --old=OLD-TGT[@OLDREV] [--new=NEW-TGT[@NEWREV]] \
>> [PATH...]
>>
>>That is when "-r" was made to provide "defaults" for the peg revs, in order to
>>maintain compatibility with the v1.0 UI syntax.
>
> Per my other email, that's not true. Setting '-r' doesn't do anything
> for peg revs. (Perhaps it should?)
By "the peg revs" I meant the revisions specified as "@OLDREV" and "@NEWREV".
(Is that what you understood?) The "-r" does indeed provide default values for
OLDREV and NEWREV in my tests. In this test, "f" was renamed to "g" in r1353,
and HEAD is r1358:
$ svn diff --old=$HERE_URL/f --new=$HERE_URL/f
svn: '[...]/f' was not found in the repository at revision 1358
$ svn diff -r1350:1352 --old=$HERE_URL/f --new=$HERE_URL/f
Index: f
===================================================================
[...]
>> * error or warn if both "-r" and an "@" are specified, because that was
>>never intended
>
> That's currently mandatory if a path has been deleted.
With the diff invocation syntax type (1), both "-r" and "@" are required for
comparing two revisions of a deleted item.
It appears that you are confusing that with this "--old/--new" diff invocation
syntax (2) in which "-r" does not specify operative revisions.
>> * deprecate "-r" by changing the help to say so
>>
>>but those would both be incompatible UI changes. Would either of them be
>>desirable on balance?
>
> I think -r is the logical usage - not URL@REV. It fits with up, log,
> blame, etc.
Logical usage for doing what? It's not simply a question of having one or the
other, it's a question of what each one means.
If we support "--old=URL@REV", as with other commands it should mean that REV
is the "peg" revision, i.e. a revision in which the specified URL exists and
the revision in which it should first be found. It already is supported and
does mean that, so there's no problem.
If we support "-r N[:M] --old...", as with other commands it should mean
"operate on revisions N and M, or the revision range from N to M, of the
object", but this is neither what it currently means nor a good fit for the
syntax which specified TWO separate objects (--old and --new), so that's why I
say we should (eventually) drop the "-r" option from this syntax altogether.
>>In short, I'm trying to find a migration path from having this horrible meaning
>>of "-r" to eventually either not having "-r" or having it specify operative
>>revisions as it does in other commands and particularly in other forms of "svn
>>diff".
>>
>>Does anyone see any better steps along that road?
>
> I'd go back to our 1.0 UI and drop peg revs entirely. *duck* -- justin
Heh.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Diff --old --new: too many peg revision specifiers
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On 3/13/06, Julian Foad <ju...@btopenworld.com> wrote:
> We've got a nasty relic of a meaning of "-r" in "diff --old --new" syntax.
I think independently I ran into what you mean. The -r syntax doesn't
do anything close to what it should do.
> The peg-rev notation was added in r9707, round about v1.1, changing the help to:
>
> 2. diff [-r N[:M]] --old=OLD-TGT[@OLDREV] [--new=NEW-TGT[@NEWREV]] \
> [PATH...]
>
> That is when "-r" was made to provide "defaults" for the peg revs, in order to
> maintain compatibility with the v1.0 UI syntax.
Per my other email, that's not true. Setting '-r' doesn't do anything
for peg revs. (Perhaps it should?)
> * error or warn if both "-r" and an "@" are specified, because that was
> never intended
That's currently mandatory if a path has been deleted.
> * deprecate "-r" by changing the help to say so
>
> but those would both be incompatible UI changes. Would either of them be
> desirable on balance?
I think -r is the logical usage - not URL@REV. It fits with up, log,
blame, etc.
> In short, I'm trying to find a migration path from having this horrible meaning
> of "-r" to eventually either not having "-r" or having it specify operative
> revisions as it does in other commands and particularly in other forms of "svn
> diff".
>
> Does anyone see any better steps along that road?
I'd go back to our 1.0 UI and drop peg revs entirely. *duck* -- justin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org