ph/parseopt-sh reloaded

ph/parseopt-sh reloaded

[Pierre Habouzit]

Okay here is a fixed series wrt the security issue Junio raised.
Instead of the old PARSEOPT_OPTS variable, I now only have
OPTIONS_KEEPDASHDASH to be set to non empty if you want to add
--keep-dashdash. The reason for that is that I dislike that every single
git-rev-parse --parseopt user had to do a `PASREOPT_OPTS=` at the
begining of each script, it's error prone, and ugly.

PARSEOPT_OPTS was an overkill as it wasn't really used for anything else
than OPTIONS_KEEPDASHDASH, and if it has to be used for more, it'll be
easy to extend the specification parser to take options on stdin rather
than through parameters.

I also removed the PARSEOPT_OPTS from git-clone.sh as it was a spurious
use, I don't intend users to override this variable, it's indeed an
internal that changes git-rev-parse --parseopt behaviour in a
incompatible way for the scripts that uses it, it should not be
user-tweakable anyway.

The 10 patch series (and not 11, I forgot about 7 when I incrementally
sent the previous one) is fetcheable from my repository:

  git://git.madism.org/git.git on branch ph/parseopt-sh

(ph/parseopt has the remaining patches that are problematic right now
either because of the small change -h vs. -H or the patches that
conflicts with git-fetch series right now, but in the spirit this is
definitely a ph/parseopt series).

Cheers,

[PATCH 01/10] Add a parseopt mode to git-rev-parse to bring parse-options to shell scripts.

[Pierre Habouzit]

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 Documentation/git-rev-parse.txt |   75 ++++++++++++++++++++++-
 builtin-rev-parse.c             |  126 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 199 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 4758c33..6811656 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -23,6 +23,13 @@ distinguish between them.
 
 OPTIONS
 -------
+--parseopt::
+        Use `git-rev-parse` in option parsing mode (see PARSEOPT section below).
+
+--keep-dash-dash::
+	Only meaningful in `--parseopt` mode. Tells the option parser to echo
+        out the first `--` met instead of skipping it.
+
 --revs-only::
 	Do not output flags and parameters not meant for
 	`git-rev-list` command.
@@ -288,10 +295,74 @@ Here are a handful examples:
    C^@              I J F
    F^! D            G H D F
 
+PARSEOPT
+--------
+
+In `--parseopt` mode, `git-rev-parse` helps massaging options to bring to shell
+scripts the same facilities C builtins have. It works as an option normalizer
+(e.g. splits single switches aggregate values), a bit like `getopt(1)` does.
+
+It takes on the standard input the specification of the options to parse and
+understand, and echoes on the standard ouput a line suitable for `sh(1)` `eval`
+to replace the arguments with normalized ones.  In case of error, it ouputs
+usage on the standard error stream, and exits with code 129.
+
+Input Format
+~~~~~~~~~~~~
+
+`git-ref-parse --parseopt` input format is fully text based. It has two parts,
+separated by a line that contains only `--`. The lines before (should be more
+than one) are used for the usage. The lines after describe the options.
+
+Each line of options has this format:
+
+------------
+<opt_spec><arg_spec>? SP+ help LF
+------------
+
+`<opt_spec>`::
+	its format is the short option character, then the long option name
+        separated by a comma. Both parts are not required, though at least one
+        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 optionnal one (`?` though its use is discouraged) or none
+        (no `<arg_spec>` in that case).
+
+The rest of the line after as many spaces up to the ending line feed is used
+as the help associated to the option.
+
+Blank lines are ignored, and lines that don't match this specification are used
+as option group headers (start the line with a space to purposely create such
+lines).
+
+Example
+~~~~~~~
+
+------------
+OPTS_SPEC="\
+some-command [options] <args>...
+
+some-command does foo and bar !
+--
+h,help    show the help
+
+foo       some nifty option --foo
+bar=      some cool option --bar with an argument
+
+  An option group Header
+C?        option C with an optionnal argument"
+
+eval `echo "$OPTS_SPEC" | git-rev-parse --parseopt -- "$@" || echo exit $?`
+------------
+
+
 Author
 ------
-Written by Linus Torvalds <torvalds@osdl.org> and
-Junio C Hamano <junkio@cox.net>
+Written by Linus Torvalds <torvalds@osdl.org> .
+Junio C Hamano <junkio@cox.net> and Pierre Habouzit <madcoder@debian.org>
 
 Documentation
 --------------
diff --git a/builtin-rev-parse.c b/builtin-rev-parse.c
index 8d78b69..054519b 100644
--- a/builtin-rev-parse.c
+++ b/builtin-rev-parse.c
@@ -8,6 +8,7 @@
 #include "refs.h"
 #include "quote.h"
 #include "builtin.h"
+#include "parse-options.h"
 
 #define DO_REVS		1
 #define DO_NOREV	2
@@ -209,6 +210,128 @@ static int try_difference(const char *arg)
 	return 0;
 }
 
