Some peg rev help text missing from command-line client
9 answers - 1342 bytes -

Sat, 06 May 2006, Giovanni Bajo wrote:
Garrett Rooney <rooneg (AT) electricjellyfish (DOT) netwrote:
I believe Dan Rall added support for that to svn log already. Not
sure about info.
It looks like "svn info" support for peg-revs was added in r13144 (but the help
screen doesn't explicitally mention its support).
It appears that several commands to the command-line binary are
missing help text along the lines of what we have for 'checkout':
$ svn help co
checkout (co): Check out a working copy from a repository.
usage: checkout URL[@REV] [PATH]
If specified, REV determines in which revision the URL is first
looked up.
In addition, some of the help text -- for the commands that have it --
is inconsistent. At least in some cases, it is appropriately
inconsistent.
* Mentions peg revs:
blame, cat, checkout, diff, export, list, merge
* Does not apply:
add, cleanup, commit, delete, help, import, lock, mkdir, move,
[propdel, propedit, propget, proplist, propset] (even
non-revprops?), resolved, revert, status, switch, unlock, update
* Does not mention, but needs to now/future:
copy, info, log
Does this look like the appropriate set of commands which need peg rev
help text added for them?
No.1 | | 881 bytes |
| 
Giovanni Bajo wrote:
Daniel Rall wrote:
Mentions peg revs:
>blame, cat, checkout, diff, export, list, merge
Does not apply:
>add, cleanup, commit, delete, help, import, lock, mkdir, move,
>[propdel, propedit, propget, proplist, propset] (even
>non-revprops?), resolved, revert, status, switch, unlock, update
Does not mention, but needs to now/future:
>copy, info, log
>>
>>Does this look like the appropriate set of commands which need peg rev
>>help text added for them?
FWIW, it's fine by me!
thing that seems wrong in this list is 'switch' -- can we not use the
peg-rev syntax with 'svn switch'? If not, that's a(n admittedly minor)
shortcoming.
No.2 | | 646 bytes |
| 
Daniel Rall wrote:
>* Mentions peg revs:
blame, cat, checkout, diff, export, list, merge
>* Does not apply:
add, cleanup, commit, delete, help, import, lock, mkdir, move,
[propdel, propedit, propget, proplist, propset] (even
non-revprops?), resolved, revert, status, switch, unlock, update
>* Does not mention, but needs to now/future:
copy, info, log
>
>Does this look like the appropriate set of commands which need peg rev
>help text added for them?
FWIW, it's fine by me!
No.3 | | 1759 bytes |
| 
Tue, 09 May 2006, C. Michael Pilato wrote:
Giovanni Bajo wrote:
Daniel Rall wrote:
Mentions peg revs:
>blame, cat, checkout, diff, export, list, merge
Does not apply:
>add, cleanup, commit, delete, help, import, lock, mkdir, move,
>[propdel, propedit, propget, proplist, propset] (even
>non-revprops?), resolved, revert, status, switch, unlock, update
Does not mention, but needs to now/future:
>copy, info, log
>>
>>Does this look like the appropriate set of commands which need peg rev
>>help text added for them?
FWIW, it's fine by me!
thing that seems wrong in this list is 'switch' -- can we not use the
peg-rev syntax with 'svn switch'? If not, that's a(n admittedly minor)
shortcoming.
Yeah, I debated on that one (which also applies to 'switch
'). Neither prototype from svn_client.h includes a peg rev
parameter:
svn_error_t *
svn_client_switch(svn_revnum_t *result_rev,
const char *path,
const char *url,
const svn_opt_revision_t *revision,
svn_boolean_t recurse,
svn_client_ctx_t *ctx,
apr_pool_t *pool);
svn_error_t *
svn_client_relocate(const char *dir,
const char *from,
const char *to,
svn_boolean_t recurse,
svn_client_ctx_t *ctx,
apr_pool_t *pool);
While a peg rev for the PATH parameter seems fairly redundant with
RESULT_REV, perhaps I should provide it for the sake of UI
consistency? Looks like this would involve rev'ing the
svn_client_switch() API.
And how about svn_client_relocate()?
No.4 | | 1615 bytes |
| 
Daniel Rall wrote:
>thing that seems wrong in this list is 'switch' -- can we not use the
>>peg-rev syntax with 'svn switch'? If not, that's a(n admittedly minor)
>>shortcoming.
Yeah, I debated on that one (which also applies to 'switch
'). Neither prototype from svn_client.h includes a peg rev
parameter:
svn_error_t *
svn_client_switch(svn_revnum_t *result_rev,
const char *path,
const char *url,
const svn_opt_revision_t *revision,
svn_boolean_t recurse,
svn_client_ctx_t *ctx,
apr_pool_t *pool);
svn_error_t *
svn_client_relocate(const char *dir,
const char *from,
const char *to,
svn_boolean_t recurse,
svn_client_ctx_t *ctx,
apr_pool_t *pool);
While a peg rev for the PATH parameter seems fairly redundant with
RESULT_REV, perhaps I should provide it for the sake of UI
consistency? Looks like this would involve rev'ing the
svn_client_switch() API.
RESULT_REV tells the revision number that you were switched to, and is not
the same as a peg revision. (It's more closely related to the REVISIN
argument, which can represent a revision number or revision keyword.)
And how about svn_client_relocate()?
Relocate doesn't even have an operative revision, so I think it doesn't need
to have a peg revision either. It's really just about rewriting working
copy URLs, and the whole point is that you *shouldn't* be using it to
traverse Time (only Space).
No.5 | | 1446 bytes |
| 
Tue, 09 May 2006, C. Michael Pilato wrote:
Daniel Rall wrote:
>thing that seems wrong in this list is 'switch' -- can we not use the
>>peg-rev syntax with 'svn switch'? If not, that's a(n admittedly minor)
>>shortcoming.
Yeah, I debated on that one (which also applies to 'switch
'). Neither prototype from svn_client.h includes a peg rev
parameter:
svn_error_t *
svn_client_switch(svn_revnum_t *result_rev,
const char *path,
const char *url,
const svn_opt_revision_t *revision,
svn_boolean_t recurse,
svn_client_ctx_t *ctx,
apr_pool_t *pool);
While a peg rev for the PATH parameter seems fairly redundant with
RESULT_REV, perhaps I should provide it for the sake of UI
consistency? Looks like this would involve rev'ing the
svn_client_switch() API.
RESULT_REV tells the revision number that you were switched to, and is not
the same as a peg revision. (It's more closely related to the REVISIN
argument, which can represent a revision number or revision keyword.)
Mike, would you mind describing how the use cases for the following
commands differ?:
svn switch -r37
svn switch @37
I'd have thought both commands were intended to "switch my working
copy to the 'foo' branch as it looked in r37."
No.6 | | 3381 bytes |
| 
Tue, 09 May 2006, Giovanni Bajo wrote:
Daniel Rall wrote:
>* Mentions peg revs:
blame, cat, checkout, diff, export, list, merge
>* Does not apply:
add, cleanup, commit, delete, help, import, lock, mkdir, move,
[propdel, propedit, propget, proplist, propset] (even
non-revprops?), resolved, revert, status, switch, unlock, update
>* Does not mention, but needs to now/future:
copy, info, log
>
>Does this look like the appropriate set of commands which need peg rev
>help text added for them?
FWIW, it's fine by me!
Hrm, svn_client_copy3() doesn't take an explicit peg rev either, only
a SRC_REVISIN parameter:
svn_error_t *
svn_client_copy3(svn_commit_info_t **commit_info_p,
const char *src_path,
const svn_opt_revision_t *src_revision,
const char *dst_path,
svn_client_ctx_t *ctx,
apr_pool_t *pool);
For 'copy', I suppose that these two (redudant) UIs for specifying
SRC_REVISIN should be handled in subversion/svn/copy-cmd.c, rather
than via the API? AFAICT, specifying SRC_REVISIN via a peg rev is
currently not handled there.
(This is a related discussion to the one about 'switch'.)
Here's a patch adding documentation for 'info' and 'log', which I will
propose for backport to 1.4.x after it's committed:
[[[
* subversion/svn/main.c
(svn_cl__cmd_table): Document that 'info' and 'log' accept a peg
rev.
Found by: giovannibajo
]]]
Index: subversion/svn/main.c
subversion/svn/main.c(revision 19580)
subversion/svn/main.c(working copy)
@@ -367,10 +367,11 @@
{ "info", svn_cl__info, {0}, N_
("Display information about a local or remote item.\n"
- "usage: info [TARGET]\n"
+ "usage: info [TARGET[@REV]]\n"
"\n"
" Print information about each TARGET (default: '.')\n"
- " TARGET may be either a working-copy path or URL.\n"),
+ " TARGET may be either a working-copy path or URL. If specified, REV\n"
+ " determines in which revision the target is first looked up.\n\n"),
{'r', 'R', svn_cl__targets_opt, svn_cl__incremental_opt, svn_cl__xml_opt,
SVN_CL__AUTHPTINS, svn_cl__config_dir_opt} },
@@ -411,13 +412,15 @@
{ "log", svn_cl__log, {0}, N_
("Show the log messages for a set of revision(s) and/or file(s).\n"
"usage: 1. log [PATH]\n"
- " 2. log URL [PATH]\n"
+ " 2. log URL[@REV] [PATH]\n"
"\n"
" 1. Print the log messages for a local PATH (default: '.').\n"
" The default revision range is BASE:1.\n"
"\n"
" 2. Print the log messages for the PATHs (default: '.') under URL.\n"
- " The default revision range is HEAD:1.\n"
+ " If specified, REV determines in which revision the URL is first\n"
+ " looked up. The default revision range is HEAD:1.\n"
+ " \n"
"\n"
" With -v, also print all affected paths with each log message.\n"
" With -q, don't print the log message body itself (note that this is\n"
PGP SIGNATURE
Version: GnuPG v1.4.2.2 (GNU/Linux)
rF67tTGuAtFG55j2rdP64xo=
=sSRN
PGP SIGNATURE
No.7 | | 1460 bytes |
| 
Daniel Rall wrote:
Mike, would you mind describing how the use cases for the following
commands differ?:
Sure, this is the whole reason the peg revision / operative revision syntax
discrimination exists:
svn switch -r37
Adding implied values, this is:
svn switch -r37 @HEAD
So, switch my working copy to the way that the thing that currently (in
HEAD) lives at branches/foo looked back in revision 37 (even though it might
not have lived at branches/foo back in that revision).
svn switch @37
Adding implied values, this is:
svn switch -rHEAD @37
So, switch my working copy to the way that the thing that lived at
branches/foo back in revision 37 looks today in HEAD (even though it might
have since been moved to some other place).
See the difference?
There's a little gotcha in the second case in that Subversion can't really
search forward in history, so it just crosses its fingers and says, "I don't
know how to find out definitively where /branches/foo@37 might possible live
in HEAD (which could be multiple places, thanks to copies), but I can at
least see if some fork of that same line of history can still be found at
/branches/foo and, if so, use that."
I'd have thought both commands were intended to "switch my working copy
to the 'foo' branch as it looked in r37."
Nuh-uh. See above.
No.8 | | 417 bytes |
| 
C. Michael Pilato wrote:
Daniel Rall wrote:
>>svn switch @37
Adding implied values, this is:
svn switch -rHEAD @37
Actually, I should back up a step here. I don't recall which way the
community went on this discussion, whether an unspecified operational
revision default to HEAD or to the same value as the peg revision.
No.9 | | 693 bytes |
| 
May 9, 2006, at 10:59 AM, C. Michael Pilato wrote:
C. Michael Pilato wrote:
>Daniel Rall wrote:
svn switch @37
>>
>Adding implied values, this is:
>>
>svn switch -rHEAD @37
>
Actually, I should back up a step here. I don't recall which way the
community went on this discussion, whether an unspecified operational
revision default to HEAD or to the same value as the peg revision.
Blast back to the past, but the operative revision defaults to the
peg revision in my testing of 1.4.2.
Regards,
Blair