All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Eric Sunshine <sunshine@sunshineco.com>
Cc: Git List <git@vger.kernel.org>,
	Baruch Burstein <bmburstein@gmail.com>,
	Randall Becker <rsbecker@nexbridge.com>
Subject: Re: [RFC PATCH] vreportf: ensure sensible ordering of normal and error output
Date: Tue, 30 Nov 2021 15:47:14 -0500	[thread overview]
Message-ID: <YaaN0pibKWgjcVk3@coredump.intra.peff.net> (raw)
In-Reply-To: <CAPig+cRQqm8Ce29PnkndT47NNxM3UhJv12RZGZZJD-AyGVC7Zw@mail.gmail.com>

On Tue, Nov 30, 2021 at 09:05:54AM -0500, Eric Sunshine wrote:

> >   - shouldn't status messages like this go to stderr anyway? I know some
> >     people follow the "unless it is an error, it should not to go
> >     stderr" philosophy. But I think in general our approach in Git is
> >     more "if it is the main output of the program, it goes to stdout; if
> >     it is chatter or progress for the user, it goes to stderr".
> 
> I considered this as well and agree that it would be a nicer localized
> fix, but...
> 
> (1) I don't think the practice is documented anywhere, so people --
> including me when I wrote builtin/worktree.c -- might not know about
> it. Indeed, we don't seem to be entirely consistent about doing it
> this way. Randomly picking submodule-helper.c, for instance, I see
> status-like messages going to stdout:
> 
>     printf(_("Entering '%s'\n"), displaypath);
>     printf(_("Synchronizing submodule url for '%s'\n"), ...);
> 
>     if (...)
>         format = _("Cleared directory '%s'\n");
>     else
>         format = _("Could not remove submodule work tree '%s'\n");
>     printf(format, displaypath);

Yeah, we've definitely not been consistent here. There's no silver
bullet for this aside from vigilance during review, but probably laying
out guidelines could help.

Here's a past discussion (that actually goes the other way: somebody
complaining that stderr should be on stdout!) where I laid out my mental
model:

  https://lore.kernel.org/git/20110907215716.GJ13364@sigill.intra.peff.net/

> (2) With git-worktree being four or five years old, for
> backward-compatibility concerns, I worry that "that ship has sailed",
> where 'that' is the freedom to relocate those status-like messages
> from stdout to stderr. I don't want to break tooling which exists
> around git-worktree.

IMHO it would be OK to change these. They are, after all, marked for
translation, so they're not reliably machine-readable anyway. It's
possible that some script could not be parsing them, but just trying to
redirect them. Or even keying on content in stderr as a sign of an error
(as tcl likes to do). But I don't think that's a guarantee we want to be
bound by.

See 68b939b2f0 (clone: send diagnostic messages to stderr, 2013-09-18)
for a similar case in the past.

> I'd be happy to be wrong on the second point -- indeed, git-worktree
> is still marked "experimental" in the man-page, but that may not mean
> anything this late in the game -- and submit a patch which places
> git-worktree's status-like messages on stderr instead of stdout.
> Thoughts?

I'm in favor. :)

-Peff

  parent reply	other threads:[~2021-11-30 20:47 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-30  4:39 [RFC PATCH] vreportf: ensure sensible ordering of normal and error output Eric Sunshine
2021-11-30  5:13 ` Junio C Hamano
2021-11-30  7:14   ` Jeff King
2021-11-30  7:23     ` Jeff King
2021-11-30 15:10       ` Ævar Arnfjörð Bjarmason
2021-11-30 20:52         ` Jeff King
2021-11-30 14:15     ` Eric Sunshine
2021-11-30  7:21 ` Jeff King
2021-11-30 14:05   ` Eric Sunshine
2021-11-30 14:57     ` Eric Sunshine
2021-12-01 13:51       ` "breaking" command output message parsing (was: [RFC PATCH] vreportf: ensure sensible ordering of normal and error output) Ævar Arnfjörð Bjarmason
2021-12-01 14:34         ` Eric Sunshine
2021-11-30 20:47     ` Jeff King [this message]
2021-12-01  2:36       ` [RFC PATCH] vreportf: ensure sensible ordering of normal and error output Eric Sunshine
2021-12-01  5:38         ` Eric Sunshine
2021-12-01 21:20       ` Ævar Arnfjörð Bjarmason
2021-12-02  0:43         ` Junio C Hamano

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=YaaN0pibKWgjcVk3@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=bmburstein@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=rsbecker@nexbridge.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.