Re: [RFC/PATCH v2 2/3] submodule: introduce --[no-]display-comment-char
- From
Jens Lehmann <jens.lehmann@web.de>
- Date
- Aug 28, 2013, 21:07 UTC
- Message-ID
- <521E6674.1010201@web.de>
- In-Reply-To
- <1377694024-24173-2-git-send-email-Matthieu.Moy@imag.fr>
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 ;-)?
Show 52 quoted lines
> 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
>