On Thu, Jun 23, 2011 at 11:10 PM, Dave Reisner <[email protected]> wrote: > Forwarding this, as it seems to have been eaten by gmail as well on the > initial send... > > d > > ----- Forwarded message from Dave Reisner <[email protected]> ----- > >> Date: Wed, 22 Jun 2011 20:38:52 -0400 >> From: Dave Reisner <[email protected]> >> To: [email protected] >> Cc: Dave Reisner <[email protected]> >> Subject: [PATCH 8/9] repo-add: move command invocation out of arg parsing >> loop >> >> This is mostly a move to make the code easier to read, but it's my feeling >> that the arg parsing loop should parse args, and the actual program logic >> should be separated from this. Agreed.
>> Signed-off-by: Dave Reisner <[email protected]> >> --- >> scripts/repo-add.sh.in | 23 ++++++++++++----------- >> 1 files changed, 12 insertions(+), 11 deletions(-) >> >> diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in >> index e75055f..8865188 100644 >> --- a/scripts/repo-add.sh.in >> +++ b/scripts/repo-add.sh.in >> @@ -529,9 +529,10 @@ trap 'trap_exit "$(gettext "TERM signal caught. >> Exiting...")"' TERM HUP QUIT >> trap 'trap_exit "$(gettext "Aborted by user! Exiting...")"' INT >> trap 'trap_exit "$(gettext "An unknown error has occured. Exiting...")"' ERR >> >> +declare -a args >> success=0 >> # parse arguments >> -while [[ $# > 0 ]]; do >> +while (( $# )); do >> case "$1" in >> -q|--quiet) QUIET=1;; >> -d|--delta) DELTA=1;; >> @@ -562,21 +563,21 @@ while [[ $# > 0 ]]; do >> VERIFY=1 >> ;; >> *) >> - if [[ -z $REPO_DB_FILE ]]; then >> - REPO_DB_FILE="$1" >> - LOCKFILE="$REPO_DB_FILE.lck" >> - check_repo_db >> - else >> - case "$cmd" in >> - repo-add) add $1 && success=1 ;; >> - repo-remove) remove $1 && success=1 ;; >> - esac >> - fi >> + args+=("$1") >> ;; >> esac >> shift >> done >> >> +REPO_DB_FILE=${args[0]} >> +LOCKFILE=$REPO_DB_FILE.lck >> +check_repo_db >> + >> +# we've already validated our command -- just trim it to get the function >> +for arg in "${args[@]:1}"; do >> + "${cmd#repo-}" "$arg" && success=1 >> +done The original case statement was so much more readable, can we just keep it something more like that please? I don't think we're going to be adding repo-elephant anytime soon. This is too much magic and someone will be unable to find where add and remove are actually called from in the script. >> + >> # if at least one operation was a success, re-zip database >> if (( success )); then >> msg "$(gettext "Creating updated database file '%s'")" "$REPO_DB_FILE" >> -- >> 1.7.5.4 >> > > ----- End forwarded message ----- > >