+static int parseopt_dump(const struct option *o, const char *arg, int unset)
+{
+	struct strbuf *parsed = o->value;
+	if (unset)
+		strbuf_addf(parsed, " --no-%s", o->long_name);
+	else if (o->short_name)
+		strbuf_addf(parsed, " -%c", o->short_name);
+	else
+		strbuf_addf(parsed, " --%s", o->long_name);
+	if (arg) {
+		strbuf_addch(parsed, ' ');
+		sq_quote_buf(parsed, arg);
+	}
+	return 0;
+}
+
+static const char *skipspaces(const char *s)
+{
+	while (isspace(*s))
+		s++;
+	return s;
+}
+
+static int cmd_parseopt(int argc, const char **argv, const char *prefix)
+{
+	static int keep_dashdash = 0;
+	static char const * const parseopt_usage[] = {
+		"git-rev-parse --parseopt [options] -- [<args>...]",
+		NULL
+	};
+	static struct option parseopt_opts[] = {
+		OPT_BOOLEAN(0, "keep-dashdash", &keep_dashdash,
+					"keep the `--` passed as an arg"),
+		OPT_END(),
+	};
+
+	struct strbuf sb, parsed;
+	const char **usage = NULL;
+	struct option *opts = NULL;
+	int onb = 0, osz = 0, unb = 0, usz = 0;
+
+	strbuf_init(&parsed, 0);
+	strbuf_addstr(&parsed, "set --");
+	argc = parse_options(argc, argv, parseopt_opts, parseopt_usage,
+	                     PARSE_OPT_KEEP_DASHDASH);
+	if (argc < 1 || strcmp(argv[0], "--"))
+		usage_with_options(parseopt_usage, parseopt_opts);
+
+	strbuf_init(&sb, 0);
+	/* get the usage up to the first line with a -- on it */
+	for (;;) {
+		if (strbuf_getline(&sb, stdin, '\n') == EOF)
+			die("premature end of input");
+		ALLOC_GROW(usage, unb + 1, usz);
+		if (!strcmp("--", sb.buf)) {
+			if (unb < 1)
+				die("no usage string given before the `--' separator");
+			usage[unb] = NULL;
+			break;
+		}
+		usage[unb++] = strbuf_detach(&sb, NULL);
+	}
+
+	/* parse: (<short>|<short>,<long>|<long>)[=?]? SP+ <help> */
+	while (strbuf_getline(&sb, stdin, '\n') != EOF) {
+		const char *s;
+		struct option *o;
+
+		if (!sb.len)
+			continue;
+
+		ALLOC_GROW(opts, onb + 1, osz);
+		memset(opts + onb, 0, sizeof(opts[onb]));
+
+		o = &opts[onb++];
+		s = strchr(sb.buf, ' ');
+		if (!s || *sb.buf == ' ') {
+			o->type = OPTION_GROUP;
+			o->help = xstrdup(skipspaces(s));
+			continue;
+		}
+
+		o->type = OPTION_CALLBACK;
+		o->help = xstrdup(skipspaces(s));
+		o->value = &parsed;
+		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;
+		}
+
+		if (s - sb.buf == 1) /* short option only */
+			o->short_name = *sb.buf;
+		else if (sb.buf[1] != ',') /* long option only */
+			o->long_name = xmemdupz(sb.buf, s - sb.buf);
+		else {
+			o->short_name = *sb.buf;
+			o->long_name = xmemdupz(sb.buf + 2, s - sb.buf - 2);
+		}
+	}
+	strbuf_release(&sb);
+
+	/* put an OPT_END() */
+	ALLOC_GROW(opts, onb + 1, osz);
+	memset(opts + onb, 0, sizeof(opts[onb]));
+	argc = parse_options(argc, argv, opts, usage,
+	                     keep_dashdash ? PARSE_OPT_KEEP_DASHDASH : 0);
+
+	strbuf_addf(&parsed, " --");
+	sq_quote_argv(&parsed, argv, argc, 0);
+	puts(parsed.buf);
+	return 0;
+}
+
 int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 {
 	int i, as_is = 0, verify = 0;
@@ -216,6 +339,9 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 
 	git_config(git_default_config);
 
+	if (argc > 1 && !strcmp("--parseopt", argv[1]))
+		return cmd_parseopt(argc - 1, argv + 1, prefix);
+
 	for (i = 1; i < argc; i++) {
 		const char *arg = argv[i];
 
-- 
1.5.3.5.1509.g66d41

[PATCH 02/10] Update git-sh-setup(1) to allow transparent use of git-rev-parse --parseopt

[Pierre Habouzit]

If you set OPTIONS_SPEC, git-sh-setups uses git-rev-parse --parseopt
automatically.

It also diverts usage to re-exec $0 with the -h option as parse-options.c
will catch that.

If you need git-rev-parse --parseopt to keep the `--` the user may have
passed to your command, just set OPTIONS_KEEPDASHDASH to a non empty value.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 git-sh-setup.sh |   48 ++++++++++++++++++++++++++++++------------------
 1 files changed, 30 insertions(+), 18 deletions(-)

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 86d7d4c..e1cf885 100755
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -16,9 +16,36 @@ die() {
 	exit 1
 }
 
-usage() {
-	die "Usage: $0 $USAGE"
-}
+if test -n "$OPTIONS_SPEC"; then
+	usage() {
+		exec "$0" -h
+	}
+
+	parseopt_extra=
+	[ -n "$OPTIONS_KEEPDASHDASH" ] &&
+		parseopt_extra="$parseopt_extra --keep-dashdash"
+
+	eval `echo "$OPTIONS_SPEC" | git rev-parse --parseopt $parseopt_extra -- "$@" || echo exit $?`
+else
+	usage() {
+		die "Usage: $0 $USAGE"
+	}
+
+	if [ -z "$LONG_USAGE" ]
+	then
+		LONG_USAGE="Usage: $0 $USAGE"
+	else
+		LONG_USAGE="Usage: $0 $USAGE
+
+$LONG_USAGE"
+	fi
+
+	case "$1" in
+		-h|--h|--he|--hel|--help)
+		echo "$LONG_USAGE"
+		exit
+	esac
+fi
 
 set_reflog_action() {
 	if [ -z "${GIT_REFLOG_ACTION:+set}" ]
@@ -91,21 +118,6 @@ get_author_ident_from_commit () {
 	LANG=C LC_ALL=C sed -ne "$pick_author_script"
 }
 
-if [ -z "$LONG_USAGE" ]
-then
-	LONG_USAGE="Usage: $0 $USAGE"
-else
-	LONG_USAGE="Usage: $0 $USAGE
-
-$LONG_USAGE"
-fi
-
-case "$1" in
-	-h|--h|--he|--hel|--help)
-	echo "$LONG_USAGE"
-	exit
-esac
-
 # Make sure we are in a valid repository of a vintage we understand.
 if [ -z "$SUBDIRECTORY_OK" ]
 then
-- 
1.5.3.5.1509.g66d41

[PATCH 03/10] Migrate git-clean.sh to use git-rev-parse --parseopt.

[Pierre Habouzit]

Also minor consistency tweaks in how errors are caught.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 git-clean.sh |   38 ++++++++++++++++++++------------------
 1 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/git-clean.sh b/git-clean.sh
index 4491738..6959433 100755
--- a/git-clean.sh
+++ b/git-clean.sh
@@ -3,16 +3,21 @@
 # Copyright (c) 2005-2006 Pavel Roskin
 #
 
-USAGE="[-d] [-f] [-n] [-q] [-x | -X] [--] <paths>..."
-LONG_USAGE='Clean untracked files from the working directory
-	-d	remove directories as well
-	-f	override clean.requireForce and clean anyway
-	-n 	don'\''t remove anything, just show what would be done
-	-q	be quiet, only report errors
-	-x	remove ignored files as well
-	-X	remove only ignored files
+OPTIONS_SPEC="\
+git-clean [options] <paths>...
+
+Clean untracked files from the working directory
+
 When optional <paths>... arguments are given, the paths
-affected are further limited to those that match them.'
+affected are further limited to those that match them.
+--
+d remove directories as well
+f override clean.requireForce and clean anyway
+n don't remove anything, just show what would be done
+q be quiet, only report errors
+x remove ignored files as well
+X remove only ignored files"
+
 SUBDIRECTORY_OK=Yes
 . git-sh-setup
 require_work_tree
@@ -55,23 +60,20 @@ do
 		shift
 		break
 		;;
-	-*)
-		usage
-		;;
 	*)
