[PATCH Cogito] Improve option parsing for cg-log

[PATCH Cogito] Improve option parsing for cg-log

[Marcel Holtmann]

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

Hi Petr,

the attached patch changes the option parsing, because otherwise we are
stuck to a specific order.

Regards

Marcel


Signed-off-by: Marcel Holtmann <marcel@holtmann.org>


[-- Attachment #2: patch --]
[-- Type: text/x-patch, Size: 1294 bytes --]

Index: cg-log
===================================================================
--- 456fffab323ed972b8e47fa0fb83c3b5cd9243d7/cg-log  (mode:100755)
+++ uncommitted/cg-log  (mode:100755)
@@ -27,29 +27,34 @@
 # at least somewhere it does. Bash is broken.
 trap exit SIGPIPE
 
-if [ "$1" = "-c" ]; then
-	shift
-	# See terminfo(5), "Color Handling"
-	colheader="$(tput setaf 2)"    # Green
-	colauthor="$(tput setaf 6)"    # Cyan
-	colcommitter="$(tput setaf 5)" # Magenta
-	colfiles="$(tput setaf 4)"     # Blue
-	colsignoff="$(tput setaf 3)"   # Yellow
-	coldefault="$(tput op)"        # Restore default
-else
-	colheader=
-	colauthor=
-	colcommitter=
-	colfiles=
-	colsignoff=
-	coldefault=
-fi
-
+colheader=
+colauthor=
+colcommitter=
+colfiles=
+colsignoff=
+coldefault=
 list_files=
-if [ "$1" = "-f" ]; then
-	shift
-	list_files=1
-fi
+while [ "$1" ]; do
+	case "$1" in
+	-c)
+		# See terminfo(5), "Color Handling"
+		colheader="$(tput setaf 2)"    # Green
+		colauthor="$(tput setaf 6)"    # Cyan
+		colcommitter="$(tput setaf 5)" # Magenta
+		colfiles="$(tput setaf 4)"     # Blue
+		colsignoff="$(tput setaf 3)"   # Yellow
+		coldefault="$(tput op)"        # Restore default
+		shift
+		;;
+	-f)
+		list_files=1
+		shift
+		;;
+	*)
+		break
+		;;
+	esac
+done
 
 list_commit_files()
 {

Re: [PATCH Cogito] Improve option parsing for cg-log

[Petr Baudis]

Dear diary, on Thu, May 12, 2005 at 10:51:54PM CEST, I got a letter
where Marcel Holtmann <marcel@holtmann.org> told me that...
> Hi Petr,

Hi,

> the attached patch changes the option parsing, because otherwise we are
> stuck to a specific order.

thanks, applied. However, you didn't include the -r options parsing in
there yet.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
C++: an octopus made by nailing extra legs onto a dog. -- Steve Taylor

Re: [PATCH Cogito] Improve option parsing for cg-log

[Marcel Holtmann]

Hi Petr,

> > the attached patch changes the option parsing, because otherwise we are
> > stuck to a specific order.
> 
> thanks, applied. However, you didn't include the -r options parsing in
> there yet.

what do you mean by that?

Regards

Marcel

Re: [PATCH Cogito] Improve option parsing for cg-log

[Petr Baudis]

Dear diary, on Thu, May 12, 2005 at 11:49:46PM CEST, I got a letter
where Marcel Holtmann <marcel@holtmann.org> told me that...
> Hi Petr,
> 
> > > the attached patch changes the option parsing, because otherwise we are
> > > stuck to a specific order.
> > 
> > thanks, applied. However, you didn't include the -r options parsing in
> > there yet.
> 
> what do you mean by that?

The -r option still must be after all the other options.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
C++: an octopus made by nailing extra legs onto a dog. -- Steve Taylor

Re: [PATCH Cogito] Improve option parsing for cg-log

[Marcel Holtmann]

Hi Petr,

> > > > the attached patch changes the option parsing, because otherwise we are
> > > > stuck to a specific order.
> > > 
> > > thanks, applied. However, you didn't include the -r options parsing in
> > > there yet.
> > 
> > what do you mean by that?
> 
> The -r option still must be after all the other options.

I see what you mean and it seems that I missed that option. Must be
because you put the list_commit_files() between them and I assumed that
there is no further option parsing.

Do you really wanna keep the double meaning of -r. Depending on a
previous -r it is $log_start or $log_end.

Regards

Marcel

Re: [PATCH Cogito] Improve option parsing for cg-log

[Dan Holmsand]

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

Petr Baudis wrote:
> The -r option still must be after all the other options.
> 

I've been thinking about option parsing as well, and I think cogito 
could use a more "getopt-like" handling of options. "cg-log -cf"
or "cg-diff -rorigin" is just so much easier to type...

The attached patch implements that for cg-log and cg-diff, by means of 
two new helper functions in cg-Xlib. It also improves error-handling a bit.

How about it?

/dan

[-- Attachment #2: optparse.patch --]
[-- Type: text/x-patch, Size: 5284 bytes --]

Better option parsing in cg-diff and cg-log. Uses a new function, optparse,
in cg-Xlib, that allows for getopt-style option parsing.

This means that "cg-log -cf" is equivalent to "cg-log -c -f", as is
"cg-log -fc" and "cg-log -fc -rHEAD --" and even
"cg-log --file-list --color --revision=HEAD".

---

 cg-Xlib |   33 ++++++++++++++++++++++++++++++
 cg-diff |   53 ++++++++++++++++++++++---------------------------
 cg-log  |   69 ++++++++++++++++++++++++----------------------------------------
 3 files changed, 83 insertions(+), 72 deletions(-)

Index: cg-Xlib
===================================================================
--- de641904363cd3759f132ee7c0dfaf8a2ee58388/cg-Xlib  (mode:100755)
+++ 048128149b56f55427713c78a8a3ca9da811e589/cg-Xlib  (mode:100755)
@@ -62,6 +62,39 @@
 	fi
 done
 
+# option parsing
+
+opts=($@)
+
+optshift() {
+	unset opts[0]
+	opts=("${opts[@]}")
+	[ -z "$1" -o -n "$opts" ] || die "option \`$1' requires an argument"
+}
+
+optparse() {
+	[ -z "$1" ] && case $opts in
+		--) optshift; return 1 ;;
+		-*) return 0 ;;
+		*)  return 1 ;;
+	esac
+
+	local match=${1%=} minmatch=${2:-1} o=$opts val
+	[[ $1 == *= ]] && val=$match
+	case $match in
+	--*)	[ "$val" ] && o=${opts%%=*}
+		[ ${#o} -ge $((2 + $minmatch)) -a \
+			"${match:0:${#o}}" = "$o" ] || return 1
+		[[ -n "$val" && "$opts" == *=* ]] && opts[0]=${opts#*=} \
+			|| optshift $val ;;
+	-?)	[[ $o == $match* ]] || return 1
+		[[ $o != -?-* || -n "$val" ]] || die "unrecognized option \`$o'"
+		opts[0]=${o#$match}
+		[ "$opts" ] && { [ "$val" ] || opts[0]=-${opts}; } \
+			|| optshift $val ;;
+	*)	die "optparse cannot handle $1" ;;
+	esac
+}
 
 # Compatibility hacks:
 # 2005-04-26
