# [PATCH] Improve documentation for git-sh-setup. 7 messages from 2008-02-21 to 2008-02-24. Participants: Yann Dirson, Pierre Habouzit, Junio C Hamano. Thread: https://gitlist.dev/t/12250 ## Yann Dirson, 2008-02-21 23:01 Subject: [PATCH] Improve documentation for git-sh-setup. Message-ID: <20080221230101.27644.49229.stgit@gandelf.nowhere.earth> URL: https://gitlist.dev/e/20080221230101.27644.49229.stgit%40gandelf.nowhere.earth ``` Signed-off-by: Yann Dirson Cc: Pierre Habouzit --- 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. ``` ## Pierre Habouzit, 2008-02-22 09:35 Subject: Re: [PATCH] Improve documentation for git-sh-setup. Message-ID: <20080222093558.GC29114@artemis.madism.org> URL: https://gitlist.dev/e/20080222093558.GC29114%40artemis.madism.org In-Reply-To: <20080221230101.27644.49229.stgit@gandelf.nowhere.earth> ``` On Thu, Feb 21, 2008 at 11:01:28PM +0000, Yann Dirson wrote: > Signed-off-by: Yann Dirson > Cc: Pierre Habouzit > +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 ``` ## Junio C Hamano, 2008-02-22 17:17 Subject: Re: [PATCH] Improve documentation for git-sh-setup. Message-ID: <7vbq68ap1c.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vbq68ap1c.fsf%40gitster.siamese.dyndns.org In-Reply-To: <20080222093558.GC29114@artemis.madism.org> ``` Pierre Habouzit 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. ``` ## Pierre Habouzit, 2008-02-22 18:19 Subject: Re: [PATCH] Improve documentation for git-sh-setup. Message-ID: <20080222181927.GA4555@artemis.madism.org> URL: https://gitlist.dev/e/20080222181927.GA4555%40artemis.madism.org In-Reply-To: <7vbq68ap1c.fsf@gitster.siamese.dyndns.org> ``` On Fri, Feb 22, 2008 at 05:17:51PM +0000, Junio C Hamano wrote: > Pierre Habouzit 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 ``` ## Pierre Habouzit, 2008-02-23 14:09 Subject: [PATCH] parse-opt: bring PARSE_OPT_HIDDEN to git-rev-parse --parseopt Message-ID: <20080223140926.GD10967@artemis.madism.org> URL: https://gitlist.dev/e/20080223140926.GD10967%40artemis.madism.org In-Reply-To: <20080222181927.GA4555@artemis.madism.org> ``` Signed-off-by: Pierre Habouzit --- 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: ------------ -? SP+ help LF +? SP+ help LF ------------ ``:: @@ -334,10 +334,15 @@ Each line of options has this format: is necessary. `h,help`, `dry-run` and `f` are all three correct ``. -``:: - an `` tells the option parser if the option has an argument - (`=`), an optional one (`?` though its use is discouraged) or none - (no `` in that case). +``:: + `` 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 ``` ## Junio C Hamano, 2008-02-24 08:42 Subject: Re: [PATCH] parse-opt: bring PARSE_OPT_HIDDEN to git-rev-parse --parseopt Message-ID: <7vbq66ycct.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vbq66ycct.fsf%40gitster.siamese.dyndns.org In-Reply-To: <20080223140926.GD10967@artemis.madism.org> ``` Pierre Habouzit 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 ^=. ``` ## Pierre Habouzit, 2008-02-24 08:49 Subject: Re: [PATCH] parse-opt: bring PARSE_OPT_HIDDEN to git-rev-parse --parseopt Message-ID: <20080224084953.GA7623@artemis.madism.org> URL: https://gitlist.dev/e/20080224084953.GA7623%40artemis.madism.org In-Reply-To: <7vbq66ycct.fsf@gitster.siamese.dyndns.org> ``` On Sun, Feb 24, 2008 at 08:42:26AM +0000, Junio C Hamano wrote: > Pierre Habouzit 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 ```