{"thread":{"id":"17669","subject":"[PATCH/RFC] Add progress options","startedAt":"2009-02-09T04:54:08Z","lastAt":"2009-12-29T03:06:29Z","messageCount":12,"participants":["Brent Goodrick","Tay Ray Chuan","Johannes Schindelin","Miklos Vajna","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"103812","messageId":"18831.46833.862.196815@hungover.brentg.com","threadId":"17669","inReplyTo":null,"subject":"[PATCH/RFC] Add progress options","fromName":"Brent Goodrick","fromEmail":"bgoodr@gmail.com","sentAt":"2009-02-09T04:54:08Z","receivedAt":"2009-02-09T04:54:08Z","isPatch":true,"sender":{"key":"bgoodr@gmail.com","avatar":"https://gravatar.com/avatar/2399bf5a3468b3516a892183edfd43a7fa0300a2e9d5072189020186961150bc?d=mp&s=160"},"body":"\nAdd --progress, --no-progress, and --progress-verbose options, which\ncorrespond to the GIT_PROGRESS environment variable setting of \"1\",\n\"0\", and \"verbose\", respectively.\n\nSigned-off-by: Brent Goodrick <bgoodr@gmail.com>\n---\n\nIn reference to the \"CR codes from git commands\" thread, specifically\nhttp://marc.info/?l=git&m=123273617713654&w=2 where Dscho kindly gave\nme an outline on how to get started, which I reiterate below to show\nwhere I felt I had to deviate:\n\nDscho> \t\nDscho> Hi,\nDscho> \nDscho> On Fri, 23 Jan 2009, Brent Goodrick wrote:\nDscho> \nDscho> >  - Bare minimum: Add a new --no-cr option\nDscho> \nDscho> I do not see any value of this over \"--progress | tr '\\r' '\\n'\".  (The\nDscho> --progress option being the natural counterpart to --no-progress,\nDscho> _forcing_ the display of the progress.)\nDscho> \nDscho> And I disagree that --no-progress would be hard to implement.  Just have a\nDscho> look at 7d1864c(Introduce is_bare_repository() and core.bare configuration\nDscho> variable).\nDscho> \nDscho> Basically, you'll have to\nDscho> \nDscho> - introduce a global variable to both environment.c and\nDscho> cache.h,\nDscho> \nDscho> - set it to -1 by default,\n\nI ended up having to rely exclusively upon an environment variable for\nthis state (indicated by GIT_PROGRESS_ENVIRONMENT in my patch which is\ndefined to be \"GIT_PROGRESS\") since parts of the code are in different\nprocesses. This turned out to be a nice side-effect since I plan on\nsetting \"GIT_PROGRESS\" in my shell startup environment to be\n\"verbose\", as for my work, I do indeed want to see all progress, but\njust not all of the CR codes.\n\nDscho> \nDscho> - handle a \"--progress\" and \"--no-progress\" option in git.c, setting the\nDscho>  global variable git_show_progress to 1 or 0, respectively,\nDscho> \nDscho> - teach start_progress_delay() to return NULL if\nDscho> git_show_progress == 0,\n\nI tried that, but then I got a worse condition of not seeing any\nmessaging at all for long-running processing. This might lead people\nto think that something is hung up, and then CTRL-C'ing out, leaving\nthe repo in a bad state. What I thought we really needed instead is\njust the final \"done\" messages, but not the intervening progress\nmessages such as percentage complete messages. To that end, I allowed\nprogress.c to continue to create the SIGALRM handler and logic as\nbefore, but modify its output ever so slightly.\n\nDscho> \nDscho> - modify all users of start_progress*() to respect git_show_progress == 1,\nDscho>  which probably means to look for \"isatty\" in builtin-pack-objects.c and\nDscho>  builtin-unpack-objects.c\n\nI ended up not having to mess with isatty logic at all since only the\nlines that are emitted with the CR codes had to be addressed.\n\nDscho> \nDscho> - add documentation to Documentation/git.txt what --progress and\nDscho>  --no-progress do,\nDscho> \nDscho> - add a simple test script to t/ (maybe t/t0005-progress.sh) that tests\nDscho>  that --progress works -- maybe you find a clever way to test\nDscho>  --no-progress, too, but that would be harder, as the progress is turned\nDscho>  off by default for the scripts anyway...)\n\nI did not at this point attempt to write the test as a shell script,\nbut had to manually test it via my FSF Emacs session. There might be a\nway to test this out via some elisp code in FSF Emacs, but I doubt we\nwant to do that since that would create a dependency on Emacs in the\ngit test suite.\n\nNote that I found that the CR codes show up in git clone output in the\nEmacs compile mode (M-x compile) which are displayed as \"^M\", but not\nin the shell buffers (M-x shell).\n\nI had sent a question to the mailing list about this specific issue\nbut got no answer, so I concluded to just proceed with the PATCH/RFC\nfirst to see what folks say to the changes.\n\nThanks,\nbgoodr\n\n Documentation/git.txt                  |   15 +++++++++++++++\n builtin-pack-objects.c                 |    2 +-\n cache.h                                |    1 +\n contrib/completion/git-completion.bash |    3 +++\n git.c                                  |   10 ++++++++--\n progress.c                             |   31 ++++++++++++++++++++++++++-----\n sideband.c                             |   29 +++++++++++++++++++++++++----\n 7 files changed, 79 insertions(+), 12 deletions(-)\n\ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex 0c7bba3..2f9d4e8 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -11,6 +11,7 @@ SYNOPSIS\n [verse]\n 'git' [--version] [--exec-path[=GIT_EXEC_PATH]]\n     [-p|--paginate|--no-pager]\n+    [--progress|--no-progress|--progress-verbose]\n     [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE]\n     [--help] COMMAND [ARGS]\n \n@@ -179,6 +180,20 @@ help ...`.\n --no-pager::\n \tDo not pipe git output into a pager.\n \n+--progress::\n+\tShow progress for long-running processing. This corresponds to\n+\tsetting the GIT_PROGRESS environment variable to 1. This is\n+\tthe default.\n+\n+--no-progress::\n+\tInstead of showing intermediate progress, just show\n+\tcompletion of major steps in processing. This corresponds to\n+\tsetting the GIT_PROGRESS environment variable to 0.\n+\n+--progress-verbose::\n+\tEnable progress and show intermediate progress results as one\n+\tper line.\n+\n --git-dir=<path>::\n \tSet the path to the repository. This can also be controlled by\n \tsetting the GIT_DIR environment variable. It can be an absolute\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex cb51916..7ba5268 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -81,7 +81,7 @@ static int depth = 50;\n static int delta_search_threads;\n static int pack_to_stdout;\n static int num_preferred_base;\n-static struct progress *progress_state;\n+static struct progress *progress_state = NULL;\n static int pack_compression_level = Z_DEFAULT_COMPRESSION;\n static int pack_compression_seen;\n \ndiff --git a/cache.h b/cache.h\nindex 2d889de..6246fce 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -371,6 +371,7 @@ static inline enum object_type object_type(unsigned int mode)\n #define GITATTRIBUTES_FILE \".gitattributes\"\n #define INFOATTRIBUTES_FILE \"info/attributes\"\n #define ATTRIBUTE_MACRO_PREFIX \"[attr]\"\n+#define GIT_PROGRESS_ENVIRONMENT \"GIT_PROGRESS\"\n #define GIT_NOTES_REF_ENVIRONMENT \"GIT_NOTES_REF\"\n #define GIT_NOTES_DEFAULT_REF \"refs/notes/commits\"\n \ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 307bf5d..ca8b2d9 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1723,6 +1723,9 @@ _git ()\n \t\t--*)   __gitcomp \"\n \t\t\t--paginate\n \t\t\t--no-pager\n+\t\t\t--progress\n+\t\t\t--no-progress\n+\t\t\t--progress-verbose\n \t\t\t--git-dir=\n \t\t\t--bare\n \t\t\t--version\ndiff --git a/git.c b/git.c\nindex c2b181e..0409471 100644\n--- a/git.c\n+++ b/git.c\n@@ -5,7 +5,7 @@\n #include \"run-command.h\"\n \n const char git_usage_string[] =\n-\t\"git [--version] [--exec-path[=GIT_EXEC_PATH]] [-p|--paginate|--no-pager] [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE] [--help] COMMAND [ARGS]\";\n+\t\"git [--version] [--exec-path[=GIT_EXEC_PATH]] [-p|--paginate|--no-pager] [--progress|--no-progress|--progress-verbose] [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE] [--help] COMMAND [ARGS]\";\n \n const char git_more_info_string[] =\n \t\"See 'git help COMMAND' for more information on a specific command.\";\n@@ -116,7 +116,13 @@ static int handle_options(const char*** argv, int* argc, int* envchanged)\n \t\t\tsetenv(GIT_DIR_ENVIRONMENT, getcwd(git_dir, sizeof(git_dir)), 0);\n \t\t\tif (envchanged)\n \t\t\t\t*envchanged = 1;\n-\t\t} else {\n+\t\t} else if (!strcmp(cmd, \"--progress\"))\n+\t\t\tsetenv(GIT_PROGRESS_ENVIRONMENT, \"1\", 1);\n+\t\telse if (!strcmp(cmd, \"--no-progress\"))\n+\t\t\tsetenv(GIT_PROGRESS_ENVIRONMENT, \"0\", 1);\n+\t\telse if (!strcmp(cmd, \"--progress-verbose\"))\n+\t\t\tsetenv(GIT_PROGRESS_ENVIRONMENT, \"verbose\", 1);\n+\t\telse {\n \t\t\tfprintf(stderr, \"Unknown option: %s\\n\", cmd);\n \t\t\tusage(git_usage_string);\n \t\t}\ndiff --git a/progress.c b/progress.c\nindex 55a8687..c96d594 100644\n--- a/progress.c\n+++ b/progress.c\n@@ -10,6 +10,7 @@\n \n #include \"git-compat-util.h\"\n #include \"progress.h\"\n+#include \"cache.h\"\n \n #define TP_IDX_MAX      8\n \n@@ -36,6 +37,8 @@ struct progress {\n };\n \n static volatile sig_atomic_t progress_update;\n+static int show_progress = 1;\n+static int show_progress_verbose = 0;\n \n static void progress_interval(int signum)\n {\n@@ -90,15 +93,22 @@ static int display(struct progress *progress, unsigned n, const char *done)\n \n \tprogress->last_value = n;\n \ttp = (progress->throughput) ? progress->throughput->display : \"\";\n-\teol = done ? done : \"   \\r\";\n+\t/* When show_progress_verbose is true, put each progress message on a\n+\t * separate line: */\n+\teol = done ? done : (show_progress_verbose ? \"   \\n\" : \"   \\r\");\n \tif (progress->total) {\n \t\tunsigned percent = n * 100 / progress->total;\n \t\tif (percent != progress->last_percent || progress_update) {\n \t\t\tprogress->last_percent = percent;\n-\t\t\tfprintf(stderr, \"%s: %3u%% (%u/%u)%s%s\",\n-\t\t\t\tprogress->title, percent, n,\n-\t\t\t\tprogress->total, tp, eol);\n-\t\t\tfflush(stderr);\n+\t\t\t/* Show progress everytime if specified. But if progress is turned\n+\t\t\t * off, only show the last message (when done is non-NULL) as a\n+\t\t\t * summary: */\n+\t\t\tif (show_progress || done) {\n+\t\t\t\tfprintf(stderr, \"%s: %3u%% (%u/%u)%s%s\",\n+\t\t\t\t\t\tprogress->title, percent, n,\n+\t\t\t\t\t\tprogress->total, tp, eol);\n+\t\t\t\tfflush(stderr);\n+\t\t\t}\n \t\t\tprogress_update = 0;\n \t\t\treturn 1;\n \t\t}\n@@ -206,6 +216,17 @@ struct progress *start_progress_delay(const char *title, unsigned total,\n \t\t\t\t       unsigned percent_treshold, unsigned delay)\n {\n \tstruct progress *progress = malloc(sizeof(*progress));\n+\tconst char * show_progress_env = getenv(GIT_PROGRESS_ENVIRONMENT);\n+\tif (show_progress_env) {\n+\t\tif (!strcmp(show_progress_env, \"true\"))\n+\t\t\tshow_progress_env = \"1\";\n+\t\tif (!strcmp(show_progress_env, \"false\"))\n+\t\t\tshow_progress_env = \"0\";\n+\t\tif (!strcmp(show_progress_env, \"verbose\"))\n+\t\t\tshow_progress_verbose = 1;\n+\t\telse if (!strcmp(show_progress_env, \"0\"))\n+\t\t\tshow_progress = 0;\n+\t}\n \tif (!progress) {\n \t\t/* unlikely, but here's a good fallback */\n \t\tfprintf(stderr, \"%s...\\n\", title);\ndiff --git a/sideband.c b/sideband.c\nindex cca3360..4bfb206 100644\n--- a/sideband.c\n+++ b/sideband.c\n@@ -1,5 +1,6 @@\n #include \"pkt-line.h\"\n #include \"sideband.h\"\n+#include \"cache.h\"\n \n /*\n  * Receive multiplexed output stream over git native protocol.\n@@ -26,7 +27,19 @@ int recv_sideband(const char *me, int in_stream, int out, int err)\n \tchar buf[LARGE_PACKET_MAX + 2*FIX_SIZE];\n \tchar *suffix, *term;\n \tint skip_pf = 0;\n-\n+\tint show_progress = 1;\n+\tint show_progress_verbose = 0;\n+\tconst char * show_progress_env = getenv(GIT_PROGRESS_ENVIRONMENT);\n+\tif (show_progress_env) {\n+\t\tif (!strcmp(show_progress_env, \"true\"))\n+\t\t\tshow_progress_env = \"1\";\n+\t\tif (!strcmp(show_progress_env, \"false\"))\n+\t\t\tshow_progress_env = \"0\";\n+\t\tif (!strcmp(show_progress_env, \"0\"))\n+\t\t\tshow_progress = 0;\n+\t\telse if (!strcmp(show_progress_env, \"verbose\"))\n+\t\t\tshow_progress_verbose = 1;\n+\t}\n \tmemcpy(buf, PREFIX, pf);\n \tterm = getenv(\"TERM\");\n \tif (term && strcmp(term, \"dumb\"))\n@@ -58,6 +71,7 @@ int recv_sideband(const char *me, int in_stream, int out, int err)\n \t\t\tdo {\n \t\t\t\tchar *b = buf;\n \t\t\t\tint brk = 0;\n+\t\t\t\tint cr = 0;\n \n \t\t\t\t/*\n \t\t\t\t * If the last buffer didn't end with a line\n@@ -78,9 +92,14 @@ int recv_sideband(const char *me, int in_stream, int out, int err)\n \t\t\t\t\t\tbrk = 0;\n \t\t\t\t\t\tbreak;\n \t\t\t\t\t}\n-\t\t\t\t\tif (b[brk-1] == '\\n' ||\n-\t\t\t\t\t    b[brk-1] == '\\r')\n+\t\t\t\t\tif (b[brk-1] == '\\n')\n \t\t\t\t\t\tbreak;\n+\t\t\t\t\telse if (b[brk-1] == '\\r') {\n+\t\t\t\t\t\tif (show_progress_verbose)\n+\t\t\t\t\t\t\tb[brk-1] = '\\n';\n+\t\t\t\t\t\tcr = 1;\n+\t\t\t\t\t\tbreak;\n+\t\t\t\t\t}\n \t\t\t\t}\n \n \t\t\t\t/*\n@@ -95,7 +114,9 @@ int recv_sideband(const char *me, int in_stream, int out, int err)\n \t\t\t\t\tmemcpy(save, b + brk, sf);\n \t\t\t\t\tb[brk + sf - 1] = b[brk - 1];\n \t\t\t\t\tmemcpy(b + brk - 1, suffix, sf);\n-\t\t\t\t\tsafe_write(err, b, brk + sf);\n+\t\t\t\t\t/* Progress messages are those that end in CR codes */\n+\t\t\t\t\tif (show_progress || !cr)\n+\t\t\t\t\t\tsafe_write(err, b, brk + sf);\n \t\t\t\t\tmemcpy(b + brk, save, sf);\n \t\t\t\t\tlen -= brk;\n \t\t\t\t} else {\n-- \n1.6.2.rc0.3.ge80f6\n"},{"id":"130334","messageId":"1261761126-5784-1-git-send-email-rctay89@gmail.com","threadId":"17669","inReplyTo":"18831.46833.862.196815@hungover.brentg.com","subject":"[PATCH 0/4] clone: use --progress to mean -v","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-12-25T17:12:02Z","receivedAt":"2009-12-25T17:12:02Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"This series makes git-clone follow the \"argument convention\" of\ngit-pack-objects, where the option --progress is used to force\nreporting of reporting. This was previously done with -v/--verbose.\n\nThis in effect ensures a single consistent convention regarding\nprogress reporting for git commands. Having two conventions may\npotentially confuse users.\n\nOn a related note, Brent wrote a patch a while back [1]. This series is\nnot as ambitious his and does not deal with the main git options. In\nfact, only the last patch effects the titular change. If a consensus is\nreached on this though, I don't rule out a separate patch/series to\nset the convention at the main git level.\n\nPS. If someone can enlighten me on the proper noun for the git\n    executable (I said \"main git\"), I would be very thankful.\n\nPPS. Merry Christmas and happy holidays. :)\n\nTay Ray Chuan (4):\n  check stderr with isatty() instead of stdout when deciding to show\n    progress\n  git-clone.txt: reword description of progress behaviour\n  clone: set transport->verbose when -v/--verbose is used\n  clone: use --progress to force progress reporting\n\n Documentation/git-clone.txt |   12 +++++++++---\n builtin-clone.c             |    6 ++++++\n t/t5702-clone-options.sh    |    3 ++-\n transport-helper.c          |    2 +-\n transport.c                 |    2 +-\n transport.h                 |    2 +-\n 6 files changed, 20 insertions(+), 7 deletions(-)\n\nFootnotes:\n[1] http://marc.info/?l=git&m=123415527432713\n\n--\nCheers,\nRay Chuan\n"},{"id":"130335","messageId":"1261761126-5784-2-git-send-email-rctay89@gmail.com","threadId":"17669","inReplyTo":"1261761126-5784-1-git-send-email-rctay89@gmail.com","subject":"[PATCH 1/4] check stderr with isatty() instead of stdout when deciding to show progress","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-12-25T17:12:03Z","receivedAt":"2009-12-25T17:12:03Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Make transport code (viz. transport.c::fetch_refs_via_pack() and\ntransport-helper.c::standard_options()) that decides to show progress\ncheck if stderr is a terminal, instead of stdout. After all, progress\nreports (via the API in progress.[ch]) are sent to stderr.\n\nUpdate the documentation for git-clone to say \"standard error\" as well.\n\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n Documentation/git-clone.txt |    2 +-\n transport-helper.c          |    2 +-\n transport.c                 |    2 +-\n transport.h                 |    2 +-\n 4 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex 7ccd742..f298fdd 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -101,7 +101,7 @@ objects from the source repository into a pack in the cloned repository.\n \n --verbose::\n -v::\n-\tDisplay the progress bar, even in case the standard output is not\n+\tDisplay the progress bar, even in case the standard error is not\n \ta terminal.\n \n --no-checkout::\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 11f3d7e..b886985 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -169,7 +169,7 @@ static void standard_options(struct transport *t)\n \tchar buf[16];\n \tint n;\n \tint v = t->verbose;\n-\tint no_progress = v < 0 || (!t->progress && !isatty(1));\n+\tint no_progress = v < 0 || (!t->progress && !isatty(2));\n \n \tset_helper_option(t, \"progress\", !no_progress ? \"true\" : \"false\");\n \ndiff --git a/transport.c b/transport.c\nindex 3eea836..24c7f1d 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -476,7 +476,7 @@ static int fetch_refs_via_pack(struct transport *transport,\n \targs.include_tag = data->followtags;\n \targs.verbose = (transport->verbose > 0);\n \targs.quiet = (transport->verbose < 0);\n-\targs.no_progress = args.quiet || (!transport->progress && !isatty(1));\n+\targs.no_progress = args.quiet || (!transport->progress && !isatty(2));\n \targs.depth = data->depth;\n \n \tfor (i = 0; i < nr_heads; i++)\ndiff --git a/transport.h b/transport.h\nindex 9e74406..68fda6a 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -63,7 +63,7 @@ struct transport {\n \tint (*disconnect)(struct transport *connection);\n \tchar *pack_lockfile;\n \tsigned verbose : 3;\n-\t/* Force progress even if the output is not a tty */\n+\t/* Force progress even if stderr is not a tty */\n \tunsigned progress : 1;\n };\n \n-- \n1.6.6.278.g3f5f\n"},{"id":"130336","messageId":"1261761126-5784-3-git-send-email-rctay89@gmail.com","threadId":"17669","inReplyTo":"1261761126-5784-2-git-send-email-rctay89@gmail.com","subject":"[PATCH 2/4] git-clone.txt: reword description of progress behaviour","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-12-25T17:12:04Z","receivedAt":"2009-12-25T17:12:04Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Mention progress reporting behaviour in the descriptions for -q/\n--quiet and -v/--verbose options, in the style of git-pack-objects.txt.\n\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n Documentation/git-clone.txt |    9 ++++++---\n 1 files changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex f298fdd..e722e6c 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -96,13 +96,16 @@ objects from the source repository into a pack in the cloned repository.\n \n --quiet::\n -q::\n-\tOperate quietly.  This flag is also passed to the `rsync'\n+\tOperate quietly.  Progress is not reported to the standard\n+\terror stream. This flag is also passed to the `rsync'\n \tcommand when given.\n \n --verbose::\n -v::\n-\tDisplay the progress bar, even in case the standard error is not\n-\ta terminal.\n+\tProgress status is reported on the standard error stream\n+\tby default when it is attached to a terminal, unless -q\n+\tis specified. This flag forces progress status even if the\n+\tstandard error stream is not directed to a terminal.\n \n --no-checkout::\n -n::\n-- \n1.6.6.278.g3f5f\n"},{"id":"130337","messageId":"1261761126-5784-4-git-send-email-rctay89@gmail.com","threadId":"17669","inReplyTo":"1261761126-5784-3-git-send-email-rctay89@gmail.com","subject":"[PATCH 3/4] clone: set transport->verbose when -v/--verbose is used","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-12-25T17:12:05Z","receivedAt":"2009-12-25T17:12:05Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n builtin-clone.c |    4 +++-\n 1 files changed, 3 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex 5df8b0f..463fbe4 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -525,8 +525,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \n \t\tif (option_quiet)\n \t\t\ttransport->verbose = -1;\n-\t\telse if (option_verbose)\n+\t\telse if (option_verbose) {\n+\t\t\ttransport->verbose = 1;\n \t\t\ttransport->progress = 1;\n+\t\t}\n \n \t\tif (option_upload_pack)\n \t\t\ttransport_set_option(transport, TRANS_OPT_UPLOADPACK,\n-- \n1.6.6.278.g3f5f\n"},{"id":"130338","messageId":"1261761126-5784-5-git-send-email-rctay89@gmail.com","threadId":"17669","inReplyTo":"1261761126-5784-4-git-send-email-rctay89@gmail.com","subject":"[PATCH 4/4] clone: use --progress to force progress reporting","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-12-25T17:12:06Z","receivedAt":"2009-12-25T17:12:06Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Follow the argument convention of git-pack-objects, such that a\nseparate option (--preogress) is used to force progress reporting\ninstead of -v/--verbose.\n\n-v/--verbose now does not force progress reporting. Make git-clone.txt\nsay so.\n\nThis should cover all the bases in 21188b1 (Implement git clone -v),\nwhich implemented the option to force progress reporting.\n\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n Documentation/git-clone.txt |    3 +++\n builtin-clone.c             |    8 ++++++--\n t/t5702-clone-options.sh    |    3 ++-\n 3 files changed, 11 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex e722e6c..f43c8b2 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -102,6 +102,9 @@ objects from the source repository into a pack in the cloned repository.\n \n --verbose::\n -v::\n+\tRun verbosely.\n+\n+--progress::\n \tProgress status is reported on the standard error stream\n \tby default when it is attached to a terminal, unless -q\n \tis specified. This flag forces progress status even if the\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex 463fbe4..58bacbd 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -44,10 +44,13 @@ static char *option_origin = NULL;\n static char *option_branch = NULL;\n static char *option_upload_pack = \"git-upload-pack\";\n static int option_verbose;\n+static int option_progress;\n \n static struct option builtin_clone_options[] = {\n \tOPT__QUIET(&option_quiet),\n \tOPT__VERBOSE(&option_verbose),\n+\tOPT_BOOLEAN(0, \"progress\", &option_progress,\n+\t\t\t\"force progress reporting\"),\n \tOPT_BOOLEAN('n', \"no-checkout\", &option_no_checkout,\n \t\t    \"don't create a checkout\"),\n \tOPT_BOOLEAN(0, \"bare\", &option_bare, \"create a bare repository\"),\n@@ -525,10 +528,11 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \n \t\tif (option_quiet)\n \t\t\ttransport->verbose = -1;\n-\t\telse if (option_verbose) {\n+\t\telse if (option_verbose)\n \t\t\ttransport->verbose = 1;\n+\n+\t\tif (option_progress)\n \t\t\ttransport->progress = 1;\n-\t\t}\n \n \t\tif (option_upload_pack)\n \t\t\ttransport_set_option(transport, TRANS_OPT_UPLOADPACK,\ndiff --git a/t/t5702-clone-options.sh b/t/t5702-clone-options.sh\nindex 27825f5..02cb024 100755\n--- a/t/t5702-clone-options.sh\n+++ b/t/t5702-clone-options.sh\n@@ -27,7 +27,8 @@ test_expect_success 'redirected clone' '\n '\n test_expect_success 'redirected clone -v' '\n \n-\tgit clone -v \"file://$(pwd)/parent\" clone-redirected-v >out 2>err &&\n+\tgit clone --progress \"file://$(pwd)/parent\" clone-redirected-progress \\\n+\t\t>out 2>err &&\n \ttest -s err\n \n '\n-- \n1.6.6.278.g3f5f\n"},{"id":"130344","messageId":"alpine.DEB.1.00.0912260952020.4985@pacific.mpi-cbg.de","threadId":"17669","inReplyTo":"1261761126-5784-1-git-send-email-rctay89@gmail.com","subject":"Re: [PATCH 0/4] clone: use --progress to mean -v","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-12-26T08:53:50Z","receivedAt":"2009-12-26T08:53:50Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 26 Dec 2009, Tay Ray Chuan wrote:\n\n> This series makes git-clone follow the \"argument convention\" of \n> git-pack-objects, where the option --progress is used to force reporting \n> of reporting. This was previously done with -v/--verbose.\n\nNo objections from my side, although you might want to advertise more that \nthis is a change in behavior.  (Meaning in the release notes)\n\n> PS. If someone can enlighten me on the proper noun for the git\n>     executable (I said \"main git\"), I would be very thankful.\n\nI call it the \"Git wrapper\", although less polite words exist, too.\n\nCiao,\nDscho\n"},{"id":"130355","messageId":"20091227012032.GE25474@genesis.frugalware.org","threadId":"17669","inReplyTo":"1261761126-5784-5-git-send-email-rctay89@gmail.com","subject":"Re: [PATCH 4/4] clone: use --progress to force progress reporting","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2009-12-27T01:20:32Z","receivedAt":"2009-12-27T01:20:32Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Sat, Dec 26, 2009 at 01:12:06AM +0800, Tay Ray Chuan <rctay89@gmail.com> wrote:\n> -v/--verbose now does not force progress reporting. Make git-clone.txt\n> say so.\n> \n> This should cover all the bases in 21188b1 (Implement git clone -v),\n> which implemented the option to force progress reporting.\n> \n> Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>\n> ---\n>  Documentation/git-clone.txt |    3 +++\n>  builtin-clone.c             |    8 ++++++--\n>  t/t5702-clone-options.sh    |    3 ++-\n>  3 files changed, 11 insertions(+), 3 deletions(-)\n> \n> diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\n> index e722e6c..f43c8b2 100644\n> --- a/Documentation/git-clone.txt\n> +++ b/Documentation/git-clone.txt\n> @@ -102,6 +102,9 @@ objects from the source repository into a pack in the cloned repository.\n>  \n>  --verbose::\n>  -v::\n> +\tRun verbosely.\n\nWhat about mentioning this \"does not force progress reporting\" behaviour\nin documentation of the \"new\" -v option?\n"},{"id":"130357","messageId":"be6fef0d0912261922t230047b4v395fb2f6ce3ace4d@mail.gmail.com","threadId":"17669","inReplyTo":"20091227012032.GE25474@genesis.frugalware.org","subject":"Re: [PATCH 4/4] clone: use --progress to force progress reporting","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-12-27T03:22:22Z","receivedAt":"2009-12-27T03:22:22Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Sun, Dec 27, 2009 at 9:20 AM, Miklos Vajna <vmiklos@frugalware.org> wrote:\n> On Sat, Dec 26, 2009 at 01:12:06AM +0800, Tay Ray Chuan <rctay89@gmail.com> wrote:\n>> -v/--verbose now does not force progress reporting. Make git-clone.txt\n>> say so.\n>>\n>> This should cover all the bases in 21188b1 (Implement git clone -v),\n>> which implemented the option to force progress reporting.\n>>\n>> Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>\n>> ---\n>>  Documentation/git-clone.txt |    3 +++\n>>  builtin-clone.c             |    8 ++++++--\n>>  t/t5702-clone-options.sh    |    3 ++-\n>>  3 files changed, 11 insertions(+), 3 deletions(-)\n>>\n>> diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\n>> index e722e6c..f43c8b2 100644\n>> --- a/Documentation/git-clone.txt\n>> +++ b/Documentation/git-clone.txt\n>> @@ -102,6 +102,9 @@ objects from the source repository into a pack in the cloned repository.\n>>\n>>  --verbose::\n>>  -v::\n>> +     Run verbosely.\n>\n> What about mentioning this \"does not force progress reporting\" behaviour\n> in documentation of the \"new\" -v option?\n\nOk.\n\n-- \nCheers,\nRay Chuan\n"},{"id":"130358","messageId":"be6fef0d0912261927p4aff3ecet5cc26bd6dfc7e1c2@mail.gmail.com","threadId":"17669","inReplyTo":"alpine.DEB.1.00.0912260952020.4985@pacific.mpi-cbg.de","subject":"Re: [PATCH 0/4] clone: use --progress to mean -v","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-12-27T03:27:05Z","receivedAt":"2009-12-27T03:27:05Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Sat, Dec 26, 2009 at 4:53 PM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> On Sat, 26 Dec 2009, Tay Ray Chuan wrote:\n>\n>> This series makes git-clone follow the \"argument convention\" of\n>> git-pack-objects, where the option --progress is used to force reporting\n>> of reporting. This was previously done with -v/--verbose.\n>\n> No objections from my side, although you might want to advertise more that\n> this is a change in behavior.  (Meaning in the release notes)\n\nIndeed, -v/--verbose to force reporting of progress was done sometime\nlast year (Thu Oct 9 2008) so there may be scripts/applications\ndependent on this option.\n\nJunio, do you have any advice on this front?\n\n>> PS. If someone can enlighten me on the proper noun for the git\n>>     executable (I said \"main git\"), I would be very thankful.\n>\n> I call it the \"Git wrapper\", although less polite words exist, too.\n\nI see. Thanks!\n\n-- \nCheers,\nRay Chuan\n"},{"id":"130434","messageId":"7vljgmpnxj.fsf@alter.siamese.dyndns.org","threadId":"17669","inReplyTo":"be6fef0d0912261927p4aff3ecet5cc26bd6dfc7e1c2@mail.gmail.com","subject":"Re: [PATCH 0/4] clone: use --progress to mean -v","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-12-29T01:30:00Z","receivedAt":"2009-12-29T01:30:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tay Ray Chuan <rctay89@gmail.com> writes:\n\n> On Sat, Dec 26, 2009 at 4:53 PM, Johannes Schindelin\n> <Johannes.Schindelin@gmx.de> wrote:\n>> On Sat, 26 Dec 2009, Tay Ray Chuan wrote:\n>>\n>>> This series makes git-clone follow the \"argument convention\" of\n>>> git-pack-objects, where the option --progress is used to force reporting\n>>> of reporting. This was previously done with -v/--verbose.\n>>\n>> No objections from my side, although you might want to advertise more that\n>> this is a change in behavior.  (Meaning in the release notes)\n>\n> Indeed, -v/--verbose to force reporting of progress was done sometime\n> last year (Thu Oct 9 2008) so there may be scripts/applications\n> dependent on this option.\n>\n> Junio, do you have any advice on this front?\n\n[1/4] sounds like a sane thing to do regardless of the remainder of the\nseries, as stderr is where we write the progress output anyway. [2/4]\nlooks trivially correct.\n\nIt is unclear what impact [3/4] has.  I can read \"With this patch,\ntransport can pay attention to the verbose option given from the end user\nand act more verbosely, which was not something they couldn't do before\",\nbut what is the practical difference our existing users would see?  IOW,\nwhich transports are silent without this patch even when the user gives -v\nfrom the command line?\n\nAnd continuing the theme to separate the \"verbosity\" and the \"progress\"\ninto two separate switches, and push them down to the transport layer by\n[3/4], [4/4] sounds like a logical conclusion.\n\nI however wonder if it is of lessor impact if we only added --progress\nbut without removing the progress from -v.  Is there a downside?\n"},{"id":"130435","messageId":"be6fef0d0912281906p432d012av2a774e179294260f@mail.gmail.com","threadId":"17669","inReplyTo":"7vljgmpnxj.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 0/4] clone: use --progress to mean -v","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-12-29T03:06:29Z","receivedAt":"2009-12-29T03:06:29Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Tue, Dec 29, 2009 at 9:30 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Tay Ray Chuan <rctay89@gmail.com> writes:\n>\n>> On Sat, Dec 26, 2009 at 4:53 PM, Johannes Schindelin\n>> <Johannes.Schindelin@gmx.de> wrote:\n>>> On Sat, 26 Dec 2009, Tay Ray Chuan wrote:\n>>>\n>>>> This series makes git-clone follow the \"argument convention\" of\n>>>> git-pack-objects, where the option --progress is used to force reporting\n>>>> of reporting. This was previously done with -v/--verbose.\n>>>\n>>> No objections from my side, although you might want to advertise more that\n>>> this is a change in behavior.  (Meaning in the release notes)\n>>\n>> Indeed, -v/--verbose to force reporting of progress was done sometime\n>> last year (Thu Oct 9 2008) so there may be scripts/applications\n>> dependent on this option.\n>>\n>> Junio, do you have any advice on this front?\n>\n> [1/4] sounds like a sane thing to do regardless of the remainder of the\n> series, as stderr is where we write the progress output anyway. [2/4]\n> looks trivially correct.\n>\n> It is unclear what impact [3/4] has.  I can read \"With this patch,\n> transport can pay attention to the verbose option given from the end user\n> and act more verbosely, which was not something they couldn't do before\",\n> but what is the practical difference our existing users would see?  IOW,\n> which transports are silent without this patch even when the user gives -v\n> from the command line?\n\nI know at least one transport which behaves in this manner (ie. silent\neven when -v is supplied to git-clone), and that is the http (via\ncurl) transport.\n\n> I however wonder if it is of lessor impact if we only added --progress\n> but without removing the progress from -v.  Is there a downside?\n\n(Just to clarify: progress reporting will be done if stderr is a\nterminal - it will be done even if -v or --progress isn't present.\nWhat -v/--progress does is force progress reporting even if stderr is\nnot a terminal.)\n\nLeaving -v as it is (ie. forcing progress reporting) while adding\n--progress would be a \"safe\" option, as it won't break people's\nexisting setups (ie. those that depend on -v to force progress\nreporting), which the patch series does. I have in mind IDEs/editors\nthat use this behaviour to monitor progress.\n\nOn the other hand, if we decide -v shouldn't imply forcing progress\nreporting, then I think this breakable change should be made soon,\nwhen only a small minority of git commands are affected (only one,\ngit-clone). That way, we don't give users/integrators the impression\nthat -v forces progress reporting with git commands. They won't get\nannoyed when try -v to force progress reporting and find that it isn't\nthe case.\n\nBy the way, I got this \"-v doesn't imply forced progress reporting\"\nrule from Jeff (added to Cc list), who mentioned it some time ago:\n\n  Date: Mon, 8 Jun 2009 07:54:31 -0400\n  From: Jeff King <peff@peff.net>\n  Subject: Re: [Patch] Prevent cloning over http from spewing\n  Message-ID: <20090608115431.GC13775@coredump.intra.peff.net>\n\n  I was imagining:\n\n    - without \"-q\", show progress if isatty(1).\n\n    - with \"-q\", never show progress\n\n    - with \"-v\", show the \"getting pack\" and \"walk\" output we show now;\n      without \"-v\", don't show it. \"-v\" has no impact on the progress\n      indicator.\n\n-- \nCheers,\nRay Chuan\n"}]}