Bring parse_options to the shell

Bring parse_options to the shell

[Pierre Habouzit]

This is also something that itches me, so here is a proposal for a
git-parseopt helper that can be used in shell scripts as an option
normalizer like getopt(1) does.

I migrated the discussed git-clean.sh to use it as a proof of concept.

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

[PATCH] Add git-parseopt(1) to bring parse-options to shell scripts.

[Pierre Habouzit]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 8709 bytes --]

---
 .gitignore                     |    1 +
 Documentation/cmd-list.perl    |    1 +
 Documentation/git-parseopt.txt |   91 +++++++++++++++++++++++++++++
 Makefile                       |    1 +
 builtin-parseopt.c             |  123 ++++++++++++++++++++++++++++++++++++++++
 builtin.h                      |    1 +
 git.c                          |    1 +
 7 files changed, 219 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/git-parseopt.txt
 create mode 100644 builtin-parseopt.c

diff --git a/.gitignore b/.gitignore
index c8c13f5..521155c 100644
--- a/.gitignore
+++ b/.gitignore
@@ -86,6 +86,7 @@ git-pack-redundant
 git-pack-objects
 git-pack-refs
 git-parse-remote
+git-parseopt
 git-patch-id
 git-peek-remote
 git-prune
diff --git a/Documentation/cmd-list.perl b/Documentation/cmd-list.perl
index 8d21d42..c491964 100755
--- a/Documentation/cmd-list.perl
+++ b/Documentation/cmd-list.perl
@@ -147,6 +147,7 @@ git-pack-objects                        plumbingmanipulators
 git-pack-redundant                      plumbinginterrogators
 git-pack-refs                           ancillarymanipulators
 git-parse-remote                        synchelpers
+git-parseopt                            plumbingmanipulators
 git-patch-id                            purehelpers
 git-peek-remote                         purehelpers
 git-prune                               ancillarymanipulators
