{"thread":{"id":"12250","subject":"[PATCH] Improve documentation for git-sh-setup.","startedAt":"2008-02-21T23:01:28Z","lastAt":"2008-02-24T08:49:58Z","messageCount":7,"participants":["Yann Dirson","Pierre Habouzit","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"69532","messageId":"20080221230101.27644.49229.stgit@gandelf.nowhere.earth","threadId":"12250","inReplyTo":null,"subject":"[PATCH] Improve documentation for git-sh-setup.","fromName":"Yann Dirson","fromEmail":"ydirson@altern.org","sentAt":"2008-02-21T23:01:28Z","receivedAt":"2008-02-21T23:01:28Z","isPatch":true,"sender":{"key":"ydirson@altern.org","avatar":"https://avatars.githubusercontent.com/u/1190950?v=4"},"body":"Signed-off-by: Yann Dirson \nCc: Pierre Habouzit \n---\n\n Documentation/git-sh-setup.txt | 23 ++++++++++++++++++-----\n 1 files changed, 18 insertions(+), 5 deletions(-)\n\n\ndiff --git a/Documentation/git-sh-setup.txt b/Documentation/git-sh-setup.txt\nindex 16b8b75..75e6846 100644\n--- a/Documentation/git-sh-setup.txt\n+++ b/Documentation/git-sh-setup.txt\n@@ -19,12 +19,25 @@ Porcelain-ish scripts and/or are writing new ones.\n The `git-sh-setup` scriptlet is designed to be sourced (using\n `.`) by other shell scripts to set up some variables pointing at\n the normal git directories and a few helper shell functions.\n+It also provides help messages, and some amount of command-line\n+parsing.\n \n-Before sourcing it, your script should set up a few variables;\n-`USAGE` (and `LONG_USAGE`, if any) is used to define message\n-given by `usage()` shell function. `SUBDIRECTORY_OK` can be set\n-if the script can run from a subdirectory of the working tree\n-(some commands do not).\n+Before sourcing it, your script should set up a few variables.\n+\n+`OPTIONS_SPEC` allows to declare the options so that the command-line\n+can be pre-processed, and the usage message autogenerated. It must\n+follow the rules which apply to arguments to `git rev-parse\n+--parseopt`. If you need `git-rev-parse --parseopt` to keep the `--`\n+the user may have passed to your command, you should also set\n+`OPTIONS_KEEPDASHDASH` to a non empty value.\n+\n+If the constraints imposed by the use of `git rev-parse` do not fit\n+for a particular tool, `USAGE` (and `LONG_USAGE`, if any) can be used\n+to define message given by `usage()` shell function; no command-line\n+pre-processing occurs, the script has entire control over it.\n+\n+`SUBDIRECTORY_OK` can be set if the script can run from a subdirectory\n+of the working tree (some commands do not).\n \n The scriptlet sets `GIT_DIR` and `GIT_OBJECT_DIRECTORY` shell\n variables, but does *not* export them to the environment.\n"},{"id":"69567","messageId":"20080222093558.GC29114@artemis.madism.org","threadId":"12250","inReplyTo":"20080221230101.27644.49229.stgit@gandelf.nowhere.earth","subject":"Re: [PATCH] Improve documentation for git-sh-setup.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-02-22T09:35:58Z","receivedAt":"2008-02-22T09:35:58Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Thu, Feb 21, 2008 at 11:01:28PM +0000, Yann Dirson wrote:\n> Signed-off-by: Yann Dirson \n> Cc: Pierre Habouzit \n\n\n> +If the constraints imposed by the use of `git rev-parse` do not fit\n> +for a particular tool, `USAGE` (and `LONG_USAGE`, if any) can be used\n> +to define message given by `usage()` shell function; no command-line\n> +pre-processing occurs, the script has entire control over it.\n\n Actually, new scripts should be written using git rev-parse if\npossible, the USAGE/LONG_USAGE were there _before_ and I'd like to call\nthem the deprecated interface if other git hackers don't mind.\n\n git rev-parse --parseopt gives consistency in how git parses options,\nand it's A Good Thing™\n\n That makes me think that git-sh-setup(1) use should be documented in\ngitcli(5), maybe even included, and git-sh-setup(1) should *definitely*\nlink to gitcli(5). We wondered where to link gitcli from, this is\ndefinitely the place !\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"69594","messageId":"7vbq68ap1c.fsf@gitster.siamese.dyndns.org","threadId":"12250","inReplyTo":"20080222093558.GC29114@artemis.madism.org","subject":"Re: [PATCH] Improve documentation for git-sh-setup.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-22T17:17:51Z","receivedAt":"2008-02-22T17:17:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pierre Habouzit writes:\n\n> Actually, new scripts should be written using git rev-parse if\n> possible, the USAGE/LONG_USAGE were there _before_ and I'd like to call\n> them the deprecated interface if other git hackers don't mind.\n>\n> git rev-parse --parseopt gives consistency in how git parses options,\n> and it's A Good Thing™\n\nI certainly think encouraging use of parse-options in new\nscripts is a good idea, even though I suspect \"deprecated\" above\nmight be a bit too strong.\n\nI did not find accessible from the command line variant was the\nparse-opt-hidden feature, which was frustrating.\n\n> That makes me think that git-sh-setup(1) use should be documented in\n> gitcli(5), maybe even included, and git-sh-setup(1) should *definitely*\n> link to gitcli(5). We wondered where to link gitcli from, this is\n> definitely the place !\n\nSounds good. Please make it so.\n"},{"id":"69601","messageId":"20080222181927.GA4555@artemis.madism.org","threadId":"12250","inReplyTo":"7vbq68ap1c.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Improve documentation for git-sh-setup.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-02-22T18:19:28Z","receivedAt":"2008-02-22T18:19:28Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Fri, Feb 22, 2008 at 05:17:51PM +0000, Junio C Hamano wrote:\n> Pierre Habouzit writes:\n> \n> > Actually, new scripts should be written using git rev-parse if\n> > possible, the USAGE/LONG_USAGE were there _before_ and I'd like to call\n> > them the deprecated interface if other git hackers don't mind.\n> >\n> > git rev-parse --parseopt gives consistency in how git parses options,\n> > and it's A Good Thing™\n> \n> I certainly think encouraging use of parse-options in new\n> scripts is a good idea, even though I suspect \"deprecated\" above\n> might be a bit too strong.\n\n Okay.\n\n> I did not find accessible from the command line variant was the\n> parse-opt-hidden feature, which was frustrating.\n\n Well, it's probably doable using some more magic flags, I can hack\nsomething if you need to, I'll try to work something out during FOSDEM's\ntalks :)\n\n> > That makes me think that git-sh-setup(1) use should be documented in\n> > gitcli(5), maybe even included, and git-sh-setup(1) should *definitely*\n> > link to gitcli(5). We wondered where to link gitcli from, this is\n> > definitely the place !\n> \n> Sounds good. Please make it so.\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"69684","messageId":"20080223140926.GD10967@artemis.madism.org","threadId":"12250","inReplyTo":"20080222181927.GA4555@artemis.madism.org","subject":"[PATCH] parse-opt: bring PARSE_OPT_HIDDEN to git-rev-parse --parseopt","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-02-23T14:09:26Z","receivedAt":"2008-02-23T14:09:26Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"Signed-off-by: Pierre Habouzit \n---\n On ven, fév 22, 2008 at 06:19:28 +0000, Pierre Habouzit wrote:\n > On Fri, Feb 22, 2008 at 05:17:51PM +0000, Junio C Hamano wrote:\n > > I did not find accessible from the command line variant was the\n > > parse-opt-hidden feature, which was frustrating.\n > \n > Well, it's probably doable using some more magic flags, I can hack\n > something if you need to, I'll try to work something out during FOSDEM's\n > talks :)\n\n Here it is …\n I also made the parsing more extensible wrt new flags if needed.\n\n We should definitely write tests too.\n\n Documentation/git-rev-parse.txt | 15 ++++++++++-----\n builtin-rev-parse.c | 25 ++++++++++++++-----------\n 2 files changed, 24 insertions(+), 16 deletions(-)\n\ndiff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt\nindex f02f6bb..e961c20 100644\n--- a/Documentation/git-rev-parse.txt\n+++ b/Documentation/git-rev-parse.txt\n@@ -325,7 +325,7 @@ The lines after the separator describe the options.\n Each line of options has this format:\n \n ------------\n-? SP+ help LF\n+? SP+ help LF\n ------------\n \n ``::\n@@ -334,10 +334,15 @@ Each line of options has this format:\n \tis necessary. `h,help`, `dry-run` and `f` are all three correct\n \t``.\n \n-``::\n-\tan `` tells the option parser if the option has an argument\n-\t(`=`), an optional one (`?` though its use is discouraged) or none\n-\t(no `` in that case).\n+``::\n+\t`` are any suite of `*`, `=` or `?`.\n+\t* Use `=` if the option take an argument.\n+\n+\t* Use `?` to mean that the option is optional (though its use is discouraged).\n+\n+\t* Use `*` to mean that this option should not be listed in the usage\n+\t generated for the `-h` argument. It's shown for `--help-all` as\n+\t documented in linkgit:gitcli[5].\n \n The remainder of the line, after stripping the spaces, is used\n as the help associated to the option.\ndiff --git a/builtin-rev-parse.c b/builtin-rev-parse.c\nindex b9af1a5..5ffc4e0 100644\n--- a/builtin-rev-parse.c\n+++ b/builtin-rev-parse.c\n@@ -322,18 +322,21 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)\n \t\to->type = OPTION_CALLBACK;\n \t\to->help = xstrdup(skipspaces(s));\n \t\to->value = &parsed;\n+\t\to->flags = PARSE_OPT_NOARG;\n \t\to->callback = &parseopt_dump;\n-\t\tswitch (s[-1]) {\n-\t\tcase '=':\n-\t\t\ts--;\n-\t\t\tbreak;\n-\t\tcase '?':\n-\t\t\to->flags = PARSE_OPT_OPTARG;\n-\t\t\ts--;\n-\t\t\tbreak;\n-\t\tdefault:\n-\t\t\to->flags = PARSE_OPT_NOARG;\n-\t\t\tbreak;\n+\t\twhile (s > sb.buf && strchr(\"*=?\", s[-1])) {\n+\t\t\tswitch (*--s) {\n+\t\t\tcase '=':\n+\t\t\t\to->flags &= ~PARSE_OPT_NOARG;\n+\t\t\t\tbreak;\n+\t\t\tcase '?':\n+\t\t\t\to->flags &= ~PARSE_OPT_NOARG;\n+\t\t\t\to->flags |= PARSE_OPT_OPTARG;\n+\t\t\t\tbreak;\n+\t\t\tcase '*':\n+\t\t\t\to->flags &= PARSE_OPT_HIDDEN;\n+\t\t\t\tbreak;\n+\t\t\t}\n \t\t}\n \n \t\tif (s - sb.buf == 1) /* short option only */\n-- \n1.5.4.2.281.g28d0e\n\n"},{"id":"69769","messageId":"7vbq66ycct.fsf@gitster.siamese.dyndns.org","threadId":"12250","inReplyTo":"20080223140926.GD10967@artemis.madism.org","subject":"Re: [PATCH] parse-opt: bring PARSE_OPT_HIDDEN to git-rev-parse --parseopt","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-24T08:42:26Z","receivedAt":"2008-02-24T08:42:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pierre Habouzit writes:\n\n> I also made the parsing more extensible wrt new flags if needed.\n>\n> We should definitely write tests too.\n> ...\n> +\t\t\tcase '*':\n> +\t\t\t\to->flags &= PARSE_OPT_HIDDEN;\n> +\t\t\t\tbreak;\n\nI have a slight suspicion that this is not what you meant.\nYou probably meant either |= or perhaps ^=.\n"},{"id":"69770","messageId":"20080224084953.GA7623@artemis.madism.org","threadId":"12250","inReplyTo":"7vbq66ycct.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] parse-opt: bring PARSE_OPT_HIDDEN to git-rev-parse --parseopt","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-02-24T08:49:58Z","receivedAt":"2008-02-24T08:49:58Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Sun, Feb 24, 2008 at 08:42:26AM +0000, Junio C Hamano wrote:\n> Pierre Habouzit writes:\n> \n> > I also made the parsing more extensible wrt new flags if needed.\n> >\n> > We should definitely write tests too.\n> > ...\n> > +\t\t\tcase '*':\n> > +\t\t\t\to->flags &= PARSE_OPT_HIDDEN;\n> > +\t\t\t\tbreak;\n> \n> I have a slight suspicion that this is not what you meant.\n> You probably meant either |= or perhaps ^=.\n\n oooh boy of course it's a |=...\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"}]}