Index: cg-diff
===================================================================
--- de641904363cd3759f132ee7c0dfaf8a2ee58388/cg-diff  (mode:100755)
+++ 048128149b56f55427713c78a8a3ca9da811e589/cg-diff  (mode:100755)
@@ -19,34 +19,33 @@
 . ${COGITO_LIB}cg-Xlib
 
 
-id1=" "
-id2=" "
-parent=
+unset id1 id2 parent
 
+while optparse; do
+	if optparse -p || optparse --parent; then
+		parent=1
+	elif optparse -r= || optparse --revision=; then
+		if [ set != "${id1+set}" ]; then
+			id1=$opts
+			if [[ "$id1" == *:* ]]; then
+				id2=${id1#*:}
+				id1=${id1%:*}
+			fi
+		else
+			[ set != "${id2+set}" ] || die "too many versions"
+			id2=$opts
+		fi
+		optshift
+	else
+		die "unrecognized option \`$opts'"
+	fi
+done
+shift $(( $# - ${#opts[*]} ))
 
-# FIXME: The commandline parsing is awful.
-
-if [ "$1" = "-p" ]; then
-	shift
-	parent=1
-fi
-
-if [ "$1" = "-r" ]; then
-	shift
-	id1=$(echo "$1": | cut -d : -f 1)
-	[ "$id1" != "$1" ] && id2=$(echo "$1": | cut -d : -f 2)
-	shift
-fi
-
-if [ "$1" = "-r" ]; then
-	shift
-	id2="$1"
-	shift
-fi
 
 if [ "$parent" ]; then
 	id2="$id1"
-	id1=$(parent-id "$id2" | head -n 1)
+	id1=$(parent-id "$id2" | head -n 1) || exit 1
 fi
 
 
@@ -58,12 +57,8 @@
 	done
 fi
 
-if [ "$id2" = " " ]; then
-	if [ "$id1" != " " ]; then
-		tree=$(tree-id "$id1")
-	else
-		tree=$(tree-id)
-	fi
+if [ set != "${id2+set}" ]; then
+	tree=$(tree-id "${id1:-HEAD}") || exit 1
 
 	# Ensure to only diff modified files
 	git-update-cache --refresh
Index: cg-log
===================================================================
--- de641904363cd3759f132ee7c0dfaf8a2ee58388/cg-log  (mode:100755)
+++ 048128149b56f55427713c78a8a3ca9da811e589/cg-log  (mode:100755)
@@ -30,18 +30,11 @@
 # at least somewhere it does. Bash is broken.
 trap exit SIGPIPE
 
-colheader=
-colauthor=
-colcommitter=
-colfiles=
-colsignoff=
-coldefault=
-list_files=
-user=
-while [ "$1" ]; do
-	# TODO: Parse -r here too.
-	case "$1" in
-	-c)
+unset colheader colauthor colcommitter colfiles colsignoff coldefault
+unset list_files log_start log_end files user
+
+while optparse; do
+	if optparse -c || optparse --color; then
 		# See terminfo(5), "Color Handling"
 		colheader="$(tput setaf 2)"    # Green
 		colauthor="$(tput setaf 6)"    # Cyan
@@ -49,21 +42,28 @@
 		colfiles="$(tput setaf 4)"     # Blue
 		colsignoff="$(tput setaf 3)"   # Yellow
 		coldefault="$(tput op)"        # Restore default
-		shift
-		;;
-	-f)
+	elif optparse -f || optparse --file-list; then
 		list_files=1
-		shift
-		;;
-	-u*)
-		user="${1#-u}"
-		shift
-		;;
-	*)
-		break
-		;;
-	esac
+	elif optparse -u= || optparse --user=; then
+		user=$opts
+		optshift
+	elif optparse -r= || optparse --revision=; then
+		if [ set != "${log_start+set}" ]; then
+			log_start=$opts
+			if [[ "$log_start" == *:* ]]; then
+				log_end=${log_start#*:}
+				log_start=${log_start%:*}
+			fi
+		else
+			[ set != "${log_end+set}" ] || die "too many revisions"
+			log_end=$opts
+		fi
+		optshift
+	else
+		die "unrecognized option \`$opts'"
+	fi
 done
+shift $(( $# - ${#opts[*]} ))
 
 list_commit_files()
 {
@@ -92,24 +92,7 @@
 	echo "$coldefault:"
 }
 
-log_start=
-log_end=
-if [ "$1" = "-r" ]; then
-	shift
-	log_start="$1"
-	shift
-	if echo "$log_start" | grep -q ':'; then
-		log_end=$(echo "$log_start" | cut -d : -f 2)
-		log_start=$(echo "$log_start" | cut -d : -f 1)
-	fi
-fi
-if [ "$1" = "-r" ]; then
-	shift
-	log_end="$1"
-	shift
-fi
-
-if [ "$log_end" ]; then
+if [ set = "${log_end+set}" ]; then
 	id1="$(commit-id $log_start)" || exit 1
 	id2="$(commit-id $log_end)" || exit 1
 	revls="git-rev-tree $id2 ^$id1"

Re: [PATCH Cogito] Improve option parsing for cg-log

[Petr Baudis]

Dear diary, on Fri, May 13, 2005 at 11:05:34AM CEST, I got a letter
where Marcel Holtmann <marcel@holtmann.org> told me that...
> Hi Petr,

Hi,

> > The -r option still must be after all the other options.
> 
> I see what you mean and it seems that I missed that option. Must be
> because you put the list_commit_files() between them and I assumed that
> there is no further option parsing.

Uhm, I know. :-) It sorta evolved like that. We didn't yet settle down
on where to actually put the functions. I'd say right after . cg-Xlib.

> Do you really wanna keep the double meaning of -r. Depending on a
> previous -r it is $log_start or $log_end.

Yes, by all means. On one side I like the colon notation, on the other
side this was always my biggest usability problem with SVN. And it costs
us nothing and does what the user would after all expect, I think.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
C++: an octopus made by nailing extra legs onto a dog. -- Steve Taylor