[pacman-dev] Fwd: [PATCH 8/9] repo-add: move command invocation out of arg parsing loop

[Dan McGee]

On Thu, Jun 23, 2011 at 11:10 PM, Dave Reisner <d at 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 at falconindy.com> -----
>
>> Date: Wed, 22 Jun 2011 20:38:52 -0400
>> From: Dave Reisner <d at falconindy.com>
>> To: pacman-dev at archlinux.org
>> Cc: Dave Reisner <d at 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 at 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 -----
>
>