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

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

Signed-off-by: Nicolas Vigier <redacted>
---
 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

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 <redacted>

Thanks.

[...]

[...]

-	* 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

On Mon, 14 Oct 2013, Jonathan Nieder wrote:

Nicolas Vigier wrote:

quoted

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

Signed-off-by: Nicolas Vigier <redacted>

Thanks.

[...]

quoted

--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt

@@ -259,9 +259,10 @@ Each line of options has this format:

[...]

quoted

-	* 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.

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

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

On Mon, Oct 14, 2013 at 05:25:29PM +0200, Nicolas Vigier wrote:

quoted

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.