All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Sunshine <sunshine@sunshineco.com>
To: Jeff King <peff@peff.net>
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 21:36:42 -0500	[thread overview]
Message-ID: <CAPig+cT+YfgBG3Aqszp+y7iy_megboECZy3NkMqUjBj7=Z661A@mail.gmail.com> (raw)
In-Reply-To: <YaaN0pibKWgjcVk3@coredump.intra.peff.net>

On Tue, Nov 30, 2021 at 3:47 PM Jeff King <peff@peff.net> wrote:
> On Tue, Nov 30, 2021 at 09:05:54AM -0500, Eric Sunshine wrote:
> > (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:
>
> 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/

Thanks for the reference. I'll take a stab at adding a blurb about
this to CodingGuidelines.

> > (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.

That's a good point about them being marked for translation. It also
reminds me that we made some reasonably significant changes to this
exact message in 2c27002a0a (worktree: improve message when creating a
new worktree, 2018-04-24), and we didn't hear any complaints, let
alone complaints about tool breakage.

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

Nice. This and the above make me feel much more comfortable with the
idea of changing git-worktree to send these sorts of messages to
stderr rather than stdout.

> > 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. :)

Thanks. I'll drop this RFC patch and resubmit with a patch which just
fixes git-worktree to be chatty on stderr instead of stdout.

  reply	other threads:[~2021-12-01  2:36 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     ` [RFC PATCH] vreportf: ensure sensible ordering of normal and error output Jeff King
2021-12-01  2:36       ` Eric Sunshine [this message]
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='CAPig+cT+YfgBG3Aqszp+y7iy_megboECZy3NkMqUjBj7=Z661A@mail.gmail.com' \
    --to=sunshine@sunshineco.com \
    --cc=bmburstein@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --cc=rsbecker@nexbridge.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.