Re: [PATCH] git-submodule: respect -q for add/update

[Jens Lehmann <Jens.Lehmann@web.de>]

Am 04.09.2012 09:31, schrieb Orgad Shaneh:
> Signed-off-by: Orgad Shaneh <orgads@gmail.com>

Before the "Signed-off-by" is the place where you should have
explained why this would be a worthwhile change ;-)

To me this looks like you make the default noisier and require an
explicit "-q" to make it quiet again. There is a reason you don't
normally get bothered with the output of the checkout command run
under the hood of git submodule add/update, so I don't think this
change makes things better.

But you might want to think about adding a "-v/--verbose" flag to
make the submodule add/update checkouts more verbose, in case you
care about the output of the checkout command. That would be a
sane thing to do, so what about changing your patch into this
direction?

> ---
>  git-submodule.sh |   17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/git-submodule.sh b/git-submodule.sh
> index aac575e..dd57abb 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -266,6 +266,11 @@ cmd_add()
>  
>  	repo=$1
>  	sm_path=$2
> +	quiet=
> +	if test -n "$GIT_QUIET"
> +	then
> +		quiet=-q
> +	fi
>  
>  	if test -z "$sm_path"; then
>  		sm_path=$(echo "$repo" |
> @@ -332,8 +337,8 @@ Use -f if you really want to add it." >&2
>  			cd "$sm_path" &&
>  			# ash fails to wordsplit ${branch:+-b "$branch"...}
>  			case "$branch" in
> -			'') git checkout -f -q ;;
> -			?*) git checkout -f -q -B "$branch" "origin/$branch" ;;
> +			'') git checkout -f $quiet ;;
> +			?*) git checkout -f $quiet -B "$branch" "origin/$branch" ;;
>  			esac
>  		) || die "$(eval_gettext "Unable to checkout submodule '\$sm_path'")"
>  	fi
> @@ -527,6 +532,12 @@ cmd_update()
>  		shift
>  	done
>  
> +	quiet=
> +	if test -n "$GIT_QUIET"
> +	then
> +		quiet=-q
> +	fi
> +
>  	if test -n "$init"
>  	then
>  		cmd_init "--" "$@" || return
> @@ -619,7 +630,7 @@ Maybe you want to use 'update --init'?")"
>  				must_die_on_failure=yes
>  				;;
>  			*)
> -				command="git checkout $subforce -q"
> +				command="git checkout $subforce $quiet"
>  				die_msg="$(eval_gettext "Unable to checkout '\$sha1' in submodule path '\$sm_path'")"
>  				say_msg="$(eval_gettext "Submodule path '\$sm_path': checked out '\$sha1'")"
>  				;;
>