Re: [RFC/PATCH v2 2/3] submodule: introduce --[no-]display-comment-char

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

Am 28.08.2013 14:47, schrieb Matthieu Moy:
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
> (--for-status was undocumented, so I didn't bother documenting this either)

That's fine, as both if these options will - hopefully - only be used
by wt-status.c internally.

But I'm not terribly happy about having the --for-status option in the
submodule script in the first place, as I believe it should rather be
handled by wt-status.c itself (reading the output of submodule summary
using the start_command()/finish_command() combo instead of the simple
run_command() currently used and prepending the comment char by using
status_printf() & friends for each line).

Having said that (and currently not having the time to do the above
myself), if we go along the route you are currently taking I'd prefer
to only add one other to-be-obsoleted-some-time-later option to the
submodule script. So what about only adding --no-display-comment-char
(and maybe call it --drop-status-comment-char or such to make meaning
and context a bit more obvious ;-)?

>  git-submodule.sh | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 2979197..ac0fdad 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -966,6 +966,7 @@ set_name_rev () {
>  cmd_summary() {
>  	summary_limit=-1
>  	for_status=
> +	display_comment_char=t
>  	diff_cmd=diff-index
>  
>  	# parse $args after "submodule ... summary".
> @@ -981,6 +982,12 @@ cmd_summary() {
>  		--for-status)
>  			for_status="$1"
>  			;;
> +		--display-comment-char)
> +			display_comment_char=t
> +			;;
> +		--no-display-comment-char)
> +			display_comment_char=
> +			;;
>  		-n|--summary-limit)
>  			summary_limit="$2"
>  			isnumber "$summary_limit" || usage
> @@ -1151,13 +1158,18 @@ cmd_summary() {
>  		echo
>  	done |
>  	if test -n "$for_status"; then
> +		if test -n "$display_comment_char"; then
> +			filter_cmd='git stripspace -c'
> +		else
> +			filter_cmd=cat
> +		fi
>  		if [ -n "$files" ]; then
> -			gettextln "Submodules changed but not updated:" | git stripspace -c
> +			gettextln "Submodules changed but not updated:" | $filter_cmd
>  		else
> -			gettextln "Submodule changes to be committed:" | git stripspace -c
> +			gettextln "Submodule changes to be committed:" | $filter_cmd
>  		fi
> -		printf "\n" | git stripspace -c
> -		git stripspace -c
> +		printf "\n" | $filter_cmd
> +		$filter_cmd
>  	else
>  		cat
>  	fi
>