All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>,
	git@vger.kernel.org,
	"Johannes Schindelin" <johannes.schindelin@gmx.de>,
	마누엘 <nalla@hamal.uberspace.de>
Subject: Re: [PATCH] clean: explicitly `fflush` stdout
Date: Wed, 08 Apr 2020 14:51:11 -0700	[thread overview]
Message-ID: <xmqqr1wxoddc.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <20200408201454.GB2270445@coredump.intra.peff.net> (Jeff King's message of "Wed, 8 Apr 2020 16:14:54 -0400")

Jeff King <peff@peff.net> writes:

> This (and the patch) make sense to me. It might be worth factoring out a
> "read input from user" function and putting the flush there, but with
> only three affected call sites, I'm OK with it either way.
>
>>     This is yet another patch that was funneled through a Git for Windows
>>     PR. It has served us well for almost five years now, and it is beyond
>>     time that it find its final home in core Git.
>
> Agreed. I guess it didn't affect people on other platforms much because
> stdout to a terminal is generally line-buffered on POSIX systems. But
> it's better not to rely on that, plus it could create confusion if
> somebody tried to manipulate the interactive operation through a pipe
> (e.g., driving it from a GUI or something).

Hmph, I thought it was more common to do prompts etc. on the
standard error stream, which tends to make the buffering of the
output less of a problem, but apparently these prompts are given to
the standard output.  I am also OK to sprinkle fflush(stdout) but in
the longer term, it would probably be a good idea to introduce a
helper to "prompt then grab input" or "read user input" (if the
former, we'd be able to bring consistency into "which between the
standard output or the standard error does a prompt come?", and if
the latter we'd do fflush(NULL) before reading), especially if there
are many git subcommands that go interactive.

Thanks.

  reply	other threads:[~2020-04-08 21:51 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-08 19:33 [PATCH] clean: explicitly `fflush` stdout Johannes Schindelin via GitGitGadget
2020-04-08 20:14 ` Jeff King
2020-04-08 21:51   ` Junio C Hamano [this message]
2020-04-08 22:38     ` Jeff King
2020-04-10 14:03   ` Johannes Schindelin
2020-04-10 11:27 ` [PATCH v2 0/2] Explicitly fflush stdout in git clean Johannes Schindelin via GitGitGadget
2020-04-10 11:27   ` [PATCH v2 1/2] Refactor code asking the user for input interactively Johannes Schindelin via GitGitGadget
2020-04-10 12:27     ` Derrick Stolee
2020-04-10 14:01       ` Johannes Schindelin
2020-04-10 15:07     ` Jeff King
2020-04-10 17:44     ` Junio C Hamano
2020-04-10 11:27   ` [PATCH v2 2/2] Explicitly `fflush` stdout before expecting interactive input 마누엘 via GitGitGadget
2020-04-10 12:28     ` Derrick Stolee
2020-04-10 18:16   ` [PATCH v3 0/2] Explicitly fflush stdout in git clean Johannes Schindelin via GitGitGadget
2020-04-10 18:16     ` [PATCH v3 1/2] Refactor code asking the user for input interactively Johannes Schindelin via GitGitGadget
2020-04-10 18:16     ` [PATCH v3 2/2] Explicitly `fflush` stdout before expecting interactive input 마누엘 via GitGitGadget

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=xmqqr1wxoddc.fsf@gitster.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=nalla@hamal.uberspace.de \
    --cc=peff@peff.net \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.