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

2017-12-04 Thread Jeff King
On Mon, Dec 04, 2017 at 10:54:40PM +0100, Lars Schneider wrote:

> > Better, IMHO, though I still think literally saying:
> > 
> >  hint: Waiting for your editor to exit...
> > 
> > is the most accurate, which I think makes it clear that you must _exit_
> > your editor, not just save and close the file.
> 
> I think "exit" would be confusing because most graphical editors (Sublime,
> Textmate, Notepad++, ...) can open multiple files and do not need to exit. 
> The requirement is indeed save and close the file.
> 
> How about:
> 
> hint: Waiting for your editor to close the file...
> 
> I generally like that as this is technical correct from all angles.
> My only nit would be that "the file" is a bit imprecise... but
> that's probably no problem.

OK, that makes sense. There are two definitions of "exit" here. We care
about the process exiting, but of course the editor may present a
concept of "exiting" to the user that is different.

I know emacsclient behaves that way (the client process exits when the
buffer is closed, even though the rest of emacs may still be running),
but I didn't realize other editors did so. I know atom doesn't, but then
it also exits immediately, making it inappropriate for $GIT_EDITOR in
the first place.

-Peff


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

2017-12-04 Thread Lars Schneider

> On 04 Dec 2017, at 22:42, Jeff King  wrote:
> 
> On Mon, Dec 04, 2017 at 10:31:15PM +0100, Lars Schneider wrote:
> 
 I would like to add "for your input" or "for you" to convey 
 that Git is not waiting for the machine but for the user.
 
   "hint: Launched editor. Waiting for your input..."
 
 Would that work for you?
>>> 
>>> I guess "input" was the part that I found funny/confusing. The only
>>> thing we know is that we're waiting on the editor process to finish, and
>>> everything else is making assumptions about what's happening in the
>>> editor.
>> 
>> I see. How about:
>> 
>> "hint: Launched editor. Waiting for your action..."
>> (my preference)
>> 
>> or
>> 
>> "hint: Launched editor. Waiting for you..."
> 
> Better, IMHO, though I still think literally saying:
> 
>  hint: Waiting for your editor to exit...
> 
> is the most accurate, which I think makes it clear that you must _exit_
> your editor, not just save and close the file.

I think "exit" would be confusing because most graphical editors (Sublime,
Textmate, Notepad++, ...) can open multiple files and do not need to exit. 
The requirement is indeed save and close the file.

How about:

hint: Waiting for your editor to close the file...

I generally like that as this is technical correct from all angles.
My only nit would be that "the file" is a bit imprecise... but
that's probably no problem.

- Lars


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

2017-12-04 Thread Jeff King
On Mon, Dec 04, 2017 at 10:31:15PM +0100, Lars Schneider wrote:

> >> I would like to add "for your input" or "for you" to convey 
> >> that Git is not waiting for the machine but for the user.
> >> 
> >>"hint: Launched editor. Waiting for your input..."
> >> 
> >> Would that work for you?
> > 
> > I guess "input" was the part that I found funny/confusing. The only
> > thing we know is that we're waiting on the editor process to finish, and
> > everything else is making assumptions about what's happening in the
> > editor.
> 
> I see. How about:
> 
> "hint: Launched editor. Waiting for your action..."
> (my preference)
> 
> or
> 
> "hint: Launched editor. Waiting for you..."

Better, IMHO, though I still think literally saying:

  hint: Waiting for your editor to exit...

is the most accurate, which I think makes it clear that you must _exit_
your editor, not just save and close the file.

I dunno, maybe that is being overly paranoid. Certainly I have seen
graphical programs that have a mismatch with the one-process-per-action
way that most terminal editors view the world, and would hang around
even after the user thinks they are done editing. But at the same time,
those programs are unlikely to work well as $GIT_EDITOR in the first
place, because running them from the terminal may just open a new window
in an existing session and exit immediately (which is the opposite
problem -- the editor exited before the user actually did their thing).

So I'm not sure if that would be a problem in practice or not. I'm too
mired in the vim world to have any real data. Somebody like you who is
supporting a large number of less-Unixy users probably has more
perspective there.

-Peff


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