-		break
+		usage # should not happen
+		;;
 	esac
 	shift
 done
 
 if [ "$disabled" = true ]; then
-	echo "clean.requireForce set and -n or -f not given; refusing to clean"
-	exit 1
+	die "clean.requireForce set and -n or -f not given; refusing to clean"
 fi
 
-case "$ignored,$ignoredonly" in
-	1,1) usage;;
-esac
+if [ "$ignored,$ignoredonly" = "1,1" ]; then
+	die "-x and -X cannot be set together"
+fi
 
 if [ -z "$ignored" ]; then
 	excl="--exclude-per-directory=.gitignore"
-- 
1.5.3.5.1509.g66d41

[PATCH 04/10] Migrate git-clone to use git-rev-parse --parseopt

[Pierre Habouzit]

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 git-clone.sh |  102 +++++++++++++++++++++++++++++++++-------------------------
 1 files changed, 58 insertions(+), 44 deletions(-)

diff --git a/git-clone.sh b/git-clone.sh
index 0ea3c24..52c5601 100755
--- a/git-clone.sh
+++ b/git-clone.sh
@@ -8,15 +8,36 @@
 # See git-sh-setup why.
 unset CDPATH
 
+OPTIONS_SPEC="\
+git-clone [options] <repo> [<dir>]
+--
+n,no-checkout        don't create a checkout
+bare                 create a bare repository
+naked                create a bare repository
+l,local              to clone from a local repository
+no-hardlinks         don't use local hardlinks, always copy
+s,shared             setup as a shared repository
+template=            path to the template directory
+q,quiet              be quiet
+reference=           reference repository
+o,origin=            use <name> instead of 'origin' to track upstream
+u,upload-pack=       path to git-upload-pack on the remote
+depth=               create a shallow clone of that depth
+
+use-separate-remote  compatibility, do not use
+no-separate-remote   compatibility, do not use"
+
 die() {
 	echo >&2 "$@"
 	exit 1
 }
 
 usage() {
-	die "Usage: $0 [--template=<template_directory>] [--reference <reference-repo>] [--bare] [-l [-s]] [-q] [-u <upload-pack>] [--origin <name>] [--depth <n>] [-n] <repo> [<dir>]"
+	exec "$0" -h
 }
 
+eval `echo "$OPTIONS_SPEC" | git rev-parse --parseopt -- "$@" || echo exit $?`
+
 get_repo_base() {
 	(
 		cd "`/bin/pwd`" &&
@@ -106,64 +127,57 @@ depth=
 no_progress=
 local_explicitly_asked_for=
 test -t 1 || no_progress=--no-progress
-while
-	case "$#,$1" in
-	0,*) break ;;
-	*,-n|*,--no|*,--no-|*,--no-c|*,--no-ch|*,--no-che|*,--no-chec|\
-	*,--no-check|*,--no-checko|*,--no-checkou|*,--no-checkout)
-	  no_checkout=yes ;;
-	*,--na|*,--nak|*,--nake|*,--naked|\
-	*,-b|*,--b|*,--ba|*,--bar|*,--bare) bare=yes ;;
-	*,-l|*,--l|*,--lo|*,--loc|*,--loca|*,--local)
-	  local_explicitly_asked_for=yes
-	  use_local_hardlink=yes ;;
-	*,--no-h|*,--no-ha|*,--no-har|*,--no-hard|*,--no-hardl|\
-	*,--no-hardli|*,--no-hardlin|*,--no-hardlink|*,--no-hardlinks)
-	  use_local_hardlink=no ;;
-        *,-s|*,--s|*,--sh|*,--sha|*,--shar|*,--share|*,--shared)
-          local_shared=yes; ;;
-	1,--template) usage ;;
-	*,--template)
+
+while test $# != 0
+do
+	case "$1" in
+	-n|--no-checkout)
+		no_checkout=yes ;;
+	--naked|--bare)
+		bare=yes ;;
+	-l|--local)
+		local_explicitly_asked_for=yes
+		use_local_hardlink=yes
+		;;
+	--no-hardlinks)
+		use_local_hardlink=no ;;
+	-s|--shared)
+		local_shared=yes ;;
+	--template)
 		shift; template="--template=$1" ;;
-	*,--template=*)
-	  template="$1" ;;
-	*,-q|*,--quiet) quiet=-q ;;
-	*,--use-separate-remote) ;;
-	*,--no-separate-remote)
+	-q|--quiet)
+		quiet=-q ;;
+	--use-separate-remote|--no-separate-remote)
 		die "clones are always made with separate-remote layout" ;;
-	1,--reference) usage ;;
-	*,--reference)
+	--reference)
 		shift; reference="$1" ;;
