[PATCH] Convert isatty() calls to git_isatty()

[PATCH] Convert isatty() calls to git_isatty()

[Nguyễn Thái Ngọc Duy]

isatty() is used to check for interactive use cases. However if pager is
set up, standard file handles may be redirected and istty() calls later
on no longer reflect the original state.

Convert isatty() calls to git_isatty() and allow git_isatty() to cache
tty info before pager is set up.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/commit.c         |    2 +-
 builtin/fsck.c           |    2 +-
 builtin/merge.c          |    4 ++--
 builtin/pack-objects.c   |    2 +-
 builtin/pack-redundant.c |    2 +-
 builtin/prune-packed.c   |    2 +-
 builtin/prune.c          |    2 +-
 builtin/revert.c         |    2 +-
 builtin/shortlog.c       |    4 ++--
 builtin/unpack-objects.c |    2 +-
 cache.h                  |    1 +
 color.c                  |    2 +-
 pager.c                  |   17 +++++++++++++++++
 transport.c              |    4 ++--
 14 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index eba1377..cc72f13 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -672,7 +672,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 		strbuf_addbuf(&sb, &message);
 		hook_arg1 = "message";
 	} else if (logfile && !strcmp(logfile, "-")) {
-		if (isatty(0))
+		if (git_isatty(0))
 			fprintf(stderr, _("(reading log message from standard input)\n"));
 		if (strbuf_read(&sb, 0, 0) < 0)
 			die_errno(_("could not read log from standard input"));
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 8c479a7..0b4e8cf 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -637,7 +637,7 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix, fsck_opts, fsck_usage, 0);
 
 	if (show_progress == -1)
-		show_progress = isatty(2);
+		show_progress = git_isatty(2);
 	if (verbose)
 		show_progress = 0;
 
diff --git a/builtin/merge.c b/builtin/merge.c
index 62c7b68..49b9176 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -714,7 +714,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
 
 		o.renormalize = option_renormalize;
 		o.show_rename_progress =
-			show_progress == -1 ? isatty(2) : show_progress;
+			show_progress == -1 ? git_isatty(2) : show_progress;
 
 		for (x = 0; x < xopts_nr; x++)
 			if (parse_merge_opt(&o, xopts[x]))
@@ -1129,7 +1129,7 @@ static int default_edit_option(void)
 	/* Use editor if stdin and stdout are the same and is a tty */
 	return (!fstat(0, &st_stdin) &&
 		!fstat(1, &st_stdout) &&
-		isatty(0) &&
+		git_isatty(0) &&
 		st_stdin.st_dev == st_stdout.st_dev &&
 		st_stdin.st_ino == st_stdout.st_ino &&
 		st_stdin.st_mode == st_stdout.st_mode);
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 0f2e7b8..4468c84 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2328,7 +2328,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 	if (!pack_compression_seen && core_compression_seen)
 		pack_compression_level = core_compression_level;
 
