[PATCH] Improve documentation for git-sh-setup.

[PATCH] Improve documentation for git-sh-setup.

[Yann Dirson]

Signed-off-by: Yann Dirson <ydirson@altern.org>
Cc: Pierre Habouzit <madcoder@debian.org>
---

 Documentation/git-sh-setup.txt |   23 ++++++++++++++++++-----
 1 files changed, 18 insertions(+), 5 deletions(-)


diff --git a/Documentation/git-sh-setup.txt b/Documentation/git-sh-setup.txt
index 16b8b75..75e6846 100644
--- a/Documentation/git-sh-setup.txt
+++ b/Documentation/git-sh-setup.txt
@@ -19,12 +19,25 @@ Porcelain-ish scripts and/or are writing new ones.
 The `git-sh-setup` scriptlet is designed to be sourced (using
 `.`) by other shell scripts to set up some variables pointing at
 the normal git directories and a few helper shell functions.
+It also provides help messages, and some amount of command-line
+parsing.
 
-Before sourcing it, your script should set up a few variables;
-`USAGE` (and `LONG_USAGE`, if any) is used to define message
-given by `usage()` shell function.  `SUBDIRECTORY_OK` can be set
-if the script can run from a subdirectory of the working tree
-(some commands do not).
+Before sourcing it, your script should set up a few variables.
+
+`OPTIONS_SPEC` allows to declare the options so that the command-line
+can be pre-processed, and the usage message autogenerated.  It must
+follow the rules which apply to arguments to `git rev-parse
+--parseopt`.  If you need `git-rev-parse --parseopt` to keep the `--`
+the user may have passed to your command, you should also set
+`OPTIONS_KEEPDASHDASH` to a non empty value.
+
+If the constraints imposed by the use of `git rev-parse` do not fit
+for a particular tool, `USAGE` (and `LONG_USAGE`, if any) can be used
+to define message given by `usage()` shell function; no command-line
+pre-processing occurs, the script has entire control over it.
+
+`SUBDIRECTORY_OK` can be set if the script can run from a subdirectory
+of the working tree (some commands do not).
 
 The scriptlet sets `GIT_DIR` and `GIT_OBJECT_DIRECTORY` shell
 variables, but does *not* export them to the environment.

Re: [PATCH] Improve documentation for git-sh-setup.

[Pierre Habouzit]

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

On Thu, Feb 21, 2008 at 11:01:28PM +0000, Yann Dirson wrote:
> Signed-off-by: Yann Dirson <ydirson@altern.org>
> Cc: Pierre Habouzit <madcoder@debian.org>


> +If the constraints imposed by the use of `git rev-parse` do not fit
> +for a particular tool, `USAGE` (and `LONG_USAGE`, if any) can be used
> +to define message given by `usage()` shell function; no command-line
> +pre-processing occurs, the script has entire control over it.

  Actually, new scripts should be written using git rev-parse if
possible, the USAGE/LONG_USAGE were there _before_ and I'd like to call
them the deprecated interface if other git hackers don't mind.

  git rev-parse --parseopt gives consistency in how git parses options,
and it's A Good Thing™

  That makes me think that git-sh-setup(1) use should be documented in
gitcli(5), maybe even included, and git-sh-setup(1) should *definitely*
link to gitcli(5). We wondered where to link gitcli from, this is
definitely the place !

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

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

Re: [PATCH] Improve documentation for git-sh-setup.

[Junio C Hamano]

Pierre Habouzit <madcoder@debian.org> writes:

>   Actually, new scripts should be written using git rev-parse if
> possible, the USAGE/LONG_USAGE were there _before_ and I'd like to call
> them the deprecated interface if other git hackers don't mind.
>
>   git rev-parse --parseopt gives consistency in how git parses options,
> and it's A Good Thing™

I certainly think encouraging use of parse-options in new
scripts is a good idea, even though I suspect "deprecated" above
might be a bit too strong.

I did not find accessible from the command line variant was the
parse-opt-hidden feature, which was frustrating.

>   That makes me think that git-sh-setup(1) use should be documented in
> gitcli(5), maybe even included, and git-sh-setup(1) should *definitely*
> link to gitcli(5). We wondered where to link gitcli from, this is
> definitely the place !

Sounds good.  Please make it so.

Re: [PATCH] Improve documentation for git-sh-setup.

[Pierre Habouzit]

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

On Fri, Feb 22, 2008 at 05:17:51PM +0000, Junio C Hamano wrote:
> Pierre Habouzit <madcoder@debian.org> writes:
> 
> >   Actually, new scripts should be written using git rev-parse if
> > possible, the USAGE/LONG_USAGE were there _before_ and I'd like to call
> > them the deprecated interface if other git hackers don't mind.
> >
> >   git rev-parse --parseopt gives consistency in how git parses options,
> > and it's A Good Thing™
> 
> I certainly think encouraging use of parse-options in new
> scripts is a good idea, even though I suspect "deprecated" above
> might be a bit too strong.

  Okay.

