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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 28.08.2013 14:47, schrieb Matthieu Moy:
> Signed-off-by: Matthieu Moy <Matthieu.Moy@xxxxxxx>
> ---
> (--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
> 

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]