-	progress = isatty(2);
+	progress = git_isatty(2);
 	for (i = 1; i < argc; i++) {
 		const char *arg = argv[i];
 
diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
index f5c6afc..8197c9e 100644
--- a/builtin/pack-redundant.c
+++ b/builtin/pack-redundant.c
@@ -646,7 +646,7 @@ int cmd_pack_redundant(int argc, const char **argv, const char *prefix)
 
 	/* ignore objects given on stdin */
 	llist_init(&ignore);
-	if (!isatty(0)) {
+	if (!git_isatty(0)) {
 		while (fgets(buf, sizeof(buf), stdin)) {
 			sha1 = xmalloc(20);
 			if (get_sha1_hex(buf, sha1))
diff --git a/builtin/prune-packed.c b/builtin/prune-packed.c
index f9463de..498c6e9 100644
--- a/builtin/prune-packed.c
+++ b/builtin/prune-packed.c
@@ -71,7 +71,7 @@ void prune_packed_objects(int opts)
 
 int cmd_prune_packed(int argc, const char **argv, const char *prefix)
 {
-	int opts = isatty(2) ? VERBOSE : 0;
+	int opts = git_isatty(2) ? VERBOSE : 0;
 	const struct option prune_packed_options[] = {
 		OPT_BIT('n', "dry-run", &opts, "dry run", DRY_RUN),
 		OPT_NEGBIT('q', "quiet", &opts, "be quiet", VERBOSE),
diff --git a/builtin/prune.c b/builtin/prune.c
index 58d7cb8..821772e 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -158,7 +158,7 @@ int cmd_prune(int argc, const char **argv, const char *prefix)
 	}
 
 	if (show_progress == -1)
-		show_progress = isatty(2);
+		show_progress = git_isatty(2);
 	if (show_progress)
 		progress = start_progress_delay("Checking connectivity", 0, 0, 2);
 
diff --git a/builtin/revert.c b/builtin/revert.c
index e6840f2..6f87c9b 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -199,7 +199,7 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
 	int res;
 
 	memset(&opts, 0, sizeof(opts));
-	if (isatty(0))
+	if (git_isatty(0))
 		opts.edit = 1;
 	opts.action = REPLAY_REVERT;
 	git_config(git_default_config, NULL);
diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index 37f3193..c77b472 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -289,10 +289,10 @@ parse_done:
 	log.abbrev = rev.abbrev;
 
 	/* assume HEAD if from a tty */
-	if (!nongit && !rev.pending.nr && isatty(0))
+	if (!nongit && !rev.pending.nr && git_isatty(0))
 		add_head_to_pending(&rev);
 	if (rev.pending.nr == 0) {
-		if (isatty(0))
+		if (git_isatty(0))
 			fprintf(stderr, _("(reading log message from standard input)\n"));
 		read_from_stdin(&log);
 	}
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 14e04e6..4aaba69 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -501,7 +501,7 @@ int cmd_unpack_objects(int argc, const char **argv, const char *prefix)
 
 	git_config(git_default_config, NULL);
 
-	quiet = !isatty(2);
+	quiet = !git_isatty(2);
 
 	for (i = 1 ; i < argc; i++) {
 		const char *arg = argv[i];
diff --git a/cache.h b/cache.h
index 9bd8c2d..4073fc9 100644
--- a/cache.h
+++ b/cache.h
@@ -1176,6 +1176,7 @@ extern void setup_pager(void);
 extern const char *pager_program;
 extern int pager_in_use(void);
 extern int pager_use_color;
+extern int git_isatty(int);
 
 extern const char *editor_program;
 extern const char *askpass_program;
diff --git a/color.c b/color.c
index e8e2681..7151b48 100644
--- a/color.c
+++ b/color.c
@@ -183,7 +183,7 @@ int git_config_colorbool(const char *var, const char *value)
 static int check_auto_color(void)
 {
 	if (color_stdout_is_tty < 0)
-		color_stdout_is_tty = isatty(1);
+		color_stdout_is_tty = git_isatty(1);
 	if (color_stdout_is_tty || (pager_in_use() && pager_use_color)) {
 		char *term = getenv("TERM");
 		if (term && strcmp(term, "dumb"))
diff --git a/pager.c b/pager.c
index 975955b..a9380ab 100644
--- a/pager.c
+++ b/pager.c
@@ -72,12 +72,17 @@ const char *git_pager(int stdout_is_tty)
 void setup_pager(void)
 {
 	const char *pager = git_pager(isatty(1));
+	int i;
 
 	if (!pager)
 		return;
 
 	setenv("GIT_PAGER_IN_USE", "true", 1);
 
+	/* cache tty info */
+	for (i = 0; i <= 2; i++)
+		git_isatty(i);
+
 	/* spawn the pager */
 	pager_argv[0] = pager;
 	pager_process.use_shell = 1;
@@ -110,3 +115,15 @@ int pager_in_use(void)
 	env = getenv("GIT_PAGER_IN_USE");
 	return env ? git_config_bool("GIT_PAGER_IN_USE", env) : 0;
 }
+
+int git_isatty(int fd)
+{
+	static int tty[3] = { -1, -1, -1 };
+
+	if (fd < 0 || fd > 2)
+		return isatty(fd);
+
+	if (tty[fd] == -1)
+		tty[fd] = isatty(fd);
+	return tty[fd];
+}
diff --git a/transport.c b/transport.c
index cac0c06..af48f7c 100644
--- a/transport.c
+++ b/transport.c
@@ -880,7 +880,7 @@ struct transport *transport_get(struct remote *remote, const char *url)
 	const char *helper;
 	struct transport *ret = xcalloc(1, sizeof(*ret));
 
-	ret->progress = isatty(2);
+	ret->progress = git_isatty(2);
 
 	if (!remote)
 		die("No remote provided to transport_get()");
@@ -997,7 +997,7 @@ void transport_set_verbosity(struct transport *transport, int verbosity,
 	 *   2. Don't report progress, if verbosity < 0 (ie. -q/--quiet ).
 	 *   3. Report progress if isatty(2) is 1.
 	 **/
-	transport->progress = force_progress || (verbosity >= 0 && isatty(2));
+	transport->progress = force_progress || (verbosity >= 0 && git_isatty(2));
 }
 
 int transport_push(struct transport *transport,
-- 
1.7.8.36.g69ee2

Re: [PATCH] Convert isatty() calls to git_isatty()

[Johannes Sixt]

Am 2/3/2012 9:35, schrieb Nguyễn Thái Ngọc Duy:
> isatty() is used to check for interactive use cases. However if pager is
> set up, standard file handles may be redirected and istty() calls later
> on no longer reflect the original state.

So what? What's wrong with this behavior?

You converted many cases involving progress indicators. Wouldn't the new
code pipe progress output to the pager where earlier it was not shown if a
pager was present? That is plainly wrong: Progress output is destined only
for the terminal, not for the pager.

-- Hannes

Re: [PATCH] Convert isatty() calls to git_isatty()

[Nguyen Thai Ngoc Duy]

2012/2/3 Johannes Sixt <j.sixt@viscovery.net>:
> Am 2/3/2012 9:35, schrieb Nguyễn Thái Ngọc Duy:
>> isatty() is used to check for interactive use cases. However if pager is
>> set up, standard file handles may be redirected and istty() calls later
>> on no longer reflect the original state.
>
> So what? What's wrong with this behavior?
>
> You converted many cases involving progress indicators. Wouldn't the new
> code pipe progress output to the pager where earlier it was not shown if a
> pager was present? That is plainly wrong: Progress output is destined only
> for the terminal, not for the pager.

Yeah right. My mistake.
-- 
Duy