Re: [PATCH 24/30] subtree: don't let debug and progress output clash

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Apr 23, 2021, 21:07 UTC
Message-ID: <CAPig+cRaQnZ7DWYdU+efNpMgxdniXNQdF43TSFiaj6ezLELC_g@mail.gmail.com>
In-Reply-To: <20210423194230.1388945-25-lukeshu@lukeshu.com>

On Fri, Apr 23, 2021 at 3:43 PM Luke Shumaker <lukeshu@lukeshu.com> wrote:

> Currently, debug output (triggered by passing '-d') and progress output
> stomp on eachother.  The debug output is just streamed as lines to

s/eachother/each other/

Show 7 quoted lines

> stderr, and the progress output is sent to stderr as '%s\r'.  It is
> difficult to distinguish between the debug output and a progress line.
> When writing to a terminal the debug lines hide progress lines.
>
> So, when '-d' has been passed, spit out progress as 'progress: %s\n',
> instead of as '%s\r', so that it can be detected, and so that the debug
> lines don't overwrite the progress when written to a terminal.

Makes perfect sense when output is to a terminal, though might be annoying for the person who redirects stderr to a file. Just idly wondering if it makes sense to take that case into consideration... (but maybe it doesn't matter much when someone is working at debugging a problem).

Show 16 quoted lines

> Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
> ---
> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
> @@ -53,7 +53,12 @@ debug () {
>  progress () {
>         if test -z "$GIT_QUIET"
>         then
> -               printf "%s\r" "$*" >&2
> +               if test -n "$arg_debug"
> +               then
> +                       printf "progress: %s\n" "$*" >&2
> +               else
> +                       printf "%s\r" "$*" >&2
> +               fi
>         fi
>  }

Subjective (not necessarily worth changing): An `echo` would suffice in place of `printf "...\n"`:

    echo "progress: $*" >&2

It _might_ be worthwhile to have an in-code comment here explaining why progress() behaves differently in debug mode, especially if the reader is confused about why one case explicitly emits "progress:" and the other doesn't.