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

Re: [PATCH] Add "copied" subcommand to svnlook

From: Peter N. Lundblad <peter_at_famlundblad.se>
Date: 2005-09-27 23:45:03 CEST

On Tue, 27 Sep 2005, Paul Dugas wrote:

> On Tue, September 27, 2005 5:14 am, Peter N. Lundblad wrote:
> > What about calling this option --copies instead?
>
> No really preference here. Someone else suggested "--show-copy-info" so
> that's what I did. Who's the boss here?
>
Heh, the full committers (including me). We only have show- in one other
option, I think it is redundant, so lets settle on --copy-info unless
someone objects.

> > Why do you remove the leading slash? (And, be more generous with spaces
> > around operators, please.
>
> Just because that's how it looked in a suggestion I got earlier as to how
> it should look. I'm easy either way. Who makes this kind of "preference"
> call?
>
As above:-) I didn't find that suggestion for no slash at the beginning. I
don't think it should be remvoed, for consistency.

> > Else, it looks good. Would you mind creating a basic test case for this in
> > svnlook_tests.py?
>
> Um... Never been a Python programmer and at first glance at that script,
> I remember why. ;) I'll take another look and see what I can swing.
>
Looking forward to that. It is not hard, but a basic test case for the
format is a good starter.

> [[[
> Add "--show-copy-info" option to "svnlook changed" subcommand.
>
> With this option, output for new entries in the repository created
> using "svn copy" are prefixed with "A + " (the "+" is new) and an
> additional line appears after it with the details for where it was
> copied from (path and revision).
>
Good summary!

> Index: subversion/svnlook/main.c
> ===================================================================
> --- subversion/svnlook/main.c (revision 16222)
> +++ subversion/svnlook/main.c (working copy)
> @@ -479,6 +485,7 @@
> static svn_error_t *
> print_changed_tree (svn_repos_node_t *node,
> const char *path /* UTF-8! */,
> + svn_boolean_t show_copy_info,
> apr_pool_t *pool)
> {
> const char *full_path;
> @@ -511,10 +518,24 @@
> /* Print this node unless told to skip it. */
> if (print_me)
> {
> - SVN_ERR (svn_cmdline_printf (pool, "%s %s%s\n",
> - status,
> - path,
> - node->kind == svn_node_dir ? "/" : ""));
> + if (show_copy_info && node->copyfrom_path)
> + {
> + SVN_ERR (svn_cmdline_printf (pool, "%s+ %s%s\n",
> + status,
> + path,
> + node->kind == svn_node_dir ? "/" : ""));
> + SVN_ERR (svn_cmdline_printf (pool, " (from %s%s:r%ld)\n",
> + node->copyfrom_path,
> + (node->kind == svn_node_dir ? "/" : ""),
> + node->copyfrom_rev));
> + }
> + else
> + {
> + SVN_ERR (svn_cmdline_printf (pool, "%s %s%s\n",
> + status,
> + path,
> + node->kind == svn_node_dir ? "/" : ""));
> + }

I liked your previous way of doing this better, just with an extended
status char array to make room for the new column and a boolean
flag. It was more straight-forward and less code duplication.

Thanks,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Sep 27 23:48:25 2005

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.