diff --git a/Documentation/git-parseopt.txt b/Documentation/git-parseopt.txt
new file mode 100644
index 0000000..77f5a10
--- /dev/null
+++ b/Documentation/git-parseopt.txt
@@ -0,0 +1,91 @@
+git-parseopt(1)
+============
+
+NAME
+----
+git-parseopt - Git shell script helper similar to getopt(1)
+
+
+SYNOPSIS
+--------
+'git-parseopt' [-u] [--keep-dashdash] -- [<args>...]
+
+DESCRIPTION
+-----------
+
+This is a helper for scripts to behave the same as git C commands. It merely
+breaks and normalize options so that the option parsing becomes shorter and
+easier to read. It takes the specifications of the options to undersand on its
+standard input.
+
+In case of error, it ouputs usage on the standard error stream, and exits with
+code 129. Else its ouput is suitable for evaluation with "eval" to replace
+the script arguments with the normalized form.
+
+OPTIONS
+-------
+
+--keep-dash-dash::
+	Keep the `--` in `<args>...`.
+
+\--::
+	Following arguments are the options to parse. This isn't optional.
+
+<args>...::
+	Options to parse and deal with
+
+INPUT FORMAT
+------------
+
+`git-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)
+is 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.
+
+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
+C?        option C with an optionnal argument"
+
+eval `echo "$OPTS_SPEC" | git-parseopt -- "$@" || echo exit $?`
+------------
+
+Author
+------
+Written by Pierre Habouzit <madcoder@debian.org>
+
+Documentation
+--------------
+Documentation by Pierre Habouzit.
+
+GIT
+---
+Part of the gitlink:git[7] suite
diff --git a/Makefile b/Makefile
index 042f79e..7c5df55 100644
--- a/Makefile
+++ b/Makefile
@@ -352,6 +352,7 @@ BUILTIN_OBJS = \
 	builtin-mv.o \
 	builtin-name-rev.o \
 	builtin-pack-objects.o \
+	builtin-parseopt.o \
 	builtin-prune.o \
 	builtin-prune-packed.o \
 	builtin-push.o \
diff --git a/builtin-parseopt.c b/builtin-parseopt.c
new file mode 100644
index 0000000..c032f5a
--- /dev/null
+++ b/builtin-parseopt.c
@@ -0,0 +1,123 @@
+/*
+ * Copyright © 2007 Pierre Habouzit <madcoder@debian.org>
+ */
+#include "git-compat-util.h"
+#include "cache.h"
+#include "quote.h"
+#include "parse-options.h"
+
+static int keep_dashdash = 0;
+static struct strbuf parsed = STRBUF_INIT;
+
+static struct option parseopt_opts[] = {
+	OPT_BOOLEAN(0, "keep-dashdash", &keep_dashdash,
+				"keep the `--` passed as an arg"),
+	OPT_END(),
+};
+
+static char const * const parseopt_usage[] = {
+	"git-parseopt [options] -- [<args>...]",
+	NULL
+};
+
+static int parseopt_dump(const struct option *o, const char *arg, int unset)
+{
+	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;
+}
+
+int cmd_parseopt(int argc, const char **argv, const char *prefix)
+{
+	struct strbuf sb;
+	const char **usage = NULL;
+	struct option *opts = NULL;
+	int onb = 0, osz = 0, unb = 0, usz = 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;
+
+		ALLOC_GROW(opts, onb + 1, osz);
+		memset(opts + onb, 0, sizeof(opts[onb]));
+
+		o = &opts[onb++];
+		s = strchr(sb.buf, ' ');
+		if (!s || *sb.buf == ' ')
+			die("invalid option spec");
+
+		o->type = OPTION_CALLBACK;
+		o->help = xstrdup(skipspaces(s));
+		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;
+}
diff --git a/builtin.h b/builtin.h
index 9a6213a..b2ef7b6 100644
--- a/builtin.h
+++ b/builtin.h
@@ -55,6 +55,7 @@ extern int cmd_merge_file(int argc, const char **argv, const char *prefix);
 extern int cmd_mv(int argc, const char **argv, const char *prefix);
 extern int cmd_name_rev(int argc, const char **argv, const char *prefix);
 extern int cmd_pack_objects(int argc, const char **argv, const char *prefix);
+extern int cmd_parseopt(int argc, const char **argv, const char *prefix);
 extern int cmd_pickaxe(int argc, const char **argv, const char *prefix);
 extern int cmd_prune(int argc, const char **argv, const char *prefix);
 extern int cmd_prune_packed(int argc, const char **argv, const char *prefix);
diff --git a/git.c b/git.c
index 4e10581..89524dd 100644
--- a/git.c
+++ b/git.c
@@ -333,6 +333,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "mv", cmd_mv, RUN_SETUP | NEED_WORK_TREE },
 		{ "name-rev", cmd_name_rev, RUN_SETUP },
 		{ "pack-objects", cmd_pack_objects, RUN_SETUP },
+		{ "parseopt", cmd_parseopt },
 		{ "pickaxe", cmd_blame, RUN_SETUP },
 		{ "prune", cmd_prune, RUN_SETUP },
 		{ "prune-packed", cmd_prune_packed, RUN_SETUP },
-- 
1.5.3.5.1458.g2aa13-dirty

[PATCH] Update git-sh-setup(1) to allow transparent use of git-parseopt(1)

[Pierre Habouzit]

If you set OPTIONS_SPEC, git-sh-setups uses git-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-parseopt(1) (like
--keep-dashdash).
---
 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..d7b13e0 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 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.1458.g2aa13-dirty

[PATCH] Migrate git-clean.sh to use git-parseoptions(1)

[Pierre Habouzit]

Also minor consistency tweaks in how errors are caught.
---

    This patch is a pretty good example of the fact that git-parseopt(1)
    is not very disruptive to the general coding style thanks to
    git-sh-setup.

 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.1458.g2aa13-dirty

Re: Bring parse_options to the shell

[Pierre Habouzit]

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

On Fri, Nov 02, 2007 at 03:09:18PM +0000, Pierre Habouzit wrote:
> This is also something that itches me, so here is a proposal for a
> git-parseopt helper that can be used in shell scripts as an option
> normalizer like getopt(1) does.
> 
> I migrated the discussed git-clean.sh to use it as a proof of concept.

  Needless to say, this is fetchable from
git://git.madism.org/git.git#ph/parseopt

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

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

Re: Bring parse_options to the shell

[Linus Torvalds]

On Fri, 2 Nov 2007, Pierre Habouzit wrote:

> On Fri, Nov 02, 2007 at 03:09:18PM +0000, Pierre Habouzit wrote:
> > This is also something that itches me, so here is a proposal for a
> > git-parseopt helper that can be used in shell scripts as an option
> > normalizer like getopt(1) does.
> > 
> > I migrated the discussed git-clean.sh to use it as a proof of concept.
> 
>   Needless to say, this is fetchable from
> git://git.madism.org/git.git#ph/parseopt

Hmm. Any reason why you didn't just extend on "git rev-parse"?

That command was written exactly to parse a command line. This is really 
cheesy, and doesn't really work right (it splits up numbers too), but you 
get the idea..

		Linus

---
 builtin-rev-parse.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/builtin-rev-parse.c b/builtin-rev-parse.c
index 8d78b69..10b62f7 100644
--- a/builtin-rev-parse.c
+++ b/builtin-rev-parse.c
@@ -116,6 +116,17 @@ static int show_flag(const char *arg)
 	if (!(filter & DO_FLAGS))
 		return 0;
 	if (filter & (is_rev_argument(arg) ? DO_REVS : DO_NOREV)) {
+		if (arg[0] == '-' && arg[1] != '-') {
+			char split[3];
+			split[0] = '-';
+			split[2] = 0;
+			while (arg[1]) {
+				split[1] = arg[1];
+				arg++;
+				show(split);
+			}
+			return 1;
+		}
 		show(arg);
 		return 1;
 	}

[PATCH] Migrate git-clone to use git-parseopt(1)

[Pierre Habouzit]

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

diff --git a/git-clone.sh b/git-clone.sh
index 0ea3c24..b30005d 100755
--- a/git-clone.sh
+++ b/git-clone.sh
@@ -8,15 +8,35 @@
 # 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 parseopt $PARSEOPT_OPTS -- "$@" || echo exit $?`