2017-12-04 Thread Junio C Hamano
Jeff King  writes:

> On Sun, Dec 03, 2017 at 01:47:04PM +0100, Lars Schneider wrote:
>
>> I am on the fence here. I like consistency but I don't want to
>> bother Git users.
>> 
>> @Peff: Do you lean into either direction? Could you imagine that
>> novice/regular users are bothered? (I don't expect experts to be
>> bothered too much as they would likely be able to find and set 
>> the advice config).
>
> I also am on the fence, and am OK with any of the options discussed.
>
> But now I've said my reservations on the list, so I can say "I told you
> so" if people complain (and naturally refuse to admit my objections if
> people love it). :)

Heh.  I am even OK with not doing anything for that matter ;-)



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

2017-12-04 Thread Lars Schneider

> On 04 Dec 2017, at 18:32, Jeff King  wrote:
> 
> On Sun, Dec 03, 2017 at 01:47:04PM +0100, Lars Schneider wrote:
> 
>> I am on the fence here. I like consistency but I don't want to
>> bother Git users.
>> 
>> @Peff: Do you lean into either direction? Could you imagine that
>> novice/regular users are bothered? (I don't expect experts to be
>> bothered too much as they would likely be able to find and set 
>> the advice config).
> 
> I also am on the fence, and am OK with any of the options discussed.
> 
> But now I've said my reservations on the list, so I can say "I told you
> so" if people complain (and naturally refuse to admit my objections if
> people love it). :)

Well, since you and I are on the fence, I will follow Junio. Then we
can at least argue that the feature is consistent on all terminals.

- Lars


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

2017-12-04 Thread Lars Schneider

> On 04 Dec 2017, at 18:26, Jeff King  wrote:
> 
> On Sun, Dec 03, 2017 at 05:39:10PM +0100, Lars Schneider wrote:
> 
> +   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?
>>> 
>>> May be the good "Launched editor. Waiting ..." message, that was used in a 
>>> previous version, itself makes sense?
>> 
>> Perfect bikeshed topic :-)
>> 
>> I would like to add "for your input" or "for you" to convey 
>> that Git is not waiting for the machine but for the user.
>> 
>>"hint: Launched editor. Waiting for your input..."
>> 
>> Would that work for you?
> 
> I guess "input" was the part that I found funny/confusing. The only
> thing we know is that we're waiting on the editor process to finish, and
> everything else is making assumptions about what's happening in the
> editor.

I see. How about:

"hint: Launched editor. Waiting for your action..."
(my preference)

or

"hint: Launched editor. Waiting for you..."

- Lars

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

2017-12-04 Thread Jeff King
On Sun, Dec 03, 2017 at 01:47:04PM +0100, Lars Schneider wrote:

> I am on the fence here. I like consistency but I don't want to
> bother Git users.
> 
> @Peff: Do you lean into either direction? Could you imagine that
> novice/regular users are bothered? (I don't expect experts to be
> bothered too much as they would likely be able to find and set 
> the advice config).

I also am on the fence, and am OK with any of the options discussed.

But now I've said my reservations on the list, so I can say "I told you
so" if people complain (and naturally refuse to admit my objections if
people love it). :)

-Peff


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

2017-12-04 Thread Jeff King
On Sat, Dec 02, 2017 at 09:15:29PM -0800, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > I tried to think of ways this "show a message and then delete it" could
> > go wrong. It should work OK with editors that just do curses-like
> > things, taking over the terminal and then restoring it at the end.
> > ...
> 
> I think that it is not worth to special-case "dumb" terminals like
> this round of the patches do.  If it so much disturbs reviewers that
> "\e[K" may not work everywhere, we can do without the "then delete
> it" part.  It was merely trying to be extra nice, and the more
> important part of the "feature" is to be noticeable, and I do think
> that not showing anything on "dumb", only because the message cannot
> be retracted, is putting the cart before the horse.  
> 
> Since especially now people are hiding this behind an advise.*
> thing, I think it is OK to show a message and waste a line, even.

Yeah, I was tempted to suggest just dropping this terminal magic
completely. But it probably _does_ work and is helpful in the majority
of cases (i.e., where people have in-terminal editors). I dunno.

