Re: [PATCH v4 2/2] launch_editor(): indicate that Git waits for user input

From: Jeff King <peff@peff.net>
Date: Dec 1, 2017, 18:29 UTC
Message-ID: <20171201182924.GB27688@sigill.intra.peff.net>
In-Reply-To: <BB878570-F214-4BAA-AB58-4A9C0F8F80AB@gmail.com>

On Fri, Dec 01, 2017 at 01:52:14PM +0100, Lars Schneider wrote:

Show 10 quoted lines

> > These are obviously the result of devils-advocate poking at the feature.
> > I doubt any editor would end its output with a CR. But the first case is
> > probably going to be common, especially for actual graphical editors. We
> > know that emacsclient prints its own line, and I wouldn't be surprised
> > if other graphical editors spew some telemetry to stderr (certainly
> > anything built against GTK tends to do so).
> 
> True! However, if a Git user is not bothered by a graphical editor that
> spews telemetry to stderr, then I think the same user wouldn't be
> bothered by one additional line printed by Git either, right?

Yeah, if there's a lot of spew, I agree it probably doesn't matter.

The "emacsclient" one is probably the worst case, because it would print a single line of its own, which would get tacked onto the "Waiting..." message printed by Git, since it doesn't end with a newline.

> > The patch itself looks fine, as far as correctly implementing the
> > design.
> 
> Thanks for the review :-)

Actually, I meant to bikeshed one part but forgot. ;)

> +                       fprintf(stderr, _("hint: Waiting for your editor input..."));

I found "waiting for editor input" to be a funny way of saying this. I input to the editor, the editor does not input to Git. :)

Maybe "waiting for your editor finish" or something would make more sense?

Or given that the goal is really just making it clear that we've spawned an editor, something like "starting editor %s..." would work. I think the "waiting for..." pattern is perfectly fine, though.

-Peff