Philip Martin wrote:
> John Szakmeister <john@szakmeister.net> writes:
>
>
>> However, I didn't implement the input validation. I sat down to do
>> it twice, and I kept stumbling on how to create an appropriate
>> error for the user. I think it would be confusing to say something
>> like "Must be one of revision BASE, COMMITTED, WORKING, or
>> UNSPECIFIED" or that a revision is disallowed for any reason.
>
>
> How else is the user going to know what to change?
The user can do nothing about this error. If this function fails, it's
because there is incorrect logic in the svn_client_cat2().
>
>> The user did nothing wrong,
>
>
> Something is wrong since the revision cannot be used.
Nope. See above.
>
>> and may have supplied a completely sound argument. So the attached
>> patch has an assert() statement the for the moment. Do you have
>> suggestions for what an appropriate error might look like?
>
>
> How about SVN_ERR_UNSUPPORTED_FEATURE? I think any error is better
> than an assert, for example you are forcing TSVN to implement the
> same restriction to avoid the assert.
>
No, absolutely not. Karl was talking about making cat_local_file()
return an error if the revision isn't what we expect. From a user
perspective, 'svn cat -rBASE' makes sense. If we fail because an
internal API's pre-condition was violated, saying something to the
effect that "'svn cat -rBASE' is not allowed" would be very confusing to
the user. I think it would be even more frustrating to find out it
wasn't a user error at all, but an internal library one. Keep in mind
cat_local_file() is a static helper function, with one caller. :-) And
no, TortoiseSVN doesn't have to add similar code to avoid a problem.
It's all hidden behind svn_client_cat2().
I don't mind making it a runtime error, I just don't know what the
appropriate, non-confusing feedback would be to a user. An assert()
comes off as an obvious internal application error, where as most
feedback on the command-line is a result of user error. I just want
them to know the difference between their mistake and one that's
obviously ours.
-John
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri May 13 17:27:37 2005