let's retry this based on git-rev-parse --parseopt

let's retry this based on git-rev-parse --parseopt

[Pierre Habouzit]

So as it seems git-rev-parse is the place to do that, here is a new
proposal, with a few more commands migrated. Of course it doesn't means
we should not rewrite some (if not all) of the shell scripts as
builtins, but it will take some pain from the user, and provide nice
option parsers from day 1 for new porcelains.

I believe that the git-am.sh migration is a perfect example of why this
option parser helps.

Note that in the patch 1, the documentation _needs_ to be reviewed by a
native speaker.

[PATCH 1/5] 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.1460.gdb47

[PATCH 2/5] 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.

PARSEOPT_OPTS can also be used to pass options git git-rev-parse --parseopt
mode (like --keep-dashdash).

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

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 86d7d4c..330b78e 100755
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -16,9 +16,32 @@ die() {
 	exit 1
 }
 
-usage() {
-	die "Usage: $0 $USAGE"
-}
+if test -n "$OPTIONS_SPEC"; then
+	usage() {
+		exec "$0" -h
+	}
+
+	eval `echo "$OPTIONS_SPEC" | git rev-parse --parseopt $PARSEOPT_OPTS -- "$@" || 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 +114,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.1460.gdb47

[PATCH 3/5] 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.1460.gdb47

[PATCH 4/5] 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..8680a7c 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 $PARSEOPT_OPTS -- "$@" || 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.1460.gdb47

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

[Pierre Habouzit]

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

diff --git a/git-am.sh b/git-am.sh
index 2514d07..e5ed6a7 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 ;;
-
-	--)
-	shift; break ;;
-	-*)
-	usage ;;
-	*)
-	break ;;
+		-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 ;;
 	esac
+	shift
 done
 
 # If the dotest directory exists, but we have finished applying all the
-- 
1.5.3.5.1460.gdb47

Re: [PATCH 5/5] Migrate git-am.sh to use git-rev-parse --parseopt

[Alex Riesen]

Pierre Habouzit, Fri, Nov 02, 2007 23:39:52 +0100:
> diff --git a/git-am.sh b/git-am.sh
> index 2514d07..e5ed6a7 100755
> --- a/git-am.sh
> +++ b/git-am.sh
...
> -	usage ;;
> -	*)
> -	break ;;
> +		-i|--interactive)
> +			interactive=t ;;
> +		-b|--binary)
> +			binary=t ;;

Did you really have to change the indentation?

Re: [PATCH 5/5] Migrate git-am.sh to use git-rev-parse --parseopt

[Pierre Habouzit]

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

On Sat, Nov 03, 2007 at 09:55:56AM +0000, Alex Riesen wrote:
> Pierre Habouzit, Fri, Nov 02, 2007 23:39:52 +0100:
> > diff --git a/git-am.sh b/git-am.sh
> > index 2514d07..e5ed6a7 100755
> > --- a/git-am.sh
> > +++ b/git-am.sh
> ....
> > -	usage ;;
> > -	*)
> > -	break ;;
> > +		-i|--interactive)
> > +			interactive=t ;;
> > +		-b|--binary)
> > +			binary=t ;;
> 
> Did you really have to change the indentation?

  Well, I'm unsure what the standard is for git, I gladly use any
indentation, I don't really care. I assumed that it wasn't indented
before becauuse instead of -i|--interactive you had:

  -i|--in|--int|....|--interactive which took a lot of space, as it
seemed to me that the case ".." in foo) esac construction in git had the
cases indented in most places. But I may be wrong.

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

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

Re: [PATCH 5/5] Migrate git-am.sh to use git-rev-parse --parseopt

[Pierre Habouzit]

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

On Sat, Nov 03, 2007 at 11:54:45AM +0000, Pierre Habouzit wrote:
> On Sat, Nov 03, 2007 at 09:55:56AM +0000, Alex Riesen wrote:
> > Pierre Habouzit, Fri, Nov 02, 2007 23:39:52 +0100:
> > > diff --git a/git-am.sh b/git-am.sh
> > > index 2514d07..e5ed6a7 100755
> > > --- a/git-am.sh
> > > +++ b/git-am.sh
> > ....
> > > -	usage ;;
> > > -	*)
> > > -	break ;;
> > > +		-i|--interactive)
> > > +			interactive=t ;;
> > > +		-b|--binary)
> > > +			binary=t ;;
> > 
> > Did you really have to change the indentation?
> 
>   Well, I'm unsure what the standard is for git, I gladly use any
> indentation, I don't really care. I assumed that it wasn't indented
> before becauuse instead of -i|--interactive you had:
> 
>   -i|--in|--int|....|--interactive which took a lot of space, as it
> seemed to me that the case ".." in foo) esac construction in git had the
> cases indented in most places. But I may be wrong.

  Okay seems I was wrong, sorry then, that wasn't done on purpose at
all.


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

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

[PATCH 5/5 FIX SPACING] 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.1496.gcb1d6-dirty

[PATCH 6/5] 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.1496.gcb1d6-dirty

[PATCH 8/5] 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.1496.gcb1d6-dirty

[PATCH 9/5] 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..5424745 100755
--- a/git-checkout.sh
+++ b/git-checkout.sh
@@ -1,6 +1,16 @@
 #!/bin/sh
 
-USAGE='[-q] [-f] [-b <new_branch>] [-m] [<branch>] [<paths>...]'
+PARSEOPT_OPTS=--keep-dashdash
+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.1496.gcb1d6-dirty

[PATCH 10/5] 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.1496.gcb1d6-dirty

[PATCH 11/5] 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.1498.gf0d63-dirty

Re: [PATCH 2/5] Update git-sh-setup(1) to allow transparent use of git-rev-parse --parseopt

[Junio C Hamano]

Pierre Habouzit <madcoder@debian.org> writes:

> 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.
>
> PARSEOPT_OPTS can also be used to pass options git git-rev-parse --parseopt
> mode (like --keep-dashdash).
>
> Signed-off-by: Pierre Habouzit <madcoder@debian.org>
> ...
> +if test -n "$OPTIONS_SPEC"; then
> +	usage() {
> +		exec "$0" -h
> +	}
> +
> +	eval `echo "$OPTIONS_SPEC" | git rev-parse --parseopt $PARSEOPT_OPTS -- "$@" || echo exit $?`

I do not quite get why you use $PARSEOPT_OPTS without setting
any yourself, which means that the users can screw themselves by
having something random and insane in their environments.

Trust me that this kind of backdoor, especially when the
intended uses of the backdoor is not documented well, will be
abused by (perhaps clueless, perhaps curious, perhaps fearless)
users and you will get blamed.

So I'd rather (1) first find out what _you_ wanted to use this
backdoor for, (2) see if that is a useful feature to share with
others instead of keeping that to yourself, and (3) if so to
have a much narrower interface to allow such an option that
cannot be abused.

The same comment applies to the git-clone one which has a
similar invocation of "rev-parse --parseopt" because it cannot
source git-sh-setup.

Re: [PATCH 9/5] Migrate git-checkout.sh to use git-rev-parse --parseopt --keep-dashdash

[Junio C Hamano]

Pierre Habouzit <madcoder@debian.org> writes:

> 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..5424745 100755
> --- a/git-checkout.sh
> +++ b/git-checkout.sh
> @@ -1,6 +1,16 @@
>  #!/bin/sh
>  
> -USAGE='[-q] [-f] [-b <new_branch>] [-m] [<branch>] [<paths>...]'
> +PARSEOPT_OPTS=--keep-dashdash
> +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
> +"

Ok, so this is how PARSEOPT_OPTS gets used.  It is a way for the
command that sources git-sh-setup to tell the parseopt code what
to do.  I can agree with this, but then all the other commands
that do not set PARSEOPT_OPTS before sourcing git-sh-setup
should set it to empty string.  Otherwise the users can screw
you with their environment variables.

Re: [PATCH 9/5] Migrate git-checkout.sh to use git-rev-parse --parseopt --keep-dashdash

[Pierre Habouzit]

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

On Sun, Nov 04, 2007 at 07:44:03AM +0000, Junio C Hamano wrote:
> Pierre Habouzit <madcoder@debian.org> writes:
> 
> > 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..5424745 100755
> > --- a/git-checkout.sh
> > +++ b/git-checkout.sh
> > @@ -1,6 +1,16 @@
> >  #!/bin/sh
> >  
> > -USAGE='[-q] [-f] [-b <new_branch>] [-m] [<branch>] [<paths>...]'
> > +PARSEOPT_OPTS=--keep-dashdash
> > +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
> > +"
> 
> Ok, so this is how PARSEOPT_OPTS gets used.  It is a way for the
> command that sources git-sh-setup to tell the parseopt code what
> to do.  I can agree with this, but then all the other commands
> that do not set PARSEOPT_OPTS before sourcing git-sh-setup
> should set it to empty string.  Otherwise the users can screw
> you with their environment variables.

  yes it's the why, and it's also the why this variable isn't quoted
because it is meant to get options to pass to git-rev-parse --parseopt
In fact it's only used in git-checkout right now. I believe the proper
way to do that is that git-sh-setup does a PARSEOPT_OPTS= and that
git-checkout.sh its sole user overrides it once git-sh-setup is sourced.
I'll send the two updated patches for that.

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

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

[UPDATED PATCH 3/5] 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.

PARSEOPT_OPTS can also be used to pass options git git-rev-parse --parseopt
mode (like --keep-dashdash).

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

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 86d7d4c..dfff0d1 100755
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -16,9 +16,34 @@ die() {
 	exit 1
 }
 
-usage() {
-	die "Usage: $0 $USAGE"
-}
+if test -n "$OPTIONS_SPEC"; then
+	# override this after . git-sh-setup if you need to use --keep-dashdash
+	PARSEOPT_OPTS=
+	usage() {
+		exec "$0" -h
+	}
+
+	eval `echo "$OPTIONS_SPEC" | git rev-parse --parseopt $PARSEOPT_OPTS -- "$@" || 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 +116,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.1498.g0a37d