I am a little wary of hiding behind "but you can disable it with a
config option", because that's still a thing that users have to actually
do to get the previous behavior. And I expect to get some "ugh, git is
too chatty and annoying" backlash once this is in a released version.

But maybe that is just being paranoid. It's not like we don't have a lot
of other advice flags. I really could go either way on this whole thing
(but I'll be setting the advice flag myself ;) ).

> > An even worse case (and yes, this is really reaching) is:
> >
> >   $ GIT_EDITOR='echo one; printf "two\\r"; vim' git commit
> >   hint: Waiting for your editor input...one
> >   Aborting commit due to empty commit message.
> >
> > There we ate the "two" line.
> 
> Yes, I would have to agree that this one is reaching, as there isn't
> any valid reason other than "the editor then wanted to do \e[K
> later" for it to end its last line with CR.  So our eating that line
> is not a problem.

Yeah, this was just me trying to come up with all possible implications.
I agree it's probably not worth worrying about.

-Peff


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

2017-12-04 Thread Jeff King
On Sun, Dec 03, 2017 at 05:39:10PM +0100, Lars Schneider wrote:

> >>> +   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?
> > 
> > May be the good "Launched editor. Waiting ..." message, that was used in a 
> > previous version, itself makes sense?
> 
> Perfect bikeshed topic :-)
> 
> I would like to add "for your input" or "for you" to convey 
> that Git is not waiting for the machine but for the user.
> 
> "hint: Launched editor. Waiting for your input..."
> 
> Would that work for you?

I guess "input" was the part that I found funny/confusing. The only
thing we know is that we're waiting on the editor process to finish, and
everything else is making assumptions about what's happening in the
editor.

-Peff


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

2017-12-04 Thread Jeff King
On Sat, Dec 02, 2017 at 09:15:49AM +0530, Kaartic Sivaraam wrote:

> > Or given that the goal is really just making it clear that we've spawned
> > an editor, something like "starting editor %s..." would work.
> 
> There was already discussion related to the "continuous tense" used in the
> phrase.
> 
> Extract from [1]:

Thanks, I missed that one. I agree with the argument there.

-Peff


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

2017-12-04 Thread Kaartic Sivaraam
On Sun, 2017-12-03 at 17:39 +0100, Lars Schneider wrote:
> > On 02 Dec 2017, at 04:45, Kaartic Sivaraam  
> > wrote:
> > 
> > On Friday 01 December 2017 11:59 PM, Jeff King wrote:
> > > On Fri, Dec 01, 2017 at 01:52:14PM +0100, Lars Schneider wrote:
> > > > 
> > > > 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?
> > 
> > May be the good "Launched editor. Waiting ..." message, that was used in a 
> > previous version, itself makes sense?
> 
> Perfect bikeshed topic :-)
> 

Yep :-)


> I would like to add "for your input" or "for you" to convey 
> that Git is not waiting for the machine but for the user.
> 
> "hint: Launched editor. Waiting for your input..."
> 
> Would that work for you?
> 

Yeah, this one does look fine.

That said, FWIW I don't have strong opinions about the phrase/sentence
except that they should be readable and shouldn't get too verbose.


Thanks, 
Kaartic


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

2017-12-03 Thread Lars Schneider

> On 02 Dec 2017, at 04:45, Kaartic Sivaraam  wrote:
> 
> On Friday 01 December 2017 11:59 PM, Jeff King wrote:
>> On Fri, Dec 01, 2017 at 01:52:14PM +0100, Lars Schneider wrote:
>>> 
>>> 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?
> 
> May be the good "Launched editor. Waiting ..." message, that was used in a 
> previous version, itself makes sense?

Perfect bikeshed topic :-)

I would like to add "for your input" or "for you" to convey 
that Git is not waiting for the machine but for the user.

"hint: Launched editor. Waiting for your input..."

Would that work for you?

Thanks,
Lars

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

2017-12-03 Thread Lars Schneider

