public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox•com>
To: Stefan Beller <sbeller@google•com>
Cc: git@vger•kernel.org
Subject: Re: [PATCH 02/12] git submodule update: Announce uninitialized modules on stderr
Date: Fri, 16 Oct 2015 13:54:29 -0700	[thread overview]
Message-ID: <xmqq7fmmedq2.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <1444960333-16003-3-git-send-email-sbeller@google.com> (Stefan Beller's message of "Thu, 15 Oct 2015 18:52:03 -0700")

Stefan Beller <sbeller@google•com> writes:

> Signed-off-by: Stefan Beller <sbeller@google•com>
> ---
>  git-submodule.sh           |  2 +-
>  t/t7400-submodule-basic.sh | 12 ++++++------
>  2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 578ec48..eea27f8 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -693,7 +693,7 @@ cmd_update()
>  			# Only mention uninitialized submodules when its
>  			# path have been specified
>  			test "$#" != "0" &&
> -			say "$(eval_gettext "Submodule path '\$displaypath' not initialized
> +			say >&2 "$(eval_gettext "Submodule path '\$displaypath' not initialized
>  Maybe you want to use 'update --init'?")"
>  			continue
>  		fi

There are quite a few other calls to "say" in this script, and you
are changing only this one to emit it to the standard error output.

My quick eyeballing of the script tells me that most of them, other
than the ones that are used in cmd_status to report the information
that the user asked to be shown on the standard output, are of "Now
I am doing this" kind fo output, which I feel are the same category
as this one that shouldn't be on the standard output.

Another thing (which relates to the one in 01/12) is that not all
output from this command comes from "say".

Perhaps the first thing to do before doing 01/12 is to sift these
messages into types and have them consistently use helpers designed
for different purposes, e.g.

 - a progress, like this one, the one in 01/12, and many other uses
   of "say"; which may want to become e.g. "say_progress".

 - an error or a warning, like "Could not remove working tree", "not
   initialized, maybe you want to do 'init' first?"; which may want
   to become something else e.g. "say_warning".

 - the real output from the program, e.g. output from cmd_status,
   would use yet another, e.g. "printf '%s\n'".

instead of converting each message that you happened to have noticed.

Note that "say" is squelched under GIT_QUIET (i.e. --quiet).  The
former two helpers we would want to make quiet (or for errors we may
not---I don't know offhand).  I do not think of any valid reason why
we want to squelch the output from cmd_status under --quiet; it is
not like the the while loop on the downstream of "list |" pipe tells
some status via its exit code.

  reply	other threads:[~2015-10-16 20:54 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-16  1:52 [RFC PATCHv1 00/12] git submodule update in C with parallel cloning Stefan Beller
2015-10-16  1:52 ` [PATCH 01/12] git submodule update: Announce skipping submodules on stderr Stefan Beller
2015-10-16 20:37   ` Junio C Hamano
2015-10-16 20:47     ` Stefan Beller
2015-10-16  1:52 ` [PATCH 02/12] git submodule update: Announce uninitialized modules " Stefan Beller
2015-10-16 20:54   ` Junio C Hamano [this message]
2015-10-16  1:52 ` [PATCH 03/12] git submodule update: Move branch calculation to where it's needed Stefan Beller
2015-10-16 20:54   ` Junio C Hamano
2015-10-16  1:52 ` [PATCH 04/12] git submodule update: Announce outcome of submodule operation to stderr Stefan Beller
2015-10-16  1:52 ` [PATCH 05/12] git submodule update: Use its own list implementation Stefan Beller
2015-10-16 21:02   ` Junio C Hamano
2015-10-16 21:08     ` Stefan Beller
2015-10-16  1:52 ` [PATCH 06/12] git submodule update: Handle unmerged submodules in C Stefan Beller
2015-10-20 21:11   ` Junio C Hamano
2015-10-20 21:21     ` Stefan Beller
2015-10-16  1:52 ` [PATCH 07/12] submodule config: keep update strategy around Stefan Beller
2015-10-16  1:52 ` [PATCH 08/12] git submodule update: check for "none" in C Stefan Beller
2015-10-16  1:52 ` [PATCH 09/12] git submodule update: Check url " Stefan Beller
2015-10-16  1:52 ` [PATCH 10/12] git submodule update: Clone projects from within C Stefan Beller
2015-10-16  1:52 ` [PATCH 11/12] submodule--helper: Do not emit submodules to process directly Stefan Beller
2015-10-16  1:52 ` [PATCH 12/12] WIP/broken Clone all outstanding submodules in parallel Stefan Beller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq7fmmedq2.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=sbeller@google$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox