Re: [[[PATCH]]] fix svn cat to skip unversioned resources

On Thu, 19 May 2005 vivek@collab.net wrote:

> [[[
> Fix svn cat so that it skips unversioned items
>
> * subversion/clients/cmdline/cat-cmd.c
> (svn_cl__cat): added logic to skip unversioned items

Please capitalize3 and end sentences with a period.

> There are other commands also which shows this behavior. Should those be
> fixed?
>
I think so.

> Index: subversion/clients/cmdline/cat-cmd.c
> ===================================================================
> --- subversion/clients/cmdline/cat-cmd.c (revision 14776)
> +++ subversion/clients/cmdline/cat-cmd.c (working copy)
> @@ -42,6 +42,7 @@
> int i;
> svn_stream_t *out;
> apr_pool_t *subpool = svn_pool_create (pool);
> + svn_error_t *err;
>
> SVN_ERR (svn_opt_args_to_target_array2 (&targets, os,
> opt_state->targets, pool));
> @@ -65,9 +66,25 @@
> SVN_ERR (svn_opt_parse_path (&peg_revision, &truepath, target,
> subpool));
>
> - SVN_ERR (svn_client_cat2 (out, truepath, &peg_revision,
> + err = (svn_client_cat2 (out, truepath, &peg_revision,
> &(opt_state->start_revision),
> ctx, subpool));

Indentation needs to be fixed. And please get rid of the extra parens
around the function call.

> + if (err)
> + {

The brace should be indented. (And the following lines relative to the
brace.)

> + if ((err->apr_err == SVN_ERR_ENTRY_NOT_FOUND) ||
> + (err->apr_err == SVN_ERR_UNVERSIONED_RESOURCE)||
> + (err->apr_err == SVN_ERR_CLIENT_IS_DIRECTORY))

the substequent lines are misaligned, and operators should be at the
beginning of the new ine.

> + {
> + if (!opt_state->quiet)
> + {
> + svn_handle_warning (stderr, err);

Too much indentation.

> + }

cat doesn't have the --quiet option and if it had, I'm not sure if we
wanted to shut these warnings up. But we are already inconsistent
here. proplist does that, but all other places which use
svn_handle_warning don't.

> + svn_error_clear (err);
> + continue;
> + }
> + else
> + return err;
> + }
> }
> svn_pool_destroy (subpool);

It looks like you need to take a look in HACKING for our style
guidelines:-)

Also, the log message should say that you even continue in some other
cases (is a directory and not found).

Could you provide a test case with the next version of this patch?

Regards,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu May 19 14:09:26 2005