Am 29.08.2013 15:05, schrieb Matthieu Moy:
> The --for-status option was an undocumented option used only by
> wt-status.c, which inserted a header and commented out the output. We can
> achieve the same result within wt-status.c, without polluting the
> submodule command-line options.
>
> This will make it easier to disable the comments from wt-status.c later.
Cool, thanks for implementing this!
But unfortunately this change collides with bc/submodule-status-ignored
(I added Brian to the CC) which is currently on its way to next. Your
patch will break the fix in the second commit, because that's only
enabled when the submodule script sees the --for-status option.
A solution for that would be to rebase your patches on top of pu, drop
the first two hunks of the change to git-submodule.sh and still pass
the --for-status option to git-submodule.sh. This would move adding the
comment characters into wt-status.c but will still enable the script to
honor the ignore=all setting when called by status.
Junio, what is our take on changing behavior of undocumented internally
used options? Do we have to add a new --for-status2 (which doesn't add
the comment characters) or is it ok to just change the behavior of the
existing option?
> Signed-off-by: Matthieu Moy <matthieu....@imag.fr>
> ---
> git-submodule.sh | 17 +----------------
> t/t7401-submodule-summary.sh | 13 -------------
> wt-status.c | 29 +++++++++++++++++++++++++++--
> 3 files changed, 28 insertions(+), 31 deletions(-)
>
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 2979197..fccdec9 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -965,7 +965,6 @@ set_name_rev () {
> #
> cmd_summary() {
> summary_limit=-1
> - for_status=
> diff_cmd=diff-index
>
> # parse $args after "submodule ... summary".
> @@ -978,9 +977,6 @@ cmd_summary() {
> --files)
> files="$1"
> ;;
> - --for-status)
> - for_status="$1"
> - ;;
> -n|--summary-limit)
> summary_limit="$2"
> isnumber "$summary_limit" || usage
> @@ -1149,18 +1145,7 @@ cmd_summary() {
> echo
> fi
> echo
> - done |
> - if test -n "$for_status"; then
> - if [ -n "$files" ]; then
> - gettextln "Submodules changed but not updated:" | git
> stripspace -c
> - else
> - gettextln "Submodule changes to be committed:" | git
> stripspace -c
> - fi
> - printf "\n" | git stripspace -c
> - git stripspace -c
> - else
> - cat
> - fi
> + done
> }
> #
> # List all submodules, prefixed with:
> diff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh
> index ac2434c..b435d03 100755
> --- a/t/t7401-submodule-summary.sh
> +++ b/t/t7401-submodule-summary.sh
> @@ -262,19 +262,6 @@ EOF
> test_cmp expected actual
> "
>
> -test_expect_success '--for-status' "
> - git submodule summary --for-status HEAD^ >actual &&
> - test_i18ncmp actual - <<EOF
> -# Submodule changes to be committed:
> -#
> -# * sm1 $head6...0000000:
> -#
> -# * sm2 0000000...$head7 (2):
> -# > Add foo9
> -#
> -EOF
> -"
> -
> test_expect_success 'fail when using --files together with --cached' "
> test_must_fail git submodule summary --files --cached
> "
> diff --git a/wt-status.c b/wt-status.c
> index 958a53c..d91661d 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -665,6 +665,10 @@ static void wt_status_print_submodule_summary(struct
> wt_status *s, int uncommitt
> char index[PATH_MAX];
> const char *env[] = { NULL, NULL };
> struct argv_array argv = ARGV_ARRAY_INIT;
> + struct strbuf cmd_stdout = STRBUF_INIT;
> + struct strbuf summary = STRBUF_INIT;
> + char *summary_content;
> + size_t len;
>
> sprintf(summary_limit, "%d", s->submodule_summary);
> snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", s->index_file);
> @@ -673,7 +677,6 @@ static void wt_status_print_submodule_summary(struct
> wt_status *s, int uncommitt
> argv_array_push(&argv, "submodule");
> argv_array_push(&argv, "summary");
> argv_array_push(&argv, uncommitted ? "--files" : "--cached");
> - argv_array_push(&argv, "--for-status");
> argv_array_push(&argv, "--summary-limit");
> argv_array_push(&argv, summary_limit);
> if (!uncommitted)
> @@ -685,9 +688,31 @@ static void wt_status_print_submodule_summary(struct
> wt_status *s, int uncommitt
> sm_summary.git_cmd = 1;
> sm_summary.no_stdin = 1;
> fflush(s->fp);
> - sm_summary.out = dup(fileno(s->fp)); /* run_command closes it */
> + sm_summary.out = -1;
> +
> run_command(&sm_summary);
> argv_array_clear(&argv);
> +
> + len = strbuf_read(&cmd_stdout, sm_summary.out, 1024);
> +
> + /* prepend header, only if there's an actual output */
> + if (len) {
> + if (uncommitted)
> + strbuf_addstr(&summary, _("Submodules changed but not
> updated:"));
> + else
> + strbuf_addstr(&summary, _("Submodule changes to be
> committed:"));
> + strbuf_addstr(&summary, "\n\n");
> + }
> + strbuf_addbuf(&summary, &cmd_stdout);
> + strbuf_release(&cmd_stdout);
> +
> + summary_content = strbuf_detach(&summary, &len);
> + strbuf_add_commented_lines(&summary, summary_content, len);
> + free(summary_content);
> +
> + summary_content = strbuf_detach(&summary, &len);
> + fprintf(s->fp, summary_content);
> + free(summary_content);
> }
>
> static void wt_status_print_other(struct wt_status *s,
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
