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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux