Subject: Re: [PATCH] rev-parse: clarify documentation for the --verify option

From: Michael Haggerty <[email-masked]>
Date: Tue, 02 Apr 2013 09:49:13 +0200

On 04/01/2013 06:56 PM, Junio C Hamano wrote:
> Junio C Hamano <[email-masked]> writes:
> 
>> Because the primary use case of this option is to implement end-user
>> input validation, I think it would be helpful to clarify use of the
>> peeler here.  Perhaps
>> ...
> 
> A "SQUASH???" patch on top of your original is queued on 'pu',
> together with the earlier "^{object}" peeler patch.  Comments,
> improvements, etc. would be nice.

Yes, your version is better.  I would make one change, though.  In your

+	Make sure the single given parameter can be turned into a
+	raw 20-byte SHA-1 that can be used to access the object
+	database, and emit it to the standard output. If it can't,
+	error out.

it could be made clearer that exactly one parameter should be provided.
 Maybe

+	Verify that exactly one parameter is provided, and that it
+       can be turned into a raw 20-byte SHA-1 that can be used to
+	access the object database.  If so, emit the SHA-1 to the
+	standard output; otherwise, error out.

But this makes it sound a little like the "raw 20-byte SHA-1" will be
output to stdout, whereas both the input and the output are in fact
40-character hex-encoded SHA-1s.  Perhaps a further change

    s/raw 20-byte SHA-1/full SHA-1/

would avoid the false implication?

Michael

-- 
Michael Haggerty
[email-masked]
http://softwareswirl.blogspot.com/