public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox•com>
To: Jeff King <peff@peff•net>
Cc: lars.schneider@autodesk•com, git@vger•kernel.org,
	sbeller@google•com, sunshine@sunshineco•com,
	kaartic.sivaraam@gmail•com, sandals@crustytoothpaste•net,
	Lars Schneider <larsxschneider@gmail•com>
Subject: Re: [PATCH v4 2/2] launch_editor(): indicate that Git waits for user input
Date: Sat, 02 Dec 2017 21:15:29 -0800	[thread overview]
Message-ID: <xmqqvahojssu.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20171130205137.GC3313@sigill.intra.peff.net> (Jeff King's message of "Thu, 30 Nov 2017 15:51:37 -0500")

Jeff King <peff@peff•net> 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.

  parent reply	other threads:[~2017-12-03  5:15 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-29 14:37 [PATCH v4 0/2] launch_editor(): indicate that Git waits for user input lars.schneider
2017-11-29 14:37 ` [PATCH v4 1/2] refactor "dumb" terminal determination lars.schneider
2017-11-30 20:30   ` Jeff King
2017-12-01  3:26   ` Kaartic Sivaraam
2017-11-29 14:37 ` [PATCH v4 2/2] launch_editor(): indicate that Git waits for user input lars.schneider
2017-11-30 20:51   ` Jeff King
2017-12-01  3:56     ` Kaartic Sivaraam
2017-12-01 12:52     ` Lars Schneider
2017-12-01 18:29       ` Jeff King
2017-12-02  3:45         ` Kaartic Sivaraam
2017-12-03 16:39           ` Lars Schneider
2017-12-04 16:41             ` Kaartic Sivaraam
2017-12-04 17:26             ` Jeff King
2017-12-04 21:31               ` Lars Schneider
2017-12-04 21:42                 ` Jeff King
2017-12-04 21:54                   ` Lars Schneider
2017-12-04 22:09                     ` Jeff King
2017-12-04 17:25           ` Jeff King
2017-12-03  5:15     ` Junio C Hamano [this message]
2017-12-03 12:47       ` Lars Schneider
2017-12-04 17:32         ` Jeff King
2017-12-04 21:34           ` Lars Schneider
2017-12-04 21:40           ` Junio C Hamano
2017-12-04 17:30       ` Jeff King
2017-11-29 18:35 ` [PATCH v4 0/2] " Thomas Adam
2017-11-30 13:55   ` Lars Schneider
2017-11-30 14:42     ` Thomas Adam
2017-11-30 15:13       ` Andreas Schwab
2017-12-01  3:41         ` Kaartic Sivaraam
2017-11-30 20:12   ` Jeff King
2017-11-30 20:51     ` Thomas Adam

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=xmqqvahojssu.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=kaartic.sivaraam@gmail$(echo .)com \
    --cc=lars.schneider@autodesk$(echo .)com \
    --cc=larsxschneider@gmail$(echo .)com \
    --cc=peff@peff$(echo .)net \
    --cc=sandals@crustytoothpaste$(echo .)net \
    --cc=sbeller@google$(echo .)com \
    --cc=sunshine@sunshineco$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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