threads / patch / 35137

patchrev-parse doc: clarify use of optional / required arguments

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

## tl;dr

5 messages between Oct 14, 2013 and Oct 15, 2013. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

Nicolas Vigier· Oct 14, 2013, 14:23 UTC · lore

"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(-)
Show changes to Documentation/git-rev-parse.txt +3 −2
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
Jonathan Nieder· Oct 14, 2013, 14:36 UTC · re: Nicolas Vigier · lore

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

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

Nicolas Vigier· Oct 14, 2013, 15:25 UTC · re: Jonathan Nieder · lore

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

On Mon, 14 Oct 2013, Jonathan Nieder wrote:
Show 26 quoted lines
> 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.

brian m. carlson· Oct 14, 2013, 23:57 UTC · re: Nicolas Vigier · lore

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

On Mon, Oct 14, 2013 at 05:25:29PM +0200, Nicolas Vigier wrote:
Show 5 quoted lines
> 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
Nicolas Vigier· Oct 15, 2013, 10:49 UTC · re: brian m. carlson · lore

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

On Mon, 14 Oct 2013, brian m. carlson wrote:
Show 11 quoted lines
> 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.

← back to recent threads