> On 03 Dec 2017, at 06:15, Junio C Hamano  wrote:
> 
> Jeff King  writes:
> 
>> I tried to think of ways this "show a message and then delete it" could
>> go wrong. It should work OK with editors that just do curses-like
>> things, taking over the terminal and then restoring it at the end.
>> ...
> 
> I think that it is not worth to special-case "dumb" terminals like
> this round of the patches do.  If it so much disturbs reviewers that
> "\e[K" may not work everywhere, we can do without the "then delete
> it" part.  It was merely trying to be extra nice, and the more
> important part of the "feature" is to be noticeable, and I do think
> that not showing anything on "dumb", only because the message cannot
> be retracted, is putting the cart before the horse.  
> 
> Since especially now people are hiding this behind an advise.*
> thing, I think it is OK to show a message and waste a line, even.

Well, my reasoning was to minimize the risk of bothering people:
People using "dumb" terminals wouldn't be bothered because nothing
changes and people using "smart" terminals would see the message
only temporarily. Of course, emacsclient users would be the
exception. They would always be bothered and therefore I added
the "advice" switch.

That being said, I can also follow your logic. If we have such
a feature then we shouldn't surprise the user. We should make it 
consistently available in all environments.

I am on the fence here. I like consistency but I don't want to
bother Git users.

@Peff: Do you lean into either direction? Could you imagine that
novice/regular users are bothered? (I don't expect experts to be
bothered too much as they would likely be able to find and set 
the advice config).


Thanks,
Lars


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

2017-12-02 Thread Junio C Hamano
Jeff King  writes:

> I tried to think of ways this "show a message and then delete it" could
> go wrong. It should work OK with editors that just do curses-like
> things, taking over the terminal and then restoring it at the end.
> ...

I think that it is not worth to special-case "dumb" terminals like
this round of the patches do.  If it so much disturbs reviewers that
"\e[K" may not work everywhere, we can do without the "then delete
it" part.  It was merely trying to be extra nice, and the more
important part of the "feature" is to be noticeable, and I do think
that not showing anything on "dumb", only because the message cannot
be retracted, is putting the cart before the horse.  

Since especially now people are hiding this behind an advise.*
thing, I think it is OK to show a message and waste a line, even.

> An even worse case (and yes, this is really reaching) is:
>
>   $ GIT_EDITOR='echo one; printf "two\\r"; vim' git commit
>   hint: Waiting for your editor input...one
>   Aborting commit due to empty commit message.
>
> There we ate the "two" line.

Yes, I would have to agree that this one is reaching, as there isn't
any valid reason other than "the editor then wanted to do \e[K
later" for it to end its last line with CR.  So our eating that line
is not a problem.


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

2017-12-01 Thread Kaartic Sivaraam

On Friday 01 December 2017 11:59 PM, Jeff King wrote:

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


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?


May be the good "Launched editor. Waiting ..." message, that was used in 
a previous version, itself makes 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.


There was already discussion related to the "continuous tense" used in 
the phrase.


Extract from [1]:

-- 8< --
> fprintf(stderr, "Launching your editor...");

"It takes quite some time to launch this special Git Editor"

As Lars pointed out, the editor may be launched in the background,
that the user would not know, but they might expect a thing to
pop up as a modal dialog as is always with UIs.

So despite it being technically wrong at this point in time,
I would phrase it in past tense or in a way that indicates that the
user needs to take action already.

The "Launching..." sounds as if I need to wait for an event to occur.
-- >8 --

[1]: 
https://public-inbox.org/git/CAGZ79kZbm8SGY4rXKZHV82E-HX9qbQ4iyCbMgJEBFQf4fj3u=q...@mail.gmail.com/




I think
the "waiting for..." pattern is perfectly fine, though.



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

2017-12-01 Thread Jeff King
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


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

2017-12-01 Thread Lars Schneider

