[PATCH 2/4] easy-build.sh: use getopts instead of rolling our own option parsing.
--- easy-build.sh | 28 1 files changed, 16 insertions(+), 12 deletions(-) diff --git a/easy-build.sh b/easy-build.sh index c586c1e..d85d810 100755 --- a/easy-build.sh +++ b/easy-build.sh @@ -25,19 +25,23 @@ if [ $# -eq 0 ]; then fi desktop= -if [ $1 = -d ]; then - case $2 in - # Note: gnome is the special gnome task, not the generic task - gnome|kde|lxde|xfce|light|all) - desktop=$2 - shift 2 - ;; - *) - show_usage - exit 1 - ;; +while getopts d:h OPT ; do + case $OPT in + d) + case $OPTARG in + # Note: gnome is the special gnome task, not the generic task + gnome|kde|lxde|xfce|light|all) + desktop=$2 + ;; + *) + show_usage + exit 1 + ;; + esac ;; + h) show_usage; exit 1;; esac -fi +done +shift $(expr $OPTIND - 1) export DISKTYPE=$1 shift -- 1.6.3.3 -- To UNSUBSCRIBE, email to debian-cd-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Re: [PATCH 2/4] easy-build.sh: use getopts instead of rolling our own option parsing.
On Friday 07 August 2009, Ian Campbell wrote: +while getopts d:h OPT ; do Is getopts also supported in dash? + case $OPT in + d) + case $OPTARG in + # Note: gnome is the special gnome task, not the generic task + gnome|kde|lxde|xfce|light|all) + desktop=$2 + ;; + *) + show_usage + exit 1 + ;; + esac ;; + h) show_usage; exit 1;; Please put the commands for the h option on separate lines (as is done for the others). AFAIK a plain 'show_usage', if not displayed as the result of an error, should have an 'exit 0'. The -h option should be listed in the usage output. Maybe it would be good to also support --help if -h is added.
Re: [PATCH 2/4] easy-build.sh: use getopts instead of rolling our own option parsing.
On Fri, Aug 07, 2009 at 02:05:52PM +0200, Frans Pop wrote: On Friday 07 August 2009, Ian Campbell wrote: +while getopts d:h OPT ; do Is getopts also supported in dash? Yes, and in POSIX. Maybe it would be good to also support --help if -h is added. getopts can't do longoptions. I'd say supporting --help is probably not worth the extra code. Max -- To UNSUBSCRIBE, email to debian-cd-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Re: [PATCH 2/4] easy-build.sh: use getopts instead of rolling our own option parsing.
On Fri, 2009-08-07 at 14:05 +0200, Frans Pop wrote:
On Friday 07 August 2009, Ian Campbell wrote:
+while getopts d:h OPT ; do
Is getopts also supported in dash?
I believe so.
I just tried it and it looks like the script is already not dash-ready:
$ dash -x ./easy-build.sh -h
+ set -e
+ export CF=CONF.sh
+ . CONF.sh
.: 1: CONF.sh: not found
(Just to show dash knows about getops since the above didn't get that
far:
$ dash -c 'getopts'
getopts: 1: Usage: getopts optstring var [arg]
)
+ case $OPT in
+ d)
+ case $OPTARG in
+ # Note: gnome is the special gnome task, not the generic
task
+ gnome|kde|lxde|xfce|light|all)
+ desktop=$2
+ ;;
+ *)
+ show_usage
+ exit 1
+ ;;
+ esac ;;
+ h) show_usage; exit 1;;
Please put the commands for the h option on separate lines (as is done
for the others).
Done.
AFAIK a plain 'show_usage', if not displayed as the result of an error,
should have an 'exit 0'.
Done.
The -h option should be listed in the usage output.
Done.
Maybe it would be good to also support --help if -h is added.
Since getopts doesn't support long options I agree with Max that it
isn't worth the complexity.
Updated patch below.
Ian.
easy-build.sh: use getopts instead of rolling our own option parsing.
---
easy-build.sh | 29 ++---
1 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/easy-build.sh b/easy-build.sh
index c586c1e..5e7a31a 100755
--- a/easy-build.sh
+++ b/easy-build.sh
@@ -6,7 +6,7 @@ set -e
## See also CONF.sh for the meaning of variables used here.
show_usage() {
- echo Usage: $(basename $0) [-d gnome|kde|lxde|xfce|light|all]
BC|NETINST|CD|DVD [ARCH ...]
+ echo Usage: $(basename $0) [-h] [-d gnome|kde|lxde|xfce|light|all]
BC|NETINST|CD|DVD [ARCH ...]
}
@@ -25,19 +25,26 @@ if [ $# -eq 0 ]; then
fi
desktop=
-if [ $1 = -d ]; then
- case $2 in
- # Note: gnome is the special gnome task, not the generic task
- gnome|kde|lxde|xfce|light|all)
- desktop=$2
- shift 2
- ;;
- *)
+while getopts d:h OPT ; do
+ case $OPT in
+ d)
+ case $OPTARG in
+ # Note: gnome is the special gnome task, not the generic task
+ gnome|kde|lxde|xfce|light|all)
+ desktop=$2
+ ;;
+ *)
+ show_usage
+ exit 1
+ ;;
+ esac ;;
+ h)
show_usage
- exit 1
+ exit 0
;;
esac
-fi
+done
+shift $(expr $OPTIND - 1)
export DISKTYPE=$1
shift
--
1.6.3.3
--
Ian Campbell
Current Noise: Witchcraft - It's So Easy
Logic is the chastity belt of the mind!
--
To UNSUBSCRIBE, email to debian-cd-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Re: [PATCH 2/4] easy-build.sh: use getopts instead of rolling our own option parsing.
On Fri, 2009-08-07 at 13:33 +0100, Ian Campbell wrote:
I just tried it and it looks like the script is already not
dash-ready:
$ dash -x ./easy-build.sh -h
+ set -e
+ export CF=CONF.sh
+ . CONF.sh
.: 1: CONF.sh: not found
I'm no expert but it looks like dash obeys $PATH when executing the ..
The manpage doesn't mention this behaviour and bash doesn't seem to
follow it but . is pretty hard to google for so I have looked up the
docs...
$ PATH=.:$PATH ./easy-build.sh -h
Usage: easy-build.sh [-h] [-d gnome|kde|lxde|xfce|light|all]
BC|NETINST|CD|DVD [ARCH ...]
works for me. So does the following I've no idea if this is correct
analysis though...
--- a/easy-build.sh
+++ b/easy-build.sh
@@ -11,7 +11,7 @@ show_usage() {
# Set configuration file to be used for the build and source it
-export CF=CONF.sh
+export CF=./CONF.sh
. $CF
export DEBIAN_CD_CONF_SOURCED=true
unset UPDATE_LOCAL
--
Ian Campbell
Current Noise: Black Label Society - Bleed For Me
First law of debate:
Never argue with a fool. People might not know the difference.
--
To UNSUBSCRIBE, email to debian-cd-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Re: [PATCH 2/4] easy-build.sh: use getopts instead of rolling our own option parsing.
On Fri, 2009-08-07 at 13:45 +0100, Ian Campbell wrote:
On Fri, 2009-08-07 at 13:33 +0100, Ian Campbell wrote:
I just tried it and it looks like the script is already not
dash-ready:
$ dash -x ./easy-build.sh -h
+ set -e
+ export CF=CONF.sh
+ . CONF.sh
.: 1: CONF.sh: not found
I'm no expert but it looks like dash obeys $PATH when executing the ..
The manpage doesn't mention this behaviour and bash doesn't seem to
follow it but . is pretty hard to google for so I have looked up the
haven't
docs...
$ PATH=.:$PATH ./easy-build.sh -h
Usage: easy-build.sh [-h] [-d gnome|kde|lxde|xfce|light|all]
BC|NETINST|CD|DVD [ARCH ...]
works for me. So does the following I've no idea if this is correct
analysis though...
--- a/easy-build.sh
+++ b/easy-build.sh
@@ -11,7 +11,7 @@ show_usage() {
# Set configuration file to be used for the build and source it
-export CF=CONF.sh
+export CF=./CONF.sh
. $CF
export DEBIAN_CD_CONF_SOURCED=true
unset UPDATE_LOCAL
--
Ian Campbell
Current Noise: Black Label Society - Bleed For Me
First law of debate:
Never argue with a fool. People might not know the difference.
--
Ian Campbell
Current Noise: Black Label Society - Lords Of Destruction
In Brooklyn, we had such great pennant races, it made the World Series
just something that came later.
-- Walter O'Malley, Dodgers owner
--
To UNSUBSCRIBE, email to debian-cd-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Re: [PATCH 2/4] easy-build.sh: use getopts instead of rolling our own option parsing.
Ian Campbell wrote: On Fri, 2009-08-07 at 13:33 +0100, Ian Campbell wrote: I just tried it and it looks like the script is already not dash-ready: $ dash -x ./easy-build.sh -h + set -e + export CF=CONF.sh + . CONF.sh .: 1: CONF.sh: not found I'm no expert but it looks like dash obeys $PATH when executing the .. The manpage doesn't mention this behaviour and bash doesn't seem to follow it but . is pretty hard to google for so I have looked up the docs... dot should look after PATH, see POSIX: http://www.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#dot so dash is correcter. ciao cate -- To UNSUBSCRIBE, email to debian-cd-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Re: [PATCH 2/4] easy-build.sh: use getopts instead of rolling our own option parsing.
Hi, one nitpick: On Fri, Aug 7, 2009 at 13:33:33 +0100, Ian Campbell wrote: +shift $(expr $OPTIND - 1) maybe 'shift $((OPTIND - 1))' so you don't fork expr? Cheers, Julien -- To UNSUBSCRIBE, email to debian-cd-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
