{"thread":{"id":"29533","subject":"[PATCH] Convert isatty() calls to git_isatty()","startedAt":"2012-02-03T08:35:01Z","lastAt":"2012-02-03T09:59:59Z","messageCount":3,"participants":["Nguyễn Thái Ngọc Duy","Johannes Sixt","Nguyen Thai Ngoc Duy"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"183701","messageId":"1328258101-10636-1-git-send-email-pclouds@gmail.com","threadId":"29533","inReplyTo":null,"subject":"[PATCH] Convert isatty() calls to git_isatty()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-02-03T08:35:01Z","receivedAt":"2012-02-03T08:35:01Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"isatty() is used to check for interactive use cases. However if pager is\nset up, standard file handles may be redirected and istty() calls later\non no longer reflect the original state.\n\nConvert isatty() calls to git_isatty() and allow git_isatty() to cache\ntty info before pager is set up.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n builtin/commit.c         |    2 +-\n builtin/fsck.c           |    2 +-\n builtin/merge.c          |    4 ++--\n builtin/pack-objects.c   |    2 +-\n builtin/pack-redundant.c |    2 +-\n builtin/prune-packed.c   |    2 +-\n builtin/prune.c          |    2 +-\n builtin/revert.c         |    2 +-\n builtin/shortlog.c       |    4 ++--\n builtin/unpack-objects.c |    2 +-\n cache.h                  |    1 +\n color.c                  |    2 +-\n pager.c                  |   17 +++++++++++++++++\n transport.c              |    4 ++--\n 14 files changed, 33 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex eba1377..cc72f13 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -672,7 +672,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\tstrbuf_addbuf(&sb, &message);\n \t\thook_arg1 = \"message\";\n \t} else if (logfile && !strcmp(logfile, \"-\")) {\n-\t\tif (isatty(0))\n+\t\tif (git_isatty(0))\n \t\t\tfprintf(stderr, _(\"(reading log message from standard input)\\n\"));\n \t\tif (strbuf_read(&sb, 0, 0) < 0)\n \t\t\tdie_errno(_(\"could not read log from standard input\"));\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex 8c479a7..0b4e8cf 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -637,7 +637,7 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, prefix, fsck_opts, fsck_usage, 0);\n \n \tif (show_progress == -1)\n-\t\tshow_progress = isatty(2);\n+\t\tshow_progress = git_isatty(2);\n \tif (verbose)\n \t\tshow_progress = 0;\n \ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 62c7b68..49b9176 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -714,7 +714,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,\n \n \t\to.renormalize = option_renormalize;\n \t\to.show_rename_progress =\n-\t\t\tshow_progress == -1 ? isatty(2) : show_progress;\n+\t\t\tshow_progress == -1 ? git_isatty(2) : show_progress;\n \n \t\tfor (x = 0; x < xopts_nr; x++)\n \t\t\tif (parse_merge_opt(&o, xopts[x]))\n@@ -1129,7 +1129,7 @@ static int default_edit_option(void)\n \t/* Use editor if stdin and stdout are the same and is a tty */\n \treturn (!fstat(0, &st_stdin) &&\n \t\t!fstat(1, &st_stdout) &&\n-\t\tisatty(0) &&\n+\t\tgit_isatty(0) &&\n \t\tst_stdin.st_dev == st_stdout.st_dev &&\n \t\tst_stdin.st_ino == st_stdout.st_ino &&\n \t\tst_stdin.st_mode == st_stdout.st_mode);\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 0f2e7b8..4468c84 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -2328,7 +2328,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \tif (!pack_compression_seen && core_compression_seen)\n \t\tpack_compression_level = core_compression_level;\n \n-\tprogress = isatty(2);\n+\tprogress = git_isatty(2);\n \tfor (i = 1; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n \ndiff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c\nindex f5c6afc..8197c9e 100644\n--- a/builtin/pack-redundant.c\n+++ b/builtin/pack-redundant.c\n@@ -646,7 +646,7 @@ int cmd_pack_redundant(int argc, const char **argv, const char *prefix)\n \n \t/* ignore objects given on stdin */\n \tllist_init(&ignore);\n-\tif (!isatty(0)) {\n+\tif (!git_isatty(0)) {\n \t\twhile (fgets(buf, sizeof(buf), stdin)) {\n \t\t\tsha1 = xmalloc(20);\n \t\t\tif (get_sha1_hex(buf, sha1))\ndiff --git a/builtin/prune-packed.c b/builtin/prune-packed.c\nindex f9463de..498c6e9 100644\n--- a/builtin/prune-packed.c\n+++ b/builtin/prune-packed.c\n@@ -71,7 +71,7 @@ void prune_packed_objects(int opts)\n \n int cmd_prune_packed(int argc, const char **argv, const char *prefix)\n {\n-\tint opts = isatty(2) ? VERBOSE : 0;\n+\tint opts = git_isatty(2) ? VERBOSE : 0;\n \tconst struct option prune_packed_options[] = {\n \t\tOPT_BIT('n', \"dry-run\", &opts, \"dry run\", DRY_RUN),\n \t\tOPT_NEGBIT('q', \"quiet\", &opts, \"be quiet\", VERBOSE),\ndiff --git a/builtin/prune.c b/builtin/prune.c\nindex 58d7cb8..821772e 100644\n--- a/builtin/prune.c\n+++ b/builtin/prune.c\n@@ -158,7 +158,7 @@ int cmd_prune(int argc, const char **argv, const char *prefix)\n \t}\n \n \tif (show_progress == -1)\n-\t\tshow_progress = isatty(2);\n+\t\tshow_progress = git_isatty(2);\n \tif (show_progress)\n \t\tprogress = start_progress_delay(\"Checking connectivity\", 0, 0, 2);\n \ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex e6840f2..6f87c9b 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -199,7 +199,7 @@ int cmd_revert(int argc, const char **argv, const char *prefix)\n \tint res;\n \n \tmemset(&opts, 0, sizeof(opts));\n-\tif (isatty(0))\n+\tif (git_isatty(0))\n \t\topts.edit = 1;\n \topts.action = REPLAY_REVERT;\n \tgit_config(git_default_config, NULL);\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex 37f3193..c77b472 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -289,10 +289,10 @@ parse_done:\n \tlog.abbrev = rev.abbrev;\n \n \t/* assume HEAD if from a tty */\n-\tif (!nongit && !rev.pending.nr && isatty(0))\n+\tif (!nongit && !rev.pending.nr && git_isatty(0))\n \t\tadd_head_to_pending(&rev);\n \tif (rev.pending.nr == 0) {\n-\t\tif (isatty(0))\n+\t\tif (git_isatty(0))\n \t\t\tfprintf(stderr, _(\"(reading log message from standard input)\\n\"));\n \t\tread_from_stdin(&log);\n \t}\ndiff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c\nindex 14e04e6..4aaba69 100644\n--- a/builtin/unpack-objects.c\n+++ b/builtin/unpack-objects.c\n@@ -501,7 +501,7 @@ int cmd_unpack_objects(int argc, const char **argv, const char *prefix)\n \n \tgit_config(git_default_config, NULL);\n \n-\tquiet = !isatty(2);\n+\tquiet = !git_isatty(2);\n \n \tfor (i = 1 ; i < argc; i++) {\n \t\tconst char *arg = argv[i];\ndiff --git a/cache.h b/cache.h\nindex 9bd8c2d..4073fc9 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1176,6 +1176,7 @@ extern void setup_pager(void);\n extern const char *pager_program;\n extern int pager_in_use(void);\n extern int pager_use_color;\n+extern int git_isatty(int);\n \n extern const char *editor_program;\n extern const char *askpass_program;\ndiff --git a/color.c b/color.c\nindex e8e2681..7151b48 100644\n--- a/color.c\n+++ b/color.c\n@@ -183,7 +183,7 @@ int git_config_colorbool(const char *var, const char *value)\n static int check_auto_color(void)\n {\n \tif (color_stdout_is_tty < 0)\n-\t\tcolor_stdout_is_tty = isatty(1);\n+\t\tcolor_stdout_is_tty = git_isatty(1);\n \tif (color_stdout_is_tty || (pager_in_use() && pager_use_color)) {\n \t\tchar *term = getenv(\"TERM\");\n \t\tif (term && strcmp(term, \"dumb\"))\ndiff --git a/pager.c b/pager.c\nindex 975955b..a9380ab 100644\n--- a/pager.c\n+++ b/pager.c\n@@ -72,12 +72,17 @@ const char *git_pager(int stdout_is_tty)\n void setup_pager(void)\n {\n \tconst char *pager = git_pager(isatty(1));\n+\tint i;\n \n \tif (!pager)\n \t\treturn;\n \n \tsetenv(\"GIT_PAGER_IN_USE\", \"true\", 1);\n \n+\t/* cache tty info */\n+\tfor (i = 0; i <= 2; i++)\n+\t\tgit_isatty(i);\n+\n \t/* spawn the pager */\n \tpager_argv[0] = pager;\n \tpager_process.use_shell = 1;\n@@ -110,3 +115,15 @@ int pager_in_use(void)\n \tenv = getenv(\"GIT_PAGER_IN_USE\");\n \treturn env ? git_config_bool(\"GIT_PAGER_IN_USE\", env) : 0;\n }\n+\n+int git_isatty(int fd)\n+{\n+\tstatic int tty[3] = { -1, -1, -1 };\n+\n+\tif (fd < 0 || fd > 2)\n+\t\treturn isatty(fd);\n+\n+\tif (tty[fd] == -1)\n+\t\ttty[fd] = isatty(fd);\n+\treturn tty[fd];\n+}\ndiff --git a/transport.c b/transport.c\nindex cac0c06..af48f7c 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -880,7 +880,7 @@ struct transport *transport_get(struct remote *remote, const char *url)\n \tconst char *helper;\n \tstruct transport *ret = xcalloc(1, sizeof(*ret));\n \n-\tret->progress = isatty(2);\n+\tret->progress = git_isatty(2);\n \n \tif (!remote)\n \t\tdie(\"No remote provided to transport_get()\");\n@@ -997,7 +997,7 @@ void transport_set_verbosity(struct transport *transport, int verbosity,\n \t *   2. Don't report progress, if verbosity < 0 (ie. -q/--quiet ).\n \t *   3. Report progress if isatty(2) is 1.\n \t **/\n-\ttransport->progress = force_progress || (verbosity >= 0 && isatty(2));\n+\ttransport->progress = force_progress || (verbosity >= 0 && git_isatty(2));\n }\n \n int transport_push(struct transport *transport,\n-- \n1.7.8.36.g69ee2\n"},{"id":"183703","messageId":"4F2BAD8B.1080403@viscovery.net","threadId":"29533","inReplyTo":"1328258101-10636-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] Convert isatty() calls to git_isatty()","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2012-02-03T09:48:59Z","receivedAt":"2012-02-03T09:48:59Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 2/3/2012 9:35, schrieb Nguyễn Thái Ngọc Duy:\n> isatty() is used to check for interactive use cases. However if pager is\n> set up, standard file handles may be redirected and istty() calls later\n> on no longer reflect the original state.\n\nSo what? What's wrong with this behavior?\n\nYou converted many cases involving progress indicators. Wouldn't the new\ncode pipe progress output to the pager where earlier it was not shown if a\npager was present? That is plainly wrong: Progress output is destined only\nfor the terminal, not for the pager.\n\n-- Hannes\n"},{"id":"183704","messageId":"CACsJy8CHwSyWqMOB9LKxK2fqa0gP7TamnesMTR-D2Tny57C=Vg@mail.gmail.com","threadId":"29533","inReplyTo":"4F2BAD8B.1080403@viscovery.net","subject":"Re: [PATCH] Convert isatty() calls to git_isatty()","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-02-03T09:59:59Z","receivedAt":"2012-02-03T09:59:59Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"2012/2/3 Johannes Sixt <j.sixt@viscovery.net>:\n> Am 2/3/2012 9:35, schrieb Nguyễn Thái Ngọc Duy:\n>> isatty() is used to check for interactive use cases. However if pager is\n>> set up, standard file handles may be redirected and istty() calls later\n>> on no longer reflect the original state.\n>\n> So what? What's wrong with this behavior?\n>\n> You converted many cases involving progress indicators. Wouldn't the new\n> code pipe progress output to the pager where earlier it was not shown if a\n> pager was present? That is plainly wrong: Progress output is destined only\n> for the terminal, not for the pager.\n\nYeah right. My mistake.\n-- \nDuy\n"}]}