+
 get_repo_base() {
 	(
 		cd "`/bin/pwd`" &&
@@ -106,64 +126,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.1458.g2aa13-dirty

Re: Bring parse_options to the shell

[Pierre Habouzit]

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

On Fri, Nov 02, 2007 at 03:51:13PM +0000, Linus Torvalds wrote:
> 
> 
> On Fri, 2 Nov 2007, Pierre Habouzit wrote:
> 
> > On Fri, Nov 02, 2007 at 03:09:18PM +0000, Pierre Habouzit wrote:
> > > This is also something that itches me, so here is a proposal for a
> > > git-parseopt helper that can be used in shell scripts as an option
> > > normalizer like getopt(1) does.
> > > 
> > > I migrated the discussed git-clean.sh to use it as a proof of concept.
> > 
> >   Needless to say, this is fetchable from
> > git://git.madism.org/git.git#ph/parseopt
> 
> Hmm. Any reason why you didn't just extend on "git rev-parse"?

Because I wasn't aware of the fact it was used to massage arguments of a
function :) Though looking at the code git-rev-parse looks quite
complicated and does not works the proper way:

It show()s the arguments along the way, whereas you definitely need to
parse them first if you end up spitting out the usage. I could of course
plumb it as a new "mode" of git-rev-parse, but it sounds awkward.

> That command was written exactly to parse a command line. This is really 
> cheesy, and doesn't really work right (it splits up numbers too), but you 
> get the idea..

  I get the idea, though parse-options is not incremental at all, this
could probably be done, but would complicate the API (we would need to
allocate a state object e.g.). And parseoptions checks that options
getting an argument have one, checks that options exists and so on. It
looks like to me that it's not easy to plumb into rev-parse without
being a brand new independant mode.

  We can do that, if we don't want yet-another-git-builtin/command, but
in the spirit it'll remain a brand new "thing".

  Though I'd be glad to hear about what others think about it.

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

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

Re: Bring parse_options to the shell

[Johannes Schindelin]

Hi,

On Fri, 2 Nov 2007, Pierre Habouzit wrote:

> On Fri, Nov 02, 2007 at 03:51:13PM +0000, Linus Torvalds wrote:
> 
> > That command [rev-parse] was written exactly to parse a command line. 
> > This is really cheesy, and doesn't really work right (it splits up 
> > numbers too), but you get the idea..
> 
>   I get the idea, though parse-options is not incremental at all, this 
> could probably be done, but would complicate the API (we would need to 
> allocate a state object e.g.). And parseoptions checks that options 
> getting an argument have one, checks that options exists and so on. It 
> looks like to me that it's not easy to plumb into rev-parse without 
> being a brand new independant mode.
> 
>   We can do that, if we don't want yet-another-git-builtin/command, but 
> in the spirit it'll remain a brand new "thing".
> 
>   Though I'd be glad to hear about what others think about it.

Yeah, rev-parse's only purpose in life is to help scripts.  (Even if it is 
used sometimes -- even by myself -- to turn symbolic names into SHA-1s.)

IMHO it makes tons of sense to put the functionality into that command, 
even if it is not incremental.

Ciao,
Dscho

Re: Bring parse_options to the shell

[Pierre Habouzit]

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

On Fri, Nov 02, 2007 at 06:21:17PM +0000, Johannes Schindelin wrote:
> Hi,
> 
> On Fri, 2 Nov 2007, Pierre Habouzit wrote:
> 
> > On Fri, Nov 02, 2007 at 03:51:13PM +0000, Linus Torvalds wrote:
> > 
> > > That command [rev-parse] was written exactly to parse a command line. 
> > > This is really cheesy, and doesn't really work right (it splits up 
> > > numbers too), but you get the idea..
> > 
> >   I get the idea, though parse-options is not incremental at all, this 
> > could probably be done, but would complicate the API (we would need to 
> > allocate a state object e.g.). And parseoptions checks that options 
> > getting an argument have one, checks that options exists and so on. It 
> > looks like to me that it's not easy to plumb into rev-parse without 
> > being a brand new independant mode.
> > 
> >   We can do that, if we don't want yet-another-git-builtin/command, but 
> > in the spirit it'll remain a brand new "thing".
> > 
> >   Though I'd be glad to hear about what others think about it.
> 
> Yeah, rev-parse's only purpose in life is to help scripts.  (Even if it is 
> used sometimes -- even by myself -- to turn symbolic names into SHA-1s.)
> 
> IMHO it makes tons of sense to put the functionality into that command, 
> even if it is not incremental.

  Okay so what do I do ? I create a new mode for git-rev-parse that does
what I do in git-parseopt ? I can do that, I don't like it a lot, but if
people it's better to work this way... It's also kind of counter
intuitive to have git *rev-parse* doing that but oh well, after all it's
plumbing

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

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