[PATCH][RFC] Improve error message returned by proplist/propget

Hello,

'svn propget' and 'svn proplist' currently return less-than-useful error
messages for non-existent paths, compared with, say, 'svn ls':

$ svn ls http://svn.collab.net/repos/svn/does/not/exist
svn: URL 'http://svn.collab.net/repos/svn/does/not/exist' non-existent in that revision
$ svn pg svn:ignore http://svn.collab.net/repos/svn/does/not/exist
svn: Unknown node kind for 'http://svn.collab.net/repos/svn/does/not/exist'
$ svn pl http://svn.collab.net/repos/svn/does/not/exist
svn: Unknown node kind for 'http://svn.collab.net/repos/svn/does/not/exist'

The attached patch adds an explicit check to propget and proplist so
that they produce the same error as 'svn ls'.

It also changes the error return in this case from SVN_ERR_NODE_UNKNOWN_KIND
to SVN_ERR_FS_NOT_FOUND, which I think is a significant improvement.

However.

svn_client_proplist2()'s docstring actually does specify the error that
should be returned in this case, and it specifies SVN_ERR_ENTRY_NOT_FOUND.
But that error doesn't really make sense in the context of a remote
operation - it's typically a working-copy error. Furthermore, if we
kept with the usual message text for that error, it wouldn't be much
better than the current one: "'http://foo/bar' is not under version
control". It works, just, but it's inconsistent with just about every
other use of SVN_ERR_ENTRY_NOT_FOUND.

So, what to do? I could keep the error code as SVN_ERR_ENTRY_NOT_FOUND
and return the error message from SVN_ERR_FS_NOT_FOUND, but that seems
a little crazy to me.

Alternatively, we could change the docstring for svn_client_proplist2()
to say that _remote_ operations can return SVN_ERR_FS_NOT_FOUND
instead. Normally I'd be against that, but as it's never worked so far,
we can't possibly break anyone.

Gah! And actually svn_client_proplist2() doesn't return
SVN_ERR_ENTRY_NOT_FOUND anyway: it returns SVN_ERR_UNVERSIONED_RESOURCE!
Can we just ditch the line from the docstring that purports to specify
a specific error return in this case?

[[[
* subversion/libsvn_client/prop_commands.c
  (svn_client_propget2, svn_client_proplist2): Explicitly check whether the
    specified path existed, and return SVN_ERR_FS_NOT_FOUND and an
    appropriate error message if not, rather than leaving it to
    remote_propget() and remote_proplist() to return a generic (and
    generally unhelpful) message to the user.
]]]

Regards,
Malcolm

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org