[UPDATED PATCH 5/5] 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.1498.g0a37d

[UPDATED PATCH 9/5] 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..f99f0d5 100755
--- a/git-checkout.sh
+++ b/git-checkout.sh
@@ -1,8 +1,18 @@
 #!/bin/sh
 
-USAGE='[-q] [-f] [-b <new_branch>] [-m] [<branch>] [<paths>...]'
+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
+PARSEOPT_OPTS=--keep-dashdash
 require_work_tree
 
 old_name=HEAD
@@ -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.1498.g0a37d

Re: [PATCH 2/5] Update git-sh-setup(1) to allow transparent use of git-rev-parse --parseopt

[Pierre Habouzit]

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

On Sun, Nov 04, 2007 at 07:43:53AM +0000, Junio C Hamano wrote:
> Trust me that this kind of backdoor, especially when the
> intended uses of the backdoor is not documented well, will be
> abused by (perhaps clueless, perhaps curious, perhaps fearless)
> users and you will get blamed.

  That has been fixed, and documented in git-sh-setup. I renumbered my 3
patches incorrectly though. Those are supposed to supersede patches 2, 4, and 9.

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

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

Re: [PATCH 2/5] Update git-sh-setup(1) to allow transparent use of git-rev-parse --parseopt

[Pierre Habouzit]

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

