repo-add: move command invocation out of arg parsing loop
On Thu, Jun 23, 2011 at 11:10 PM, Dave Reisner <d@falconindy.com> wrote:
> Forwarding this, as it seems to have been eaten by gmail as well on the
> initial send...
>
> d
>
> ----- Forwarded message from Dave Reisner <d@falconindy.com> -----
>
>> Date: Wed, 22 Jun 2011 20:38:52 -0400
>> From: Dave Reisner <d@falconindy.com>
>> To: pacman-dev@archlinux.org
>> Cc: Dave Reisner <d@falconindy.com>
>> 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 <d@falconindy.com>
>> ---
>> *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 -----
>
>
|