> On 30 Nov 2017, at 21:51, Jeff King  wrote:
> 
> On Wed, Nov 29, 2017 at 03:37:52PM +0100, lars.schnei...@autodesk.com wrote:
> ...
>> The standard advise() function is not used here as it would always add
>> a newline which would make deleting the message harder.
> 
> I tried to think of ways this "show a message and then delete it" could
> go wrong. It should work OK with editors that just do curses-like
> things, taking over the terminal and then restoring it at the end.
> 
> It does behave in a funny way if the editor produces actual lines of
> output outside of the curses handling. E.g. (I just quit vim
> immediately, hence the aborting message):
> 
>  $ GIT_EDITOR='echo foo; vim' git commit
>  hint: Waiting for your editor input...foo
>  Aborting commit due to empty commit message.
> 
> our "foo" gets tacked onto the hint line, and then our deletion does
> nothing (because the newline after "foo" bumped us to a new line, and
> there was nothing on that line to erase).
> 
> An even worse case (and yes, this is really reaching) is:
> 
>  $ GIT_EDITOR='echo one; printf "two\\r"; vim' git commit
>  hint: Waiting for your editor input...one
>  Aborting commit due to empty commit message.
> 
> There we ate the "two" line.
> 
> 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?


> The patch itself looks fine, as far as correctly implementing the
> design.

Thanks for the review :-)

- Lars



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

2017-11-30 Thread Kaartic Sivaraam

On Friday 01 December 2017 02:21 AM, Jeff King 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).



Yeah, at times 'gedit' does do what you say. And if the user 
(surprisingly!) uses an IDE such as "eclipse" or a hackable text editor 
"atom" (of course with the '-f' option) for entering his commit message 
it is likely to happen all the time for him.




I don't think there's a good way around it. Portably saying "delete
_this_ line that I wrote earlier" would probably require libcurses or
similar. So maybe we just live with it. The deletion magic makes the
common cases better (a terminal editor that doesn't print random
lines, or a graphical editor that is quiet), and everyone else can flip
the advice switch if they need to. I dunno.



---
Kaartic


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

2017-11-30 Thread Jeff King
On Wed, Nov 29, 2017 at 03:37:52PM +0100, lars.schnei...@autodesk.com wrote:

> No message is printed in a "dumb" terminal as it would not be possible
> to remove the message after the editor returns. This should not be a
> problem as this feature is targeted at novice Git users and they are
> unlikely to work with a "dumb" terminal.

I think novice users could end up in this situation with something like:

  ssh remote_host git commit

But then I'd expect most terminal-based editors to give some sort of
error in that situation, too. And at any rate, the worst case is that
they get no special "waiting..." message from Git, which is already the
status quo.  So it's probably not worth worrying about such an obscure
case.

> Power users might not want to see this message or their editor might
> already print such a message (e.g. emacsclient). Allow these users to
> suppress the message by disabling the "advice.waitingForEditor" config.

I'm happy to see the hard-coded emacsclient behavior go. Hopefully we
won't see too many complaints about people having to set the advice
flag.

> The standard advise() function is not used here as it would always add
> a newline which would make deleting the message harder.

I tried to think of ways this "show a message and then delete it" could
go wrong. It should work OK with editors that just do curses-like
things, taking over the terminal and then restoring it at the end.

It does behave in a funny way if the editor produces actual lines of
output outside of the curses handling. E.g. (I just quit vim
immediately, hence the aborting message):

  $ GIT_EDITOR='echo foo; vim' git commit
  hint: Waiting for your editor input...foo
  Aborting commit due to empty commit message.

our "foo" gets tacked onto the hint line, and then our deletion does
nothing (because the newline after "foo" bumped us to a new line, and
there was nothing on that line to erase).

An even worse case (and yes, this is really reaching) is:

  $ GIT_EDITOR='echo one; printf "two\\r"; vim' git commit
  hint: Waiting for your editor input...one
  Aborting commit due to empty commit message.

There we ate the "two" line.

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

I don't think there's a good way around it. Portably saying "delete
_this_ line that I wrote earlier" would probably require libcurses or
similar. So maybe we just live with it. The deletion magic makes the
common cases better (a terminal editor that doesn't print random
lines, or a graphical editor that is quiet), and everyone else can flip
the advice switch if they need to. I dunno.

> ---
>  Documentation/config.txt |  3 +++
>  advice.c |  2 ++
>  advice.h |  1 +
>  editor.c | 15 +++
>  4 files changed, 21 insertions(+)

The patch itself looks fine, as far as correctly implementing the
design.

-Peff