Christian Couder <christian.cou...@gmail.com> writes:

> Junio do you plan to merge this patch or would you prefer something like:
>
> +
> +Bugs & Caveats
> +--------------
> +
> +Some git commands, like `git log`, are run by default using a
> +pager. In this case, stdout and stderr are redirected to the pager and
> +are closed when the pager exits.
> +
> +If a GIT_TRACE* environment variable has been set to "1" or "2" to
> +print traces on stderr, no trace output will be printed after the
> +pager has exited.
> +
> +This can be annoying, because GIT_TRACE_PERFORMANCE by default prints
> +the performance stats for the whole command at atexit() time which
> +happens after the pager has exited.
> +
> +So the following command will print no performance stat:
> +
> +------------
> +GIT_TRACE_PERFORMANCE=2 git log -1
> +------------
> +
> +To overcome this problem, you can use for example:
> +
> +------------
> +GIT_TRACE_PERFORMANCE=2 git --no-pager log -1
> +------------
>
> ?

Should I take either one?  Which one do you prefer and why?

I do not have strong preference between the two.  I understand that
the differences are only in the example workarounds.  And I do not
think the common parts of the two patches are that great.

Even though I think the first two paragraphs do not tell a lie, I
think they are overly verbose, tell irrelevant details and does not
highlight the real issue.  You only want to say

        GIT_TRACE_* environment variables can be used to tell Git to
        show trace output to its standard error stream.  Git can
        often spawn a pager internally to run its subcommand and
        send its standard output and standard error to it.

The third paragraph is misleading.  "by default prints ... at
atexit() time" sounds as if you are hinting that the solution would
be to use some non-default way to tell it to print the stats at some
other time before atexit() to ensure that the output is done before
the pager exits, but that is not actually what you are going to
suggest.

The real source of the annoyance is not that trace output will not
be seen after the pager exits, but that PERFORMANCE trace does not
begin until the pager exits, which by definition means you would see
nothing.  "This can be annoying" is an understatement (because
sending PERFORMANCE output to pager always gives an annoying
result), and blames a wrong thing at the same time (because the fact
that trace output are sent to pager together with the program output
is not what makes it annoying; the real culprit is that PERFORMANCE
output comes only after pager exits).

I'd replace it with something like this, if I were writing this patch.

        Because GIT_TRACE_PERFORMANCE trace is generated only at the
        very end of the program with atexit(), which happens after
        the pager exits, it would not work well if you send its log
        to the standard error output and let Git spawn the pager at
        the same time.

That would make "So the following ... no performance stat:"
unnecessary, and the workarounds more obvious.  You can choose not
to use the pager, or you can send the trace output to a file.
--
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

Reply via email to