Re: 'svnlook proplist' requires path argument?

Daniel Rall <dlr@collab.net> writes:
> 'svnlook proplist' documents that its path argument is optional:
>
> $ svn help proplist
> proplist (plist, pl): List all properties on files, dirs, or revisions.
> usage: 1. proplist [TARGET[@REV]...]
> 2. proplist --revprop -r REV [TARGET]
> ...
>
> However, it's behavior in trunk shows it to be required (and has been
> since its last modification in r12605 for issue #2181?):
>
> cmdline$ svnlook pl svn-test-work/repositories/svnsync_tests-1
> subversion/svnlook/main.c:1926: (apr_err=205001)
> svnlook: Missing repository path argument
> cmdline$ svnlook pl svn-test-work/repositories/svnsync_tests-1 /
>
> The code in subversion/svnlook/main.c doesn't default to a path of "/"
> (as I'd expect it to):
>
> static svn_error_t *
> subcommand_plist (apr_getopt_t *os, void *baton, apr_pool_t *pool)
> {
> struct svnlook_opt_state *opt_state = baton;
> svnlook_ctxt_t *c;
>
> if (!opt_state->revprop && opt_state->arg1 == NULL)
> return svn_error_create
> (SVN_ERR_CL_INSUFFICIENT_ARGS, NULL,
> _("Missing repository path argument"));
>
> SVN_ERR (get_ctxt_baton (&c, opt_state, pool));
> SVN_ERR (do_plist (c, opt_state->revprop ? NULL : opt_state->arg1,
> opt_state->verbose, pool));
> return SVN_NO_ERROR;
> }
>
> The command-line argument validation which causes this behavior was
> added at the beginning of this year:
>
> ------------------------------------------------------------------------
> r12605 | maxb | 2005-01-05 16:58:15 -0800 (Wed, 05 Jan 2005) | 25 lines
> Changed paths:
> M /trunk/subversion/svnlook/main.c
> M /trunk/subversion/tests/clients/cmdline/svnlook_tests.py
> M /trunk/subversion/tests/clients/cmdline/svntest/main.py
>
> Fix issue 2181: Add "svnlook {propget,proplist} --revprop".
>
> Patch by: Oliver Klozoff <stevieoh@fastmail.fm>
> (Tweaked by me.)
>
> * subversion/svnlook/main.c
> (subcommand_plist, subcommand_pget):
> Pass NULL for the path argument to do_plist/do_pget
> when --revprop is specified.
>
> (do_plist, do_pget):
> When the 'path' argument is NULL, switch from svn_fs_node_proplist/
> svn_fs_node_prop to svn_fs_revision_proplist/svn_fs_revision_prop.
>
> * subversion/tests/clients/cmdline/svnlook_tests.py:
> (run_svnlook, expect): New utility functions.
> (test_youngest): Rename to...
> (test_misc): ...this, and make it test a whole lot more.
> (test_list): Added the new tests.
>
> * subversion/tests/clients/cmdline/svntest/main.py
> (run_command_stdin): New. Just like 'run_command',
> except it allows the caller to specify data to be piped into
> the command's stdin.
>
> ------------------------------------------------------------------------
>
> I assume that the documentation is correct, and the code needs to be
> changed accordingly?

I think the old behavior is exceedingly unwise, and would prefer that
we consider it a "bug" (or an oversight) and require the path. What
do you think?

-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