Re: [PATCH] Simplified the invocation of command action in submodule

From: Imran M Yousuf <imyousuf@gmail.com>
Date: Jan 9, 2008, 09:51 UTC
Message-ID: <7bfdc29a0801090151k22d3cd5aqedb0d4860868d4d9@mail.gmail.com>
In-Reply-To: <47848CDD.7050806@viscovery.net>

On Jan 9, 2008 2:59 PM, Johannes Sixt <j.sixt@viscovery.net> wrote:

Show 77 quoted lines

> imyousuf@gmail.com schrieb:
>
> > @@ -16,6 +16,7 @@ update=
> >  status=
> >  quiet=
> >  cached=
> > +command=
> >
> >  #
> >  # print stuff on stdout unless -q was specified
> > @@ -293,20 +294,23 @@ modules_list()
> >       done
> >  }
> >
> > +# command specifies the whole function name since
> > +# one of theirs prefix is module not modules
> >  while test $# != 0
> >  do
> >       case "$1" in
> >       add)
> >               add=1
> > +             command="module_$1"
> >               ;;
> >       init)
> > -             init=1
> > +             command="modules_$1"
> >               ;;
> >       update)
> > -             update=1
> > +             command="modules_$1"
> >               ;;
> >       status)
> > -             status=1
> > +             command="modules_list"
> >               ;;
> >       -q|--quiet)
> >               quiet=1
> > @@ -320,7 +324,7 @@ do
> >               branch="$2"; shift
> >               ;;
> >       --cached)
> > -             cached=1
> > +             command="modules_list"
>
> Don't remove cached=1 because otherwise --cached is effectively ignored.
>
> >               ;;
> >       --)
> >               break
> > @@ -345,20 +349,8 @@ case "$add,$branch" in
> >       ;;
> >  esac
> >
> > -case "$add,$init,$update,$status,$cached" in
> > -1,,,,)
> > -     module_add "$@"
> > -     ;;
> > -,1,,,)
> > -     modules_init "$@"
> > -     ;;
> > -,,1,,)
> > -     modules_update "$@"
> > -     ;;
> > -,,,*,*)
> > -     modules_list "$@"
> > -     ;;
> > -*)
> > +if [ -z $command ]; then
> >       usage
> > -     ;;
> > -esac
> > +else
> > +     "$command" "$@"
> > +fi
>
> - Previously 'git submodule' was equvalent to 'git submodule status', now
> it is an error.

Yes, I forgot to add that status is the default command. Thanks for pointing it out.

>
> - Previously, passing --cached to add, init, or update was an error, now
> it is not.

The usage statement and this behaviour is rather contradicting. The usage says that --cached can be used with all commands; so I am not sure whether using --cached with add should be an error or not. IMHO, if the previous implementation was right than the USAGE has to be changed, and if the previous implementation was incorrect, than if the default command is set to status than current implementation is right.

I would like to get comment on this until I fix the patch and resend it.

>
> -- Hannes
>

Thank you,

-- 
Imran M Yousuf