On dim, nov 04, 2007 at 09:14:49 +0000, Pierre Habouzit wrote:
> On Sun, Nov 04, 2007 at 07:43:53AM +0000, Junio C Hamano wrote:
> > Trust me that this kind of backdoor, especially when the
> > intended uses of the backdoor is not documented well, will be
> > abused by (perhaps clueless, perhaps curious, perhaps fearless)
> > users and you will get blamed.
> 
>   That has been fixed, and documented in git-sh-setup. I renumbered my 3
> patches incorrectly though. Those are supposed to supersede patches 2, 4, and 9.

  And I'm stupid this obviously doesnt work doh..... sorry, I'll
rethought this.


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

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

Re: [PATCH 9/5] Migrate git-checkout.sh to use git-rev-parse --parseopt --keep-dashdash

[Johannes Schindelin]

Hi,

On Sun, 4 Nov 2007, Junio C Hamano wrote:

> Pierre Habouzit <madcoder@debian.org> writes:
> 
> > 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..5424745 100755
> > --- a/git-checkout.sh
> > +++ b/git-checkout.sh
> > @@ -1,6 +1,16 @@
> >  #!/bin/sh
> >  
> > -USAGE='[-q] [-f] [-b <new_branch>] [-m] [<branch>] [<paths>...]'
> > +PARSEOPT_OPTS=--keep-dashdash
> > +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
> > +"
> 
> Ok, so this is how PARSEOPT_OPTS gets used.

I also read in the docs:

> 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.

Why not go the full nine yards and output something which when eval'ed 
sets the variables correctly (taking the variable names from the option 
names; long name if available, otherwise short one)?  It can also set the 
command line arguments to what's left after option parsing, with a "set" 
call.

And to prevent funny games with "PARSEOPT_OPTS=blabla git xyz", why not 
provide a function in git-sh-setup which takes the string as argument, and 
is called _after_ sourcing git-sh-setup?

Ciao,
Dscho

Re: [PATCH 9/5] Migrate git-checkout.sh to use git-rev-parse --parseopt --keep-dashdash

[Pierre Habouzit]

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

On Sun, Nov 04, 2007 at 01:48:36PM +0000, Johannes Schindelin wrote:
> > Pierre Habouzit <madcoder@debian.org> writes:
> > 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.
> 
> Why not go the full nine yards and output something which when eval'ed 
> sets the variables correctly (taking the variable names from the option 
> names; long name if available, otherwise short one)?  It can also set the 
> command line arguments to what's left after option parsing, with a "set" 
> call.

We could do that, though it's not as great as it looks like at the first
glance. If you want -vvv to work like an accumulator, then you need a
really more complex approach in the C code. To enter the gory details,
git-rev-parse --parseopt uses a callback that deals with options and
their arguments one by one, then appends a delimiter to tell the shell
script that only arguments follow, and then appends the arguments the
option parser left alone.

It does not deal with the semantics that the C has available at all,
it's up to the shell script to decide which is better.

My goal with this is not really to do all the work for the shell script
author, but rather to the user: it's not because a porcelain is new (or
not a builtin yet) that it should have a creepy interface. If it helps
the programmer as a side effect, then it's great, but this series really
is about usability to me.

Of course we can do what you propose, but it will probably be quite
sophisticated and looks to me like an overkill to what shell builtins
really are used for: prototyping a new porcelain until it becomes a new
full blown C-builtin.  I do believe in simplicity after all :)

> And to prevent funny games with "PARSEOPT_OPTS=blabla git xyz", why not 
> provide a function in git-sh-setup which takes the string as argument, and 
> is called _after_ sourcing git-sh-setup?

This one is quite a non issue, it's only used for keep-dashdash, and I
see no other need in the near future. And if the need arise, it'll still
be doable any time. So for now I've taken a non-generic way to do that,
and I believe it's fine as it is.
-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

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