Git Mailing List Archive mirror
 help / color / mirror / Atom feed
From: "Rubén Justo" <rjusto@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Git List <git@vger.kernel.org>
Subject: Re: [PATCH v2] launch_editor: waiting message on error
Date: Fri, 12 Apr 2024 01:18:55 +0200	[thread overview]
Message-ID: <d485c4cd-f963-45b7-9fa6-801738c7c066@gmail.com> (raw)
In-Reply-To: <xmqqle5lxlwm.fsf@gitster.g>

On Wed, Apr 10, 2024 at 08:44:25AM -0700, Junio C Hamano wrote:

> > My concern is that perhaps term_clear_line() might clear some useful
> > information for the user.  Although I am not sure that this concern is
> > sensible.
> 
> It happens ONLY when the error message the editor itself emits
> (which comes later on the same line as "We are waiting for your
> editor...") without terminating newline itself.  Otherwise, we'd
> have
> 
>     We are waiting ... THE EDITOR SAYS I FAILED
>     _
> 
> on the screen, and the cursor is at the _ position.  term_clear_line()
> would not clear anything.

Not with a careless editor:

--- >8 ---
#include <stdio.h>
#include <stdlib.h>

int main(void)
{
	fprintf("editing the file...");
	exit(1); /* unexpected exit */
	fprintf("\n");
	return 0;
}
--- 8< ---

> > Stepping back a bit, how painful it would be to drop the
> > term_clear_line() and start using advice_if_enabled() here?
> >
> > This is what I'm thinking about now.
> >
> > 	$ GIT_EDITOR=false git commit -a
> > 	hint: A-good-explanation-to-say-we-run-'editor'
> > 	hint: Disable this message with "git config advice.waitingForEditor false"
> > 	error: There was a problem with the editor 'false'.
> > 	Please supply the message using either -m or -F option.
> 
> I do not think the problem is in the case where the editor
> immediately exits with an error.  It is when the editor opens
> elsewhere (or more likely, opens another tab to let you edit in an
> existing GUI editor session, but the editor's window is buried under
> other windows) and your "git commit" will stay silently without
> giving you back a terminal prompt, leaving you wondering why it is
> taking so much time.

Yes ...

>
> So I am not sure if the advice mechanism is a good fit.

and I'm not sure either.

However, two things that I like: by using the advice mechanism we avoid
the term_clear_line() and we advertise better the knob.  I discovered
advice.waitingForEditor while inspecting the code.

Using the advice mechanism here is just a thought.

> > 	$ GIT_EDITOR=false git commit -a
> > 	hint: Waiting for your editor to close the file... error: There was a problem with the editor 'false'.
> > 	Please supply the message using either -m or -F option.

> I do not think we want to encourage "-m" when the end user did not
> say so.  Instead we should let them fix their editor to keep them
> more productive.

That message can be traced back to 62e09ce998 (Make git tag a builtin.,
2007-07-20).  My understanding is that we suggest -m or -F because using
the editor failed.

Are you saying we should stop giving that "Please supply ..."?

I see that message when I'm editing a commit message and decide to abort
the commit by making the editor end with an error.  So, that message is
misleading to me.

Therefore I'm fine with removing that message, however, I'm not sure
that's what you're suggesting.

At any rate, this is tangential to this series.

  reply	other threads:[~2024-04-11 23:18 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-08 21:02 [PATCH] launch_editor: waiting message on error Rubén Justo
2024-04-08 21:07 ` Rubén Justo
2024-04-08 23:09   ` [PATCH v2] " Rubén Justo
2024-04-09  1:27     ` Junio C Hamano
2024-04-09 23:38       ` Rubén Justo
2024-04-10 15:44         ` Junio C Hamano
2024-04-11 23:18           ` Rubén Justo [this message]
2024-04-12 15:46             ` Junio C Hamano
2024-04-12 17:03               ` Rubén Justo
2024-04-12 17:35                 ` Junio C Hamano
2024-04-12 18:24                   ` Rubén Justo
2024-04-12 17:05     ` [PATCH v3 0/2] launch_editor: waiting message Rubén Justo
2024-04-12 17:15       ` [PATCH v3 1/2] launch_editor: waiting for editor message Rubén Justo
2024-04-12 17:24         ` rsbecker
2024-04-12 17:37           ` Rubén Justo
2024-04-12 17:47             ` rsbecker
2024-04-13 15:06           ` Phillip Wood
2024-04-12 17:15       ` [PATCH v3 2/2] launch_editor: waiting message on error Rubén Justo
2024-04-13 15:09         ` Phillip Wood
2024-04-14  7:23           ` Rubén Justo
2024-04-14  7:39       ` [PATCH v4] " Rubén Justo
2024-04-15 14:05         ` Phillip Wood
2024-04-15 17:03           ` Rubén Justo
2024-04-15 17:20           ` Junio C Hamano
2024-04-15 17:07         ` Rubén Justo
2024-04-15 18:44           ` 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=d485c4cd-f963-45b7-9fa6-801738c7c066@gmail.com \
    --to=rjusto@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).