[PATCH] rev-parse doc: clarify use of optional / required arguments

[PATCH] rev-parse doc: clarify use of optional / required arguments

[Nicolas Vigier]

"the option is optional" was confusing as it is not the option but its
argument which is optional.

Signed-off-by: Nicolas Vigier <boklm@mars-attacks.org>
---
 Documentation/git-rev-parse.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 2b126c0..7ec9221 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -259,9 +259,10 @@ Each line of options has this format:
 
 `<flags>`::
 	`<flags>` are of `*`, `=`, `?` or `!`.
-	* Use `=` if the option takes an argument.
+	* Use `=` if the option requires an argument.
 
-	* Use `?` to mean that the option is optional (though its use is discouraged).
+	* Use `?` if the option takes an optional argument (though its use is
+	  discouraged).
 
 	* Use `*` to mean that this option should not be listed in the usage
 	  generated for the `-h` argument. It's shown for `--help-all` as
-- 
1.8.4

Re: [PATCH] rev-parse doc: clarify use of optional / required arguments

[Jonathan Nieder]

Nicolas Vigier wrote:

> "the option is optional" was confusing as it is not the option but its
> argument which is optional.
>
> Signed-off-by: Nicolas Vigier <boklm@mars-attacks.org>

Thanks.

[...]
> --- a/Documentation/git-rev-parse.txt
> +++ b/Documentation/git-rev-parse.txt
> @@ -259,9 +259,10 @@ Each line of options has this format:
[...]
> -	* Use `?` to mean that the option is optional (though its use is discouraged).
> +	* Use `?` if the option takes an optional argument (though its use is
> +	  discouraged).

This still seems confusing: the argument's use is discouraged?

Would something like the following make sense?

	* Use `?` if [...]. This is used for historical options
	  that once took no argument and later gained an optional
	  argument. Use of this feature for new options is
	  discouraged.

Hope that helps,
Jonathan

Re: [PATCH] rev-parse doc: clarify use of optional / required arguments

[Nicolas Vigier]

On Mon, 14 Oct 2013, Jonathan Nieder wrote:

> Nicolas Vigier wrote:
> 
> > "the option is optional" was confusing as it is not the option but its
> > argument which is optional.
> >
> > Signed-off-by: Nicolas Vigier <boklm@mars-attacks.org>
> 
> Thanks.
> 
> [...]
> > --- a/Documentation/git-rev-parse.txt
> > +++ b/Documentation/git-rev-parse.txt
> > @@ -259,9 +259,10 @@ Each line of options has this format:
> [...]
> > -	* Use `?` to mean that the option is optional (though its use is discouraged).
> > +	* Use `?` if the option takes an optional argument (though its use is
> > +	  discouraged).
> 
> This still seems confusing: the argument's use is discouraged?
> 
> Would something like the following make sense?
> 
> 	* Use `?` if [...]. This is used for historical options
> 	  that once took no argument and later gained an optional
> 	  argument. Use of this feature for new options is
> 	  discouraged.

Actually I don't know why it is discouraged, and I'm not sure that
it should be. It was listed as discouraged since commit 21d47835 adding
parseopt to git-rev-parse, but I have no idea why. I'm adding the author
of this commit in Cc.

The reason that I looked at this documentation in the first place was
that I was looking at adding an option '-S[<keyid>], --gpg-sign[=<keyid>]'
to git-rebase, similar to the option in git-commit, so that rebased
commits can be signed. In git-commit this option takes an optional argument,
so I think it would make sense to make it optional in git-rebase too.

Re: [PATCH] rev-parse doc: clarify use of optional / required arguments

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 923 bytes --]

On Mon, Oct 14, 2013 at 05:25:29PM +0200, Nicolas Vigier wrote:
> The reason that I looked at this documentation in the first place was
> that I was looking at adding an option '-S[<keyid>], --gpg-sign[=<keyid>]'
> to git-rebase, similar to the option in git-commit, so that rebased
> commits can be signed. In git-commit this option takes an optional argument,
> so I think it would make sense to make it optional in git-rebase too.

It's funny you say that, because I literally started on that yesterday.
I have cherry-pick and revert working, but I haven't gotten to anything
else yet.  Feel free to work on it if you're interested, as I probably
won't get around to finishing it for some time.

-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] rev-parse doc: clarify use of optional / required arguments

[Nicolas Vigier]

[-- Attachment #1: Type: text/plain, Size: 1335 bytes --]

On Mon, 14 Oct 2013, brian m. carlson wrote:

> On Mon, Oct 14, 2013 at 05:25:29PM +0200, Nicolas Vigier wrote:
> > The reason that I looked at this documentation in the first place was
> > that I was looking at adding an option '-S[<keyid>], --gpg-sign[=<keyid>]'
> > to git-rebase, similar to the option in git-commit, so that rebased
> > commits can be signed. In git-commit this option takes an optional argument,
> > so I think it would make sense to make it optional in git-rebase too.
> 
> It's funny you say that, because I literally started on that yesterday.
> I have cherry-pick and revert working, but I haven't gotten to anything
> else yet.  Feel free to work on it if you're interested, as I probably
> won't get around to finishing it for some time.

I have a patch for git-am working, but with a small problem : it has to
assume that the optional argument to -S does not start with a dash.
This is because git-rev-parse --parseopt does not allow us to see the
difference between -S-q and -S -q, we don't know if -q is the next
option or the argument to -S.

Maybe that's the reason why the use of optional argument options with
git rev-parse --parseopt is discouraged ?

I'm going to send a patch proposal so that rev-parse --parseopt gives an
empty argument for an unset optional argument.


[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]