-	*,--reference=*)
-		reference=`expr "z$1" : 'z--reference=\(.*\)'` ;;
-	*,-o|*,--or|*,--ori|*,--orig|*,--origi|*,--origin)
-		case "$2" in
+	-o,--origin)
+		shift;
+		case "$1" in
 		'')
 		    usage ;;
 		*/*)
-		    die "'$2' is not suitable for an origin name"
+		    die "'$1' is not suitable for an origin name"
 		esac
-		git check-ref-format "heads/$2" ||
-		    die "'$2' is not suitable for a branch name"
+		git check-ref-format "heads/$1" ||
+		    die "'$1' is not suitable for a branch name"
 		test -z "$origin_override" ||
 		    die "Do not give more than one --origin options."
 		origin_override=yes
-		origin="$2"; shift
+		origin="$1"
 		;;
-	1,-u|1,--upload-pack) usage ;;
-	*,-u|*,--upload-pack)
+	-u|--upload-pack)
 		shift
 		upload_pack="--upload-pack=$1" ;;
-	*,--upload-pack=*)
-		upload_pack=--upload-pack=$(expr "z$1" : 'z-[^=]*=\(.*\)') ;;
-	1,--depth) usage;;
-	*,--depth)
+	--depth)
+		shift
+		depth="--depth=$1" ;;
+	--)
 		shift
-		depth="--depth=$1";;
-	*,-*) usage ;;
-	*) break ;;
+		break ;;
+	*)
+		usage ;;
 	esac
-do
 	shift
 done
 
-- 
1.5.3.5.1509.g66d41

[PATCH 05/10] Migrate git-am.sh to use git-rev-parse --parseopt

[Pierre Habouzit]

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 git-am.sh |   93 +++++++++++++++++++++++++++++++-----------------------------
 1 files changed, 48 insertions(+), 45 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index 2514d07..2d2b1c6 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -2,11 +2,25 @@
 #
 # Copyright (c) 2005, 2006 Junio C Hamano
 
-USAGE='[--signoff] [--dotest=<dir>] [--keep] [--utf8 | --no-utf8]
-  [--3way] [--interactive] [--binary]
-  [--whitespace=<option>] [-C<n>] [-p<n>]
-  <mbox>|<Maildir>...
-  or, when resuming [--skip | --resolved]'
+OPTIONS_SPEC="\
+git-am [options] <mbox>|<Maildir>...
+git-am [options] --resolved
+git-am [options] --skip
+--
+d,dotest=       use <dir> and not .dotest
+i,interactive=  run interactively
+b,binary        pass --allo-binary-replacement to git-apply
+3,3way          allow fall back on 3way merging if needed
+s,signoff       add a Signed-off-by line to the commit message
+u,utf8          recode into utf8 (default)
+k,keep          pass -k flagg to git-mailinfo
+whitespace=     pass it through git-apply
+C=              pass it through git-apply
+p=              pass it through git-apply
+resolvemsg=     override error message when patch failure occurs
+r,resolved      to be used after a patch failure
+skip            skip the current patch"
+
 . git-sh-setup
 set_reflog_action am
 require_work_tree
@@ -110,49 +124,38 @@ git_apply_opt=
 while test $# != 0
 do
 	case "$1" in
-	-d=*|--d=*|--do=*|--dot=*|--dote=*|--dotes=*|--dotest=*)
-	dotest=`expr "z$1" : 'z-[^=]*=\(.*\)'`; shift ;;
-	-d|--d|--do|--dot|--dote|--dotes|--dotest)
-	case "$#" in 1) usage ;; esac; shift
-	dotest="$1"; shift;;
-
-	-i|--i|--in|--int|--inte|--inter|--intera|--interac|--interact|\
-	--interacti|--interactiv|--interactive)
-	interactive=t; shift ;;
-
-	-b|--b|--bi|--bin|--bina|--binar|--binary)
-	binary=t; shift ;;
-
-	-3|--3|--3w|--3wa|--3way)
-	threeway=t; shift ;;
-	-s|--s|--si|--sig|--sign|--signo|--signof|--signoff)
-	sign=t; shift ;;
-	-u|--u|--ut|--utf|--utf8)
-	utf8=t; shift ;; # this is now default
-	--no-u|--no-ut|--no-utf|--no-utf8)
-	utf8=; shift ;;
-	-k|--k|--ke|--kee|--keep)
-	keep=t; shift ;;
-
-	-r|--r|--re|--res|--reso|--resol|--resolv|--resolve|--resolved)
-	resolved=t; shift ;;
-
-	--sk|--ski|--skip)
-	skip=t; shift ;;
-
-	--whitespace=*|-C*|-p*)
-	git_apply_opt="$git_apply_opt $1"; shift ;;
-
-	--resolvemsg=*)
-	resolvemsg=${1#--resolvemsg=}; shift ;;
-
+	-i|--interactive)
+		interactive=t ;;
+	-b|--binary)
+		binary=t ;;
+	-3|--3way)
+		threeway=t ;;
+	-s--signoff)
+		sign=t ;;
+	-u|--utf8)
+		utf8=t ;; # this is now default
+	--no-utf8)
+		utf8= ;;
+	-k|--keep)
+		keep=t ;;
+	-r|--resolved)
+		resolved=t ;;
+	--skip)
+		skip=t ;;
+	-d|--dotest)
+		shift; dotest=$1;;
+	--resolvemsg)
+		shift; resolvemsg=$1 ;;
+	--whitespace)
+		git_apply_opt="$git_apply_opt $1=$2"; shift ;;
+	-C|-p)
+		git_apply_opt="$git_apply_opt $1$2"; shift ;;
 	--)
-	shift; break ;;
-	-*)
-	usage ;;
+		shift; break ;;
 	*)
-	break ;;
+		usage ;;
 	esac
+	shift
 done
 
 # If the dotest directory exists, but we have finished applying all the
-- 
1.5.3.5.1509.g66d41

[PATCH 06/10] Migrate git-merge.sh to use git-rev-parse --parseopt

[Pierre Habouzit]

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 git-merge.sh |  125 ++++++++++++++++++++++++---------------------------------
 1 files changed, 53 insertions(+), 72 deletions(-)

diff --git a/git-merge.sh b/git-merge.sh
index b9f0519..d19bfc2 100755
--- a/git-merge.sh
+++ b/git-merge.sh
@@ -3,7 +3,18 @@
 # Copyright (c) 2005 Junio C Hamano
 #
 
-USAGE='[-n] [--summary] [--[no-]commit] [--[no-]squash] [--[no-]ff] [-s <strategy>] [-m=<merge-message>] <commit>+'
+OPTIONS_SPEC="\
+git-merge [options] <remote>...
+git-merge [options] <msg> HEAD <remote>
+--
+summary              show a diffstat at the end of the merge
+n,no-summary         don't show a diffstat at the end of the merge
+squash               create a single commit instead of doing a merge
+commit               perform a commit if the merge sucesses (default)
+ff                   allow fast forward (default)
+s,strategy=          merge strategy to use
+m,message=           message to be used for the merge commit (if any)
+"
 
 SUBDIRECTORY_OK=Yes
 . git-sh-setup
@@ -132,72 +143,47 @@ merge_name () {
 	fi
 }
 
-parse_option () {
-	case "$1" in
-	-n|--n|--no|--no-|--no-s|--no-su|--no-sum|--no-summ|\
-		--no-summa|--no-summar|--no-summary)
-		show_diffstat=false ;;
-	--summary)
-		show_diffstat=t ;;
-	--sq|--squ|--squa|--squas|--squash)
-		allow_fast_forward=t squash=t no_commit=t ;;
-	--no-sq|--no-squ|--no-squa|--no-squas|--no-squash)
-		allow_fast_forward=t squash= no_commit= ;;
-	--c|--co|--com|--comm|--commi|--commit)
-		allow_fast_forward=t squash= no_commit= ;;
-	--no-c|--no-co|--no-com|--no-comm|--no-commi|--no-commit)
-		allow_fast_forward=t squash= no_commit=t ;;
-	--ff)
-		allow_fast_forward=t squash= no_commit= ;;
-	--no-ff)
-		allow_fast_forward=false squash= no_commit= ;;
-	-s=*|--s=*|--st=*|--str=*|--stra=*|--strat=*|--strate=*|\
-		--strateg=*|--strategy=*|\
-	-s|--s|--st|--str|--stra|--strat|--strate|--strateg|--strategy)
-		case "$#,$1" in
-		*,*=*)
-			strategy=`expr "z$1" : 'z-[^=]*=\(.*\)'` ;;
-		1,*)
-			usage ;;
-		*)
-			strategy="$2"
-			shift ;;
-		esac
-		case " $all_strategies " in
-		*" $strategy "*)
-			use_strategies="$use_strategies$strategy " ;;
-		*)
-			die "available strategies are: $all_strategies" ;;
-		esac
-		;;
-	-m=*|--m=*|--me=*|--mes=*|--mess=*|--messa=*|--messag=*|--message=*)
-		merge_msg=`expr "z$1" : 'z-[^=]*=\(.*\)'`
-		have_message=t
-		;;
-	-m|--m|--me|--mes|--mess|--messa|--messag|--message)
-		shift
-		case "$#" in
-		1)	usage ;;
-		esac
-		merge_msg="$1"
-		have_message=t
-		;;
-	-*)	usage ;;
-	*)	return 1 ;;
-	esac
-	shift
-	args_left=$#
-}
-
 parse_config () {
-	while test $# -gt 0
-	do
-		parse_option "$@" || usage
-		while test $args_left -lt $#
-		do
+	while test $# != 0; do
+		case "$1" in
+		-n|--no-summary)
+			show_diffstat=false ;;
+		--summary)
+			show_diffstat=t ;;
+		--squash)
+			allow_fast_forward=t squash=t no_commit=t ;;
+		--no-squash)
+			allow_fast_forward=t squash= no_commit= ;;
+		--commit)
+			allow_fast_forward=t squash= no_commit= ;;
+		--no-commit)
+			allow_fast_forward=t squash= no_commit=t ;;
+		--ff)
+			allow_fast_forward=t squash= no_commit= ;;
+		--no-ff)
+			allow_fast_forward=false squash= no_commit= ;;
+		-s|--strategy)
+			shift
+			case " $all_strategies " in
+			*" $1 "*)
+				use_strategies="$use_strategies$1 " ;;
+			*)
+				die "available strategies are: $all_strategies" ;;
+			esac
+			;;
+		-m|--message)
 			shift
-		done
+			merge_msg="$1"
+			have_message=t
+			;;
+		--)
+			shift
+			break ;;
+		*)	usage ;;
+		esac
+		shift
 	done
+	args_left=$#
 }
 
 test $# != 0 || usage
@@ -209,17 +195,12 @@ then
 	mergeopts=$(git config "branch.${branch#refs/heads/}.mergeoptions")
 	if test -n "$mergeopts"
 	then
-		parse_config $mergeopts
+		parse_config $mergeopts --
 	fi
 fi
 
-while parse_option "$@"
-do
-	while test $args_left -lt $#
-	do
-		shift
-	done
-done
+parse_config "$@"
+while test $args_left -lt $#; do shift; done
 
 if test -z "$show_diffstat"; then
     test "$(git config --bool merge.diffstat)" = false && show_diffstat=false
-- 
1.5.3.5.1509.g66d41

[PATCH 07/10] Migrate git-instaweb.sh to use git-rev-parse --parseopt

[Pierre Habouzit]

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 git-instaweb.sh |   73 ++++++++++++++++++++++---------------------------------
 1 files changed, 29 insertions(+), 44 deletions(-)

diff --git a/git-instaweb.sh b/git-instaweb.sh
index 95c3e5a..d912bf5 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -2,9 +2,20 @@
 #
 # Copyright (c) 2006 Eric Wong
 #
-USAGE='[--start] [--stop] [--restart]
-  [--local] [--httpd=<httpd>] [--port=<port>] [--browser=<browser>]
-  [--module-path=<path> (for Apache2 only)]'
+
+OPTIONS_SPEC="\
+git-instaweb [options] (--start | --stop | --restart)
+--
+l,local        only bind on 127.0.0.1
+p,port=        the port to bind to
+d,httpd=       the command to launch
+b,browser=     the browser to launch
+m,module-path= the module path (only needed for apache2)
+ Action
+stop           stop the web server
+start          start the web server
+restart        restart the web server
+"
 
 . git-sh-setup
 
@@ -78,52 +89,26 @@ do
 		start_httpd
 		exit 0
 		;;
-	--local|-l)
+	-l|--local)
 		local=true
 		;;
-	-d|--httpd|--httpd=*)
-		case "$#,$1" in
-		*,*=*)
-			httpd=`expr "$1" : '-[^=]*=\(.*\)'` ;;
-		1,*)
-			usage ;;
-		*)
-			httpd="$2"
-			shift ;;
-		esac
+	-d|--httpd)
+		shift
+		httpd="$1"
+		;;
+	-b|--browser)
+		shift
+		browser="$1"
 		;;
-	-b|--browser|--browser=*)
-		case "$#,$1" in
-		*,*=*)
-			browser=`expr "$1" : '-[^=]*=\(.*\)'` ;;
-		1,*)
-			usage ;;
-		*)
-			browser="$2"
-			shift ;;
-		esac
+	-p|--port)
+		shift
+		port="$1"
 		;;
-	-p|--port|--port=*)
-		case "$#,$1" in
-		*,*=*)
-			port=`expr "$1" : '-[^=]*=\(.*\)'` ;;
-		1,*)
-			usage ;;
-		*)
-			port="$2"
-			shift ;;
-		esac
+	-m|--module-path)
+		shift
+		module_path="$1"
 		;;
-	-m|--module-path=*|--module-path)
-		case "$#,$1" in
-		*,*=*)
-			module_path=`expr "$1" : '-[^=]*=\(.*\)'` ;;
-		1,*)
-			usage ;;
-		*)
-			module_path="$2"
-			shift ;;
-		esac
+	--)
 		;;
 	*)
 		usage
-- 
1.5.3.5.1509.g66d41

[PATCH 08/10] Migrate git-checkout.sh to use git-rev-parse --parseopt --keep-dashdash

[Pierre Habouzit]

Also fix some space versus tabs issues.
---
 git-checkout.sh |   99 +++++++++++++++++++++++++++----------------------------
 1 files changed, 49 insertions(+), 50 deletions(-)

diff --git a/git-checkout.sh b/git-checkout.sh
index 8993920..c00cedd 100755
--- a/git-checkout.sh
+++ b/git-checkout.sh
@@ -1,6 +1,16 @@
 #!/bin/sh
 
-USAGE='[-q] [-f] [-b <new_branch>] [-m] [<branch>] [<paths>...]'
+OPTIONS_KEEPDASHDASH=t
+OPTIONS_SPEC="\
+git-branch [options] [<branch>] [<paths>...]
+--
+b=          create a new branch started at <branch>
+l           create the new branchs reflog
+track       tells if the new branch should track the remote branch
+f           proceed even if the index or working tree is not HEAD
+m           performa  three-way merge on local modifications if needed
+q,quiet     be quiet
+"
 SUBDIRECTORY_OK=Sometimes
 . git-sh-setup
 require_work_tree
@@ -20,13 +30,12 @@ quiet=
 v=-v
 LF='
 '
-while [ "$#" != "0" ]; do
-    arg="$1"
-    shift
-    case "$arg" in
-	"-b")
-		newbranch="$1"
+
+while test $# != 0; do
+	case "$1" in
+	-b)
 		shift
+		newbranch="$1"
 		[ -z "$newbranch" ] &&
 			die "git checkout: -b needs a branch name"
 		git show-ref --verify --quiet -- "refs/heads/$newbranch" &&
@@ -34,64 +43,54 @@ while [ "$#" != "0" ]; do
 		git check-ref-format "heads/$newbranch" ||
 			die "git checkout: we do not like '$newbranch' as a branch name."
 		;;
-	"-l")
+	-l)
 		newbranch_log=-l
 		;;
-	"--track"|"--no-track")
-		track="$arg"
+	--track|--no-track)
+		track="$1"
 		;;
-	"-f")
+	-f)
 		force=1
 		;;
 	-m)
 		merge=1
 		;;
-	"-q")
+	-q|--quiet)
 		quiet=1
 		v=
 		;;
 	--)
+		shift
 		break
 		;;
-	-*)
-		usage
-		;;
 	*)
-		if rev=$(git rev-parse --verify "$arg^0" 2>/dev/null)
-		then
-			if [ -z "$rev" ]; then
-				echo "unknown flag $arg"
-				exit 1
-			fi
-			new_name="$arg"
-			if git show-ref --verify --quiet -- "refs/heads/$arg"
-			then
-				rev=$(git rev-parse --verify "refs/heads/$arg^0")
-				branch="$arg"
-			fi
-			new="$rev"
-		elif rev=$(git rev-parse --verify "$arg^{tree}" 2>/dev/null)
-		then
-			# checking out selected paths from a tree-ish.
-			new="$rev"
-			new_name="$arg^{tree}"
-			branch=
-		else
-			new=
-			new_name=
-			branch=
-			set x "$arg" "$@"
-			shift
-		fi
-		case "$1" in
-		--)
-			shift ;;
-		esac
-		break
+		usage
 		;;
-    esac
+	esac
+	shift
 done
 
+arg="$1"
+if rev=$(git rev-parse --verify "$arg^0" 2>/dev/null)
+then
+	[ -z "$rev" ] && die "unknown flag $arg"
+	new_name="$arg"
+	if git show-ref --verify --quiet -- "refs/heads/$arg"
+	then
+		rev=$(git rev-parse --verify "refs/heads/$arg^0")
+		branch="$arg"
+	fi
+	new="$rev"
+	shift
+elif rev=$(git rev-parse --verify "$arg^{tree}" 2>/dev/null)
+then
+	# checking out selected paths from a tree-ish.
+	new="$rev"
+	new_name="$arg^{tree}"
+	shift
+fi
+[ "$1" = "--" ] && shift
+
 case "$newbranch,$track" in
 ,--*)
 	die "git checkout: --track and --no-track require -b"
@@ -138,8 +137,8 @@ Did you intend to checkout '$@' which can not be resolved as commit?"
 	git ls-files -- "$@" |
 	git checkout-index -f -u --stdin
 
-        # Run a post-checkout hook -- the HEAD does not change so the
-        # current HEAD is passed in for both args
+	# Run a post-checkout hook -- the HEAD does not change so the
+	# current HEAD is passed in for both args
 	if test -x "$GIT_DIR"/hooks/post-checkout; then
 	    "$GIT_DIR"/hooks/post-checkout $old $old 0
 	fi
@@ -294,5 +293,5 @@ fi
 
 # Run a post-checkout hook
 if test -x "$GIT_DIR"/hooks/post-checkout; then
-        "$GIT_DIR"/hooks/post-checkout $old $new 1
+	"$GIT_DIR"/hooks/post-checkout $old $new 1
 fi
-- 
1.5.3.5.1509.g66d41

[PATCH 09/10] Migrate git-quiltimport.sh to use git-rev-parse --parseopt

[Pierre Habouzit]

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 git-quiltimport.sh |   38 +++++++++++++++-----------------------
 1 files changed, 15 insertions(+), 23 deletions(-)

diff --git a/git-quiltimport.sh b/git-quiltimport.sh
index 880c81d..b6c24c8 100755
--- a/git-quiltimport.sh
+++ b/git-quiltimport.sh
@@ -1,5 +1,11 @@
 #!/bin/sh
-USAGE='--dry-run --author <author> --patches </path/to/quilt/patch/directory>'
+OPTIONS_SPEC="\
+git-quiltimport [options]
+--
+n,dry-run     dry run
+author=       author name and email address for patches without any
+patches=      path to the quilt series and patches
+"
 SUBDIRECTORY_ON=Yes
 . git-sh-setup
 
@@ -8,39 +14,25 @@ quilt_author=""
 while test $# != 0
 do
 	case "$1" in
-	--au=*|--aut=*|--auth=*|--autho=*|--author=*)
-		quilt_author=$(expr "z$1" : 'z-[^=]*\(.*\)')
-		shift
-		;;
-
-	--au|--aut|--auth|--autho|--author)
-		case "$#" in 1) usage ;; esac
+	--author)
 		shift
 		quilt_author="$1"
-		shift
 		;;
-
-	--dry-run)
-		shift
+	-n|--dry-run)
 		dry_run=1
 		;;
-
-	--pa=*|--pat=*|--patc=*|--patch=*|--patche=*|--patches=*)
-		QUILT_PATCHES=$(expr "z$1" : 'z-[^=]*\(.*\)')
-		shift
-		;;
-
-	--pa|--pat|--patc|--patch|--patche|--patches)
-		case "$#" in 1) usage ;; esac
-		shift
+	--patches)
 		QUILT_PATCHES="$1"
 		shift
 		;;
-
+	--)
+		shift
+		break;;
 	*)
-		break
+		usage
 		;;
 	esac
+	shift
 done
 
 # Quilt Author
-- 
1.5.3.5.1509.g66d41

[PATCH 10/10] Migrate git-repack.sh to use git-rev-parse --parseopt

[Pierre Habouzit]

---
 git-repack.sh |   23 ++++++++++++++++++-----
 1 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/git-repack.sh b/git-repack.sh
index 7220635..4d4840e 100755
--- a/git-repack.sh
+++ b/git-repack.sh
@@ -3,7 +3,21 @@
 # Copyright (c) 2005 Linus Torvalds
 #
 
-USAGE='[-a|-A] [-d] [-f] [-l] [-n] [-q] [--max-pack-size=N] [--window=N] [--window-memory=N] [--depth=N]'
+OPTIONS_SPEC="\
+git-repack [options]
+--
+a               pack everything in a single pack
+A               same as -a, and keep unreachable objects too
+d               remove redundant packs, and run git-prune-packed
+f               pass --no-reuse-delta to git-pack-objects
+q,quiet         be quiet
+l               pass --local to git-pack-objects
+ Packing constraints
+window=         size of the window used for delta compression
+window-memory=  same as the above, but limit memory size instead of entries count
+depth=          limits the maximum delta depth
+max-pack-size=  maximum size of each packfile
+"
 SUBDIRECTORY_OK='Yes'
 . git-sh-setup
 
@@ -20,10 +34,9 @@ do
 	-q)	quiet=-q ;;
 	-f)	no_reuse=--no-reuse-object ;;
 	-l)	local=--local ;;
-	--max-pack-size=*) extra="$extra $1" ;;
-	--window=*) extra="$extra $1" ;;
-	--window-memory=*) extra="$extra $1" ;;
-	--depth=*) extra="$extra $1" ;;
+	--max-pack-size|--window|--window-memory|--depth)
+		extra="$extra $1=$2"; shift ;;
+	--) shift; break;;
 	*)	usage ;;
 	esac
 	shift
-- 
1.5.3.5.1509.g66d41

Re: [PATCH 04/10] Migrate git-clone to use git-rev-parse --parseopt

[Pierre Habouzit]

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

  Note: this patch now conflicts with a recent patch to make git clone
grok `--`. As git rev-parse --parseopt does that as a side effect, you
can force the update to the parseopt version without functionality loss.

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

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

Re: [PATCH 04/10] Migrate git-clone to use git-rev-parse --parseopt

[Nicolas Pitre]

On Sun, 4 Nov 2007, Pierre Habouzit wrote:

> Signed-off-by: Pierre Habouzit <madcoder@debian.org>
> ---
>  git-clone.sh |  102 +++++++++++++++++++++++++++++++++-------------------------
>  1 files changed, 58 insertions(+), 44 deletions(-)

Well, this patch was merged in "next" and broke git-clone rather badly.

Just try something as fundamental as this:

	$ git clone git://git.kernel.org/pub/scm/git/git.git
	fatal: Not a git repository

Don't we have test cases covering this really basic operation?


Nicolas

Re: [PATCH 04/10] Migrate git-clone to use git-rev-parse --parseopt

[Junio C Hamano]

Gaah.

I'd blame Linus for suggesting to make parseopt part of
rev-parse, the latter of which makes sense only inside git while
the former of which makes sense outside git.

Would something like this help?

---
 builtin-rev-parse.c |    4 ++--
 git.c               |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin-rev-parse.c b/builtin-rev-parse.c
index 054519b..d1038a0 100644
--- a/builtin-rev-parse.c
+++ b/builtin-rev-parse.c
@@ -337,11 +337,11 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 	int i, as_is = 0, verify = 0;
 	unsigned char sha1[20];
 
-	git_config(git_default_config);
-
 	if (argc > 1 && !strcmp("--parseopt", argv[1]))
 		return cmd_parseopt(argc - 1, argv + 1, prefix);
 
+	prefix = setup_git_directory();
+	git_config(git_default_config);
 	for (i = 1; i < argc; i++) {
 		const char *arg = argv[i];
 
diff --git a/git.c b/git.c
index 6c5f9af..204a6f7 100644
--- a/git.c
+++ b/git.c
@@ -340,7 +340,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "rerere", cmd_rerere, RUN_SETUP },
 		{ "reset", cmd_reset, RUN_SETUP },
 		{ "rev-list", cmd_rev_list, RUN_SETUP },
-		{ "rev-parse", cmd_rev_parse, RUN_SETUP },
+		{ "rev-parse", cmd_rev_parse },
 		{ "revert", cmd_revert, RUN_SETUP | NEED_WORK_TREE },
 		{ "rm", cmd_rm, RUN_SETUP },
 		{ "runstatus", cmd_runstatus, RUN_SETUP | NEED_WORK_TREE },

Re: [PATCH 04/10] Migrate git-clone to use git-rev-parse --parseopt

[Nicolas Pitre]

On Tue, 6 Nov 2007, Junio C Hamano wrote:

> Gaah.
> 
> I'd blame Linus for suggesting to make parseopt part of
> rev-parse, the latter of which makes sense only inside git while
> the former of which makes sense outside git.
> 
> Would something like this help?
> 
> ---
>  builtin-rev-parse.c |    4 ++--
>  git.c               |    2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)

That does help indeed.


Nicolas

Re: [PATCH 04/10] Migrate git-clone to use git-rev-parse --parseopt

[Junio C Hamano]

Nicolas Pitre <nico@cam.org> writes:

> On Sun, 4 Nov 2007, Pierre Habouzit wrote:
>
>> Signed-off-by: Pierre Habouzit <madcoder@debian.org>
>> ---
>>  git-clone.sh |  102 +++++++++++++++++++++++++++++++++-------------------------
>>  1 files changed, 58 insertions(+), 44 deletions(-)
>
> Well, this patch was merged in "next" and broke git-clone rather badly.
>
> Just try something as fundamental as this:
>
> 	$ git clone git://git.kernel.org/pub/scm/git/git.git
> 	fatal: Not a git repository
>
> Don't we have test cases covering this really basic operation?

We do, but RUN_SETUP will happily go up to find the .git/ next
to t/ directory that is the parent of trash/ directory, in which
the tests run, without reporting errors.  As parseopt does not
depend on anything in git, this will not do any harm other than
falsely succeeding the test that should not pass.

We could probably introduce an environment variable, GIT_CEILING,
that tells the setup_git_directory_gentry() never go up beyond
that point, and set it to the t/trash directory while running
the test.

Something like that may have other uses in practice.  Often
people wonder what would happen if there is /.git repository and
they would want to make sure they would not accidentally add to
the repository controlled by /.git when they have bunch of other
repositories /some/where/.git in which they usually work.

Re: [PATCH 01/10] Add a parseopt mode to git-rev-parse to bring parse-options to shell scripts.

[Ralf Wildenhues]

Hello Pierre,

A couple of language nits:

* Pierre Habouzit wrote on Sun, Nov 04, 2007 at 11:30:53AM CET:
> +PARSEOPT
> +--------
> +
> +In `--parseopt` mode, `git-rev-parse` helps massaging options to bring to shell
> +scripts the same facilities C builtins have. It works as an option normalizer
> +(e.g. splits single switches aggregate values), a bit like `getopt(1)` does.
> +
> +It takes on the standard input the specification of the options to parse and
> +understand, and echoes on the standard ouput a line suitable for `sh(1)` `eval`
> +to replace the arguments with normalized ones.  In case of error, it ouputs

s/ouputs/outputs/

> +usage on the standard error stream, and exits with code 129.
> +
> +Input Format
> +~~~~~~~~~~~~
> +
> +`git-ref-parse --parseopt` input format is fully text based. It has two parts,

s/^/The /
s/git-ref-parse/git-rev-parse/

> +separated by a line that contains only `--`. The lines before (should be more
> +than one) are used for the usage. The lines after describe the options.

I would write
  s/before/& the `--`/
  s/after/& the `--`/

or maybe write "separator" instead of `--`.

[...]
> +`<opt_spec>`::
> +	its format is the short option character, then the long option name
> +        separated by a comma. Both parts are not required, though at least one
> +        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 optionnal one (`?` though its use is discouraged) or none

s/optionnal/optional/

> +        (no `<arg_spec>` in that case).
> +
> +The rest of the line after as many spaces up to the ending line feed is used
> +as the help associated to the option.

I'd write (in case that is technically correct):
  After following white space, the rest of the line after is used as the
  help associated to the option.

> +Blank lines are ignored, and lines that don't match this specification are used
> +as option group headers (start the line with a space to purposely create such
> +lines).

I'd write:
  ... to create such lines on purpose.

> +Example
> +~~~~~~~
> +
> +------------
> +OPTS_SPEC="\
> +some-command [options] <args>...
> +
> +some-command does foo and bar !

Please no white space before "!".

> +--
> +h,help    show the help
> +
> +foo       some nifty option --foo
> +bar=      some cool option --bar with an argument
> +
> +  An option group Header
> +C?        option C with an optionnal argument"

s/optionnal/optional/

Cheers,
Ralf

Re: [PATCH 01/10] Add a parseopt mode to git-rev-parse to bring parse-options to shell scripts.

[Ralf Wildenhues]

* Ralf Wildenhues wrote on Sun, Nov 04, 2007 at 12:29:31PM CET:
> A couple of language nits:
> 
> * Pierre Habouzit wrote on Sun, Nov 04, 2007 at 11:30:53AM CET:
[...]
> > +It takes on the standard input the specification of the options to parse and
> > +understand, and echoes on the standard ouput a line suitable for `sh(1)` `eval`

Missed another  s/ouput/output/  here.

> > +to replace the arguments with normalized ones.  In case of error, it ouputs
> 
> s/ouputs/outputs/

Re: [PATCH 01/10] Add a parseopt mode to git-rev-parse to bring parse-options to shell scripts.

[Junio C Hamano]

Two comments.

 * I have updated 1/10 with typo and indentation fixes.

 * I see you changed 2/10 to use OPTIONS_KEEPDASHDASH instead of
   PARSEOPT_OPTS, but the scripts that do not want the --keep
   behaviour do not set OPTIONS_KEEPDASHDASH to empty, so I do
   not see how this updatet would make _any_ difference.  The
   user can still screw up by having OPTIONS_KEEPDASHDASH in
   their environments by mistake, curiosity or just plain
   stupidity.

Re: [PATCH 01/10] Add a parseopt mode to git-rev-parse to bring parse-options to shell scripts.

[Pierre Habouzit]

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

On Sun, Nov 04, 2007 at 10:58:13PM +0000, Junio C Hamano wrote:
> Two comments.
> 
>  * I have updated 1/10 with typo and indentation fixes.
> 
>  * I see you changed 2/10 to use OPTIONS_KEEPDASHDASH instead of
>    PARSEOPT_OPTS, but the scripts that do not want the --keep
>    behaviour do not set OPTIONS_KEEPDASHDASH to empty, so I do
>    not see how this updatet would make _any_ difference.  The
>    user can still screw up by having OPTIONS_KEEPDASHDASH in
>    their environments by mistake, curiosity or just plain
>    stupidity.

  Hmmm right, I was worried by the fact that the old PARSEOPT_OPTS was
being possibly diverted to inject malicious commands. I tend to find the
forced `OPTIONS_KEEPDASHDASH=` thing quite painful, but indeed it is
probably the sole way to guard ourselves against user stupidity (or more
likely unclean environments).

  Do you mind adding: OPTIONS_KEEPDASHDASH= front to the 8 patches that
needs it, or should I send an updated series ? (actually it's more like
the 7 that needs it as git-clean has been rewritten as a builtin if I'm
correct).

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

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