> I did not find accessible from the command line variant was the
> parse-opt-hidden feature, which was frustrating.

  Well, it's probably doable using some more magic flags, I can hack
something if you need to, I'll try to work something out during FOSDEM's
talks :)

> >   That makes me think that git-sh-setup(1) use should be documented in
> > gitcli(5), maybe even included, and git-sh-setup(1) should *definitely*
> > link to gitcli(5). We wondered where to link gitcli from, this is
> > definitely the place !
> 
> Sounds good.  Please make it so.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

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

[PATCH] parse-opt: bring PARSE_OPT_HIDDEN to git-rev-parse --parseopt

[Pierre Habouzit]

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

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
    On ven, fév 22, 2008 at 06:19:28 +0000, Pierre Habouzit wrote:
    > On Fri, Feb 22, 2008 at 05:17:51PM +0000, Junio C Hamano wrote:
    > > I did not find accessible from the command line variant was the
    > > parse-opt-hidden feature, which was frustrating.
    > 
    >   Well, it's probably doable using some more magic flags, I can hack
    > something if you need to, I'll try to work something out during FOSDEM's
    > talks :)

      Here it is …
      I also made the parsing more extensible wrt new flags if needed.

      We should definitely write tests too.

 Documentation/git-rev-parse.txt |   15 ++++++++++-----
 builtin-rev-parse.c             |   25 ++++++++++++++-----------
 2 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index f02f6bb..e961c20 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -325,7 +325,7 @@ The lines after the separator describe the options.
 Each line of options has this format:
 
 ------------
-<opt_spec><arg_spec>? SP+ help LF
+<opt_spec><flags>? SP+ help LF
 ------------
 
 `<opt_spec>`::
@@ -334,10 +334,15 @@ Each line of options has this format:
 	is necessary. `h,help`, `dry-run` and `f` are all three correct
 	`<opt_spec>`.
 
-`<arg_spec>`::
-	an `<arg_spec>` tells the option parser if the option has an argument
-	(`=`), an optional one (`?` though its use is discouraged) or none
-	(no `<arg_spec>` in that case).
+`<flags>`::
+	`<flags>` are any suite of `*`, `=` or `?`.
+	* Use `=` if the option take an argument.
+
+	* Use `?` to mean that the option is optional (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
+	  documented in linkgit:gitcli[5].
 
 The remainder of the line, after stripping the spaces, is used
 as the help associated to the option.
diff --git a/builtin-rev-parse.c b/builtin-rev-parse.c
index b9af1a5..5ffc4e0 100644
--- a/builtin-rev-parse.c
+++ b/builtin-rev-parse.c
@@ -322,18 +322,21 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)
 		o->type = OPTION_CALLBACK;
 		o->help = xstrdup(skipspaces(s));
 		o->value = &parsed;
+		o->flags = PARSE_OPT_NOARG;
 		o->callback = &parseopt_dump;
-		switch (s[-1]) {
-		case '=':
-			s--;
-			break;
-		case '?':
-			o->flags = PARSE_OPT_OPTARG;
-			s--;
-			break;
-		default:
-			o->flags = PARSE_OPT_NOARG;
-			break;
+		while (s > sb.buf && strchr("*=?", s[-1])) {
+			switch (*--s) {
+			case '=':
+				o->flags &= ~PARSE_OPT_NOARG;
+				break;
+			case '?':
+				o->flags &= ~PARSE_OPT_NOARG;
+				o->flags |= PARSE_OPT_OPTARG;
+				break;
+			case '*':
+				o->flags &= PARSE_OPT_HIDDEN;
+				break;
+			}
 		}
 
 		if (s - sb.buf == 1) /* short option only */
-- 
1.5.4.2.281.g28d0e


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

Re: [PATCH] parse-opt: bring PARSE_OPT_HIDDEN to git-rev-parse --parseopt

[Junio C Hamano]

Pierre Habouzit <madcoder@debian.org> writes:

>       I also made the parsing more extensible wrt new flags if needed.
>
>       We should definitely write tests too.
> ...
> +			case '*':
> +				o->flags &= PARSE_OPT_HIDDEN;
> +				break;

I have a slight suspicion that this is not what you meant.
You probably meant either |= or perhaps ^=.

Re: [PATCH] parse-opt: bring PARSE_OPT_HIDDEN to git-rev-parse --parseopt

[Pierre Habouzit]

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

On Sun, Feb 24, 2008 at 08:42:26AM +0000, Junio C Hamano wrote:
> Pierre Habouzit <madcoder@debian.org> writes:
> 
> >       I also made the parsing more extensible wrt new flags if needed.
> >
> >       We should definitely write tests too.
> > ...
> > +			case '*':
> > +				o->flags &= PARSE_OPT_HIDDEN;
> > +				break;
> 
> I have a slight suspicion that this is not what you meant.
> You probably meant either |= or perhaps ^=.

  oooh boy of course it's a |=...

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

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