{"thread":{"id":"27720","subject":"[PATCH] progress: use \\r as EOL only if isatty(stderr) is true","startedAt":"2011-06-28T14:40:09Z","lastAt":"2011-08-29T20:17:40Z","messageCount":21,"participants":["Steffen Daode Nurpmeso","Junio C Hamano","Jeff King","Nicolas Pitre","Miles Bader","martin f krafft","sdaoden@googlemail.com"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"170626","messageId":"1309272009-23076-1-git-send-email-sdaoden@gmail.com","threadId":"27720","inReplyTo":null,"subject":"[PATCH] progress: use \\r as EOL only if isatty(stderr) is true","fromName":"Steffen Daode Nurpmeso","fromEmail":"sdaoden@googlemail.com","sentAt":"2011-06-28T14:40:09Z","receivedAt":"2011-06-28T14:40:09Z","isPatch":true,"sender":{"key":"sdaoden@googlemail.com","avatar":null},"body":"So far progress always uses \\r to produce one-line output on stderr.\nThis only produces useful and easy parsable output if stderr is opened\non a file which does interpret CR as a real carriage return operation.\nThis patch changes EOL to the plain newline \\n control if isatty() is\nfalse instead.\n\nSigned-off-by: Steffen Daode Nurpmeso \n---\n progress.c | 4 +++-\n 1 files changed, 3 insertions(+), 1 deletions(-)\n\ndiff --git a/progress.c b/progress.c\nindex 3971f49..c548de4 100644\n--- a/progress.c\n+++ b/progress.c\n@@ -27,6 +27,7 @@ struct throughput {\n \n struct progress {\n \tconst char *title;\n+\tconst char *eol;\n \tint last_value;\n \tunsigned total;\n \tunsigned last_percent;\n@@ -90,7 +91,7 @@ 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+\teol = done ? done : progress->eol;\n \tif (progress->total) {\n \t\tunsigned percent = n * 100 / progress->total;\n \t\tif (percent != progress->last_percent || progress_update) {\n@@ -219,6 +220,7 @@ struct progress *start_progress_delay(const char *title, unsigned total,\n \t\treturn NULL;\n \t}\n \tprogress->title = title;\n+\tprogress->eol = isatty(fileno(stderr)) ? \" \\r\" : \"\\n\";\n \tprogress->total = total;\n \tprogress->last_value = -1;\n \tprogress->last_percent = -1;\n-- \n1.7.6.rc0\n"},{"id":"170631","messageId":"20110628165153.GA59095@sherwood.local","threadId":"27720","inReplyTo":"BANLkTinRe=pA=_obCmPKBjJMXH_pDfwCtw@mail.gmail.com","subject":"Re: [PATCH] progress: use \\r as EOL only if isatty(stderr) is true","fromName":"Steffen Daode Nurpmeso","fromEmail":"sdaoden@googlemail.com","sentAt":"2011-06-28T16:51:53Z","receivedAt":"2011-06-28T16:51:53Z","isPatch":true,"sender":{"key":"sdaoden@googlemail.com","avatar":null},"body":"@ Tay Ray Chuan wrote (2011-06-28 18:27+0200):\n> hmm, shouldn't the onus of checking tty fall on to callers of\n> progress* functions?\n\nI dunno and have no overview of the git(1) codebase. I have a\n\"arena/code.extern.repos/\" directory tree and use a shell script\nwhich manages it (e.g. \"$ arena-manager update\" to pull all repos).\nThe output of this script is (also) redirected into a log file via\ntee(1), and for git(1) invocations the resulting output is not very\nuseful.\n\nI agree that it is maybe silly to not use --no-progress from within\nthe script (the script comes from cvs(1) background). Maybe i should\nchange it to not use git porcelain but directly script the plumbing -\n:-) -, but i am *very* new to git(1) and in the meanwhile this simple\npatch pimps up my log output.\n\n--\nCiao, Steffen\nsdaoden(*)(gmail.com)\n() ascii ribbon campaign - against html e-mail\n/\\ www.asciiribbon.org - against proprietary attachments\n"},{"id":"170635","messageId":"20110628180439.GA86588@sherwood.local","threadId":"27720","inReplyTo":"BANLkTinRe=pA=_obCmPKBjJMXH_pDfwCtw@mail.gmail.com","subject":"Re: [PATCH] progress: use \\r as EOL only if isatty(stderr) is true","fromName":"Steffen Daode Nurpmeso","fromEmail":"sdaoden@googlemail.com","sentAt":"2011-06-28T18:04:39Z","receivedAt":"2011-06-28T18:04:39Z","isPatch":true,"sender":{"key":"sdaoden@googlemail.com","avatar":null},"body":"@ Tay Ray Chuan wrote (2011-06-28 18:27+0200):\n> hmm, shouldn't the onus of checking tty fall on to callers of\n> progress* functions?\n\nOk, in the meanwhile i have understood what you ment, but\nunfortunately i have thrown away the logfile which caused me to\nwrite the patch, and even more unfortunately i could not reproduce\nin the last hour or what any 'xy ^Mxy ^M' dump with the current\nstate of the repos. A short 'git grep -F progress' is useless\nfrom my current point of view (and with the amount of my free\ntime). The script contains these git(1) related commands anyway:\n\n git pull -v --ff-only --stat --prune && git gc\n git svn rebase [not at the time of the log: && git gc]\n\nBut that is true: i have had a log file with the mentioned content.\n\n--\nCiao, Steffen\nsdaoden(*)(gmail.com)\n() ascii ribbon campaign - against html e-mail\n/\\ www.asciiribbon.org - against proprietary attachments\n"},{"id":"170636","messageId":"7vwrg5u7oz.fsf@alter.siamese.dyndns.org","threadId":"27720","inReplyTo":"1309272009-23076-1-git-send-email-sdaoden@gmail.com","subject":"Re: [PATCH] progress: use \\r as EOL only if isatty(stderr) is true","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-06-28T18:33:48Z","receivedAt":"2011-06-28T18:33:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Steffen Daode Nurpmeso writes:\n\n> So far progress always uses \\r to produce one-line output on stderr.\n> This only produces useful and easy parsable output if stderr is opened\n> on a file which does interpret CR as a real carriage return operation.\n> This patch changes EOL to the plain newline \\n control if isatty() is\n> false instead.\n>\n> Signed-off-by: Steffen Daode Nurpmeso \n\nI kind of like this patch, in the sense that if there is a sane scenario\nto emit progress to non-tty, we should do just LF not CRLF, but I would\nlike to know the real motivation behind this proposal.\n\nI thought that we try to disable the progress pretty much everywhere when\nwe are not talking to a tty, so ugliness coming from many CRLF appearing\nin the cron e-mail shouldn't be the issue.\n"},{"id":"170639","messageId":"20110628184833.GA385@sherwood.local","threadId":"27720","inReplyTo":"7vwrg5u7oz.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] progress: use \\r as EOL only if isatty(stderr) is true","fromName":"Steffen Daode Nurpmeso","fromEmail":"sdaoden@googlemail.com","sentAt":"2011-06-28T18:48:33Z","receivedAt":"2011-06-28T18:48:33Z","isPatch":true,"sender":{"key":"sdaoden@googlemail.com","avatar":null},"body":"@ Junio C Hamano wrote (2011-06-28 20:33+0200):\n> [..]\n> I thought that we try to disable the progress pretty much\n> everywhere when we are not talking to a tty[..]\n\nIf Tay Ray Chuan would have Cc'd the list, too, i would have\nthought he tried to draw attention onto the fact that this is\nquestionable design, if you allow me this comment.\n\nIt would indeed be more logical to simply \"peek\" onto progress and\nlet that decide on it's own wether it is useful or possible to\nprint any progress indication or not.\n\n--\nCiao, Steffen\nsdaoden(*)(gmail.com)\n() ascii ribbon campaign - against html e-mail\n/\\ www.asciiribbon.org - against proprietary attachments\n"},{"id":"170640","messageId":"20110628185556.GB385@sherwood.local","threadId":"27720","inReplyTo":"7vwrg5u7oz.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] progress: use \\r as EOL only if isatty(stderr) is true","fromName":"Steffen Daode Nurpmeso","fromEmail":"sdaoden@googlemail.com","sentAt":"2011-06-28T18:55:56Z","receivedAt":"2011-06-28T18:55:56Z","isPatch":true,"sender":{"key":"sdaoden@googlemail.com","avatar":null},"body":"@ Junio C Hamano wrote (2011-06-28 20:33+0200):\n> [..]\n> I thought that we try to disable the progress pretty much\n> everywhere when we are not talking to a tty[..]\n\nThat is to say, at least some kind of progress_maybe_applicable()\nor the like, instead of having things like\n\n builtin/pack-objects.c: progress = isatty(2);\n\nspread out in the code, and just see what 'git grep -F isatty'\nreports ...\n\n--\nCiao, Steffen\nsdaoden(*)(gmail.com)\n() ascii ribbon campaign - against html e-mail\n/\\ www.asciiribbon.org - against proprietary attachments\n"},{"id":"170651","messageId":"20110628224516.GB4192@sigill.intra.peff.net","threadId":"27720","inReplyTo":"7vwrg5u7oz.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] progress: use \\r as EOL only if isatty(stderr) is true","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-06-28T22:45:16Z","receivedAt":"2011-06-28T22:45:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 28, 2011 at 11:33:48AM -0700, Junio C Hamano wrote:\n\n> > So far progress always uses \\r to produce one-line output on stderr.\n> > This only produces useful and easy parsable output if stderr is opened\n> > on a file which does interpret CR as a real carriage return operation.\n> > This patch changes EOL to the plain newline \\n control if isatty() is\n> > false instead.\n> >\n> > Signed-off-by: Steffen Daode Nurpmeso \n> \n> I kind of like this patch, in the sense that if there is a sane scenario\n> to emit progress to non-tty, we should do just LF not CRLF, but I would\n> like to know the real motivation behind this proposal.\n> \n> I thought that we try to disable the progress pretty much everywhere when\n> we are not talking to a tty, so ugliness coming from many CRLF appearing\n> in the cron e-mail shouldn't be the issue.\n\nWe certainly do try to turn off progress reporting when stderr isn't a\ntty. So unless \"--progress\" is being given explicitly, seeing it is a\nbug that should be fixed.\n\nI'm not sure dropping the CR is a good thing, though. One of the uses\nfor forcing output to a non-terminal via \"--progress\", is that something\n_else_ is going to parse the output. And that other thing gets useful\ninformation from the carriage returns.\n\nFor example, you may be piping into a file or a fifo and running 'tail'\nto a terminal on the other end. You want the CR because we are\nultimately going to a terminal.\n\nAnother example: you write a GUI wrapper around git that captures and\nparses stderr. You show progress and informative messages in a running\ndialog. The difference between CR and LF is important. The former means\n\"clear the progress line and show this new one instead\"; the latter\nmeans \"keep this on the screen and show more lines\".\n\nI'm willing to accept that there are use cases where you don't want the\nCRs, but just want a list of lines[1]. But it seems like this change\nhurts some existing use cases.\n\n-Peff\n\n[1] Actually, I would be curious to see such a use case. If you are\nplanning on saving the output, is it really useful to have a hundred\nlines saying:\n\n Compressing objects 1% (100/10000)\n Compressing objects 2% (200/10000)\n\nand so forth?\n"},{"id":"170677","messageId":"20110629174220.GA36658@sherwood.local","threadId":"27720","inReplyTo":"7vwrg5u7oz.fsf@alter.siamese.dyndns.org","subject":"[PATCH/RFC] sideband: remove line padding (was: Re: [PATCH] progress: use \\r as EOL only if isatty(stderr) is true)","fromName":"Steffen Daode Nurpmeso","fromEmail":"sdaoden@googlemail.com","sentAt":"2011-06-29T17:42:20Z","receivedAt":"2011-06-29T17:42:20Z","isPatch":true,"sender":{"key":"sdaoden@googlemail.com","avatar":null},"body":"I desperately tried to reproduce the 'xy ^Mxy ^M' case, but i\ncan't. In fact i cannot even find a piece of code which would do\nit. :-/ Such a shame.\nStill: i insist on having that seen in my arena-manager.log.\n\nWhile searching around i stumbled over fetch-pack output with an\nANSI escape sequence after a '(xy) 2>&1 | tee LOG'.\n--\nCiao, Steffen\nsdaoden(*)(gmail.com)\n() ascii ribbon campaign - against html e-mail\n/\\ www.asciiribbon.org - against proprietary attachments\n\n-- >8 --\nSubject: [PATCH/RFC] sideband: remove line padding\n\nFor formatting purposes recv_sideband() sofar appended a suffix to\nband #2 (informative) messages: dependent on getenv(\"TERM\") lines\nmay have been space-filled (\"dumb\") or padded with an ANSI escape\nsequence (ANSI EL, mode 0: clear from cursor to end of line). This\npatch removes handling of terminal specifics and any pad suffixes.\n\nIt also fixes two 'signed/unsigned comparison' compiler warnings.\n\nSigned-off-by: Steffen Daode Nurpmeso \n---\nI believe that users of fetch-pack, send-pack and archive, the three\naffected operations, will ensure that lines are refreshed as approbiate,\neven if 'brk-1' == CR. But even if not: i would move handling of\n\"TERM\" and dumb/non-dumb terminals out of this packet handler and\ninto some terminal encapsulator, which can only be color.* as far\nas i see. The question would be how this should be done. Maybe\nterm_fprintf_padln(?, desired-line-length-or--1)? Such a thing would\nsurely find other users in the codebase. A second approach would\nbe to only use the \"dumb\" pad, as in\n\n fprintf(stderr, \"%.*s %c\", brk - 1, b, b[brk - 1]);\n\nWhat do you think of that?\n\n sideband.c | 35 +++++------------------------------\n 1 files changed, 5 insertions(+), 30 deletions(-)\n\ndiff --git a/sideband.c b/sideband.c\nindex d5ffa1c..17e1793 100644\n--- a/sideband.c\n+++ b/sideband.c\n@@ -14,26 +14,13 @@\n \n #define PREFIX \"remote:\"\n \n-#define ANSI_SUFFIX \"\\033[K\"\n-#define DUMB_SUFFIX \" \"\n-\n-#define FIX_SIZE 10 /* large enough for any of the above */\n-\n int recv_sideband(const char *me, int in_stream, int out)\n {\n-\tunsigned pf = strlen(PREFIX);\n-\tunsigned sf;\n-\tchar buf[LARGE_PACKET_MAX + 2*FIX_SIZE];\n-\tchar *suffix, *term;\n+\tunsigned pf = sizeof(PREFIX) - 1;\n+\tchar buf[LARGE_PACKET_MAX + 2*sizeof(PREFIX)];\n \tint skip_pf = 0;\n \n \tmemcpy(buf, PREFIX, pf);\n-\tterm = getenv(\"TERM\");\n-\tif (term && strcmp(term, \"dumb\"))\n-\t\tsuffix = ANSI_SUFFIX;\n-\telse\n-\t\tsuffix = DUMB_SUFFIX;\n-\tsf = strlen(suffix);\n \n \twhile (1) {\n \t\tint band, len;\n@@ -82,20 +69,8 @@ int recv_sideband(const char *me, int in_stream, int out)\n \t\t\t\t\t\tbreak;\n \t\t\t\t}\n \n-\t\t\t\t/*\n-\t\t\t\t * Let's insert a suffix to clear the end\n-\t\t\t\t * of the screen line if a line break was\n-\t\t\t\t * found. Also, if we don't skip the\n-\t\t\t\t * prefix, then a non-empty string must be\n-\t\t\t\t * present too.\n-\t\t\t\t */\n-\t\t\t\tif (brk > (skip_pf ? 0 : (pf+1 + 1))) {\n-\t\t\t\t\tchar save[FIX_SIZE];\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\tfprintf(stderr, \"%.*s\", brk + sf, b);\n-\t\t\t\t\tmemcpy(b + brk, save, sf);\n+\t\t\t\tif ((unsigned)brk > (skip_pf ? 0 : (pf+1 +1))) {\n+\t\t\t\t\tfprintf(stderr, \"%.*s\", brk, b);\n \t\t\t\t\tlen -= brk;\n \t\t\t\t} else {\n \t\t\t\t\tint l = brk ? brk : len;\n@@ -133,7 +108,7 @@ ssize_t send_sideband(int fd, int band, const char *data, ssize_t sz, int packet\n \t\tchar hdr[5];\n \n \t\tn = sz;\n-\t\tif (packet_max - 5 < n)\n+\t\tif ((unsigned)packet_max - 5 < n)\n \t\t\tn = packet_max - 5;\n \t\tif (0 <= band) {\n \t\t\tsprintf(hdr, \"%04x\", n + 5);\n-- \n1.7.6.1.ge79e.dirty\n"},{"id":"170678","messageId":"alpine.LFD.2.00.1106291414140.2142@xanadu.home","threadId":"27720","inReplyTo":"20110629174220.GA36658@sherwood.local","subject":"Re: [PATCH/RFC] sideband: remove line padding (was: Re: [PATCH] progress: use \\r as EOL only if isatty(stderr) is true)","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2011-06-29T18:15:10Z","receivedAt":"2011-06-29T18:15:10Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Wed, 29 Jun 2011, Steffen Daode Nurpmeso wrote:\n\n> For formatting purposes recv_sideband() sofar appended a suffix to\n> band #2 (informative) messages: dependent on getenv(\"TERM\") lines\n> may have been space-filled (\"dumb\") or padded with an ANSI escape\n> sequence (ANSI EL, mode 0: clear from cursor to end of line). This\n> patch removes handling of terminal specifics and any pad suffixes.\n\nWhy?\n\n\nNicolas\n"},{"id":"170682","messageId":"7vtyb8s4kv.fsf@alter.siamese.dyndns.org","threadId":"27720","inReplyTo":"20110628224516.GB4192@sigill.intra.peff.net","subject":"Re: [PATCH] progress: use \\r as EOL only if isatty(stderr) is true","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-06-29T21:36:16Z","receivedAt":"2011-06-29T21:36:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I'm willing to accept that there are use cases where you don't want the\n> CRs, but just want a list of lines[1]. But it seems like this change\n> hurts some existing use cases.\n>\n> -Peff\n>\n> [1] Actually, I would be curious to see such a use case. If you are\n> planning on saving the output, is it really useful to have a hundred\n> lines saying:\n>\n> Compressing objects 1% (100/10000)\n> Compressing objects 2% (200/10000)\n>\n> and so forth?\n\nAll valid arguments against the change, I think. Thanks.\n"},{"id":"170693","messageId":"buohb78aqfg.fsf@dhlpc061.dev.necel.com","threadId":"27720","inReplyTo":"7vtyb8s4kv.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] progress: use \\r as EOL only if isatty(stderr) is true","fromName":"Miles Bader","fromEmail":"miles@gnu.org","sentAt":"2011-06-30T04:33:55Z","receivedAt":"2011-06-30T04:33:55Z","isPatch":true,"sender":{"key":"miles@gnu.org","avatar":"https://gravatar.com/avatar/01069b69593af7bff28e2f97afeb3644ae6fe2f5f56cb3a8cf34c5fb8c36efe5?d=mp&s=160"},"body":"Junio C Hamano writes:\n> All valid arguments against the change, I think. Thanks.\n\nYeah, I think better to leave it alone.\n\nI sometimes pipe the output of such progress-meter-using programs\nthrough a filter that both sends the output to the terminal (where the\n\\rs are clearly desired) and logs it to a file -- and the \\rs are even\nuseful for the latter purpose, and the filter can use them to delete\nall but the last copy of each line before logging.\n\n-Miles\n\n-- \nMy books focus on timeless truths. -- Donald Knuth\n"},{"id":"170731","messageId":"20110630211329.GC63317@sherwood.local","threadId":"27720","inReplyTo":"alpine.LFD.2.00.1106291414140.2142@xanadu.home","subject":"Re: [PATCH/RFC] sideband: remove line padding (was: Re: [PATCH] progress: use \\r as EOL only if isatty(stderr) is true)","fromName":"Steffen Daode Nurpmeso","fromEmail":"sdaoden@googlemail.com","sentAt":"2011-06-30T21:13:29Z","receivedAt":"2011-06-30T21:13:29Z","isPatch":true,"sender":{"key":"sdaoden@googlemail.com","avatar":null},"body":"@ Nicolas Pitre wrote (2011-06-29 20:15+0200):\n> Why?\n\nOk, they don't. (Your initial patch was from 2008, and since\ngit(1) is evolving very fast, it might have been that padding is\nno longer of any use at all.)\nSo, then, i'm the right person to turn the big wheel:\n\n- Move color.* and even progress.* stuff into new visual.[hc].\n- Add a vis[ual]_init() which does check isatty() for [012] and\n does the TERM check (i.e. \"dumb\" or otherwise).\n There could also be flags which could be used to restrict what\n is checked ...\n- Replace calls to isatty(xy) by std{in,out,err}_is_tty(), defined\n in visual.h.\n Calling vis[ual]_init() would not be needed for these.\n (But they could be inline if it would be.)\n- Add a series of vis_print functions in equal spirit to the\n color series which use detected terminal capabilities to ensure\n that a line consists only of the printed data.\n\nI (almost) had that state hour ago, but it's ... (The problem with\nthat print series is that in the dumb case the data is to be injected\nbefore a possibly contained NL/CR sequence, so that already inspected\ndata is to be reinspected. Or data has to be copied around. Or\nwrites have to be splitted, but here i'm talking about stderr, and\nthat's unbuffered, and so each invocation goes through the OS.)\n\nSo what else could be done to remove TERM and ANSI escape sequence\nknowledge from a nice multiband stream splitter, and to avoid that\nit writes the escape sequence if output is stupidly redirected to a\nfile? Well, visual.h could consist of a single function only:\n\n const char *ansi_el_if_tty_and_termok_else_null(void);\n--\nCiao, Steffen\nsdaoden(*)(gmail.com)\n() ascii ribbon campaign - against html e-mail\n/\\ www.asciiribbon.org - against proprietary attachments\n"},{"id":"170734","messageId":"alpine.LFD.2.00.1106302331050.1371@xanadu.home","threadId":"27720","inReplyTo":"20110630211329.GC63317@sherwood.local","subject":"Re: [PATCH/RFC] sideband: remove line padding (was: Re: [PATCH] progress: use \\r as EOL only if isatty(stderr) is true)","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2011-07-01T03:46:58Z","receivedAt":"2011-07-01T03:46:58Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Thu, 30 Jun 2011, Steffen Daode Nurpmeso wrote:\n\n> @ Nicolas Pitre wrote (2011-06-29 20:15+0200):\n> > Why?\n> \n> Ok, they don't. (Your initial patch was from 2008, and since\n> git(1) is evolving very fast, it might have been that padding is\n> no longer of any use at all.)\n\nBut they absolutely still are. What makes you think they aren't \nanymore?\n\n> So, then, i'm the right person to turn the big wheel:\n\nI'm afraid you'll have to understand _why_ that padding is still needed \nfirst. Luckily the various commit messages for sideband.c are well \ndetailed on the reasons for the current code.\n\n\nNicolas\n"},{"id":"174391","messageId":"cover.1314472512.git.sdaoden@gmail.com","threadId":"27720","inReplyTo":"7vwrg5u7oz.fsf@alter.siamese.dyndns.org","subject":"[PATCH] checkout: be quiet if not on isatty()","fromName":"Steffen Daode Nurpmeso","fromEmail":"sdaoden@googlemail.com","sentAt":"2011-08-27T19:45:51Z","receivedAt":"2011-08-27T19:45:51Z","isPatch":true,"sender":{"key":"sdaoden@googlemail.com","avatar":null},"body":"(Original subject was:\nRe: [PATCH] progress: use \\r as EOL only if isatty(stderr) is true)\n\n@ Junio C Hamano wrote (2011-06-28 20:33+0200):\n> [..]\n> I thought that we try to disable the progress pretty much\n> everywhere when we are not talking to a tty[..]\n\nToday i got this by accident:\n\n ================================================================\n openbsd-src.git-null: performing reduce\n Checking out files: 17% (11410/65508) ^MChecking out files:\n 18% (11792/65508) ^M [..]\n\nThis is the output of my dumb arena-manager, which i append after\nthe first scissor for reference. (Maybe someone finds it useful.\nIt's a dumb sh(1) thing, i don't do shell scripting that often.)\nI haven't actually tried it yet, but setting .quiet for\nunpack_trees may stop this output. (Compilation succeeds,\nthough.) I still and *truly* have no idea of the git(1)\ninternals, so i may oversee .. just *any* thinkable side-effect.\nHope this helps a bit.\n(Actual patch as response. Have a nice weekend.)\n\n--Steffen\nCiao, sdaoden(*)(gmail.com)\nASCII ribbon campaign ( ) More nuclear fission plants\n against HTML e-mail X can serve more coloured\n and proprietary attachments / \\ and sounding animations\n-- >8 --\n#!/bin/bash\n# NOTE: this acts according to extensions, e.g.:\n# docutils.svn-git-null: git svn\n# groff.cvs-git-null : git cvsimport\n# openbsd-src.git-null : git\n# vim.hg-null : Mercurial\n# If a '-null' is in the suffix one may use 'reduce' and 'expand'\n# modes; for git(1) these modes require an empty NULL branch:\n# $ git co --orphan NULL && git rm -rf '*' &&\n# echo DEFAULT-BRANCH >NULL && git add NULL && git ci -m NULL\n# Mercurial has builtin support for 'null'. (Nice for backups.)\n\n# Top dir where everything happens\nARENA=\"$HOME/arena/code.extern.repos\"\n\n##\n\nGIT=git\nHG=hg\nSVN=svn\nCVS='cvs -fz 9 update -ACRPd'\nESTAT=0\nLOGFILE=lastlog.txt\nCVSROOT=/nonexistent\nCURR=/nonexistent\n\nlog() {\n echo \"$*\"\n echo \"$*\" >> $LOGFILE\n}\nlogerr() {\n echo >&2 \"ERROR: $*\"\n echo \"ERROR: $*\" >> $LOGFILE\n ESTAT=1\n}\n\nSEP='================================================================'\nintro() {\n log ''\n log $SEP\n log $1\n}\nfinal() {\n local es=$?\n if test $es -eq 0; then\n log \"... ok: $1\"\n else\n logerr \"$1\"\n ESTAT=$es\n fi\n log $SEP\n log ''\n}\n\nif test \"$BASH\" = x\"\"; then\n echo >&2 \"This script needs the GNU bash shell interpreter\"\n exit 1\nfi\ncd $ARENA || {\n echo >&2 \"Failed to chdir to $ARENA\"\n exit 1\n}\nrm -rf $LOGFILE || {\n echo >&2 \"Failed to remove stale $LOGFILE\"\n exit 1\n}\n\nMODE=\"$1\"\nshift\nPARAMS=\"$@\"\nset -u\ntest $# -ne 0 || PARAMS=$(echo *.*)\n\n# Perform basename cleanup and move over to $params[]\nlog \"$0: script startup, mode $MODE\"\ndeclare -a params\nfor rd in $PARAMS; do\n rd=$(echo \"$rd\" | sed -Ee 's/(.+)\\/+$/\\1/' -e 's/.*\\/([^/]+)$/\\1/')\n params[${#params[*]}]=\"$rd\"\ndone\n\nGITDID=\ncase \"$MODE\" in\nreduce|expand)\n if test \"$MODE\" == 'reduce'; then\n git_branch='NULL'\n hg_branch='null'\n else\n git_branch=\n hg_branch=''\n fi\n\n for rd in ${params[@]}; do\n if test \"$rd\" == \"${rd/null/}\"; then\n log \"[$rd: $MODE does not apply]\"\n continue\n fi\n\n intro \"$rd: performing $MODE\"\n set -o pipefail\n ( cd \"$rd\" || exit 1\n if test \"$rd\" != \"${rd/git-null}\"; then\n # On branch NULL file NULL contains master branch's name\n if test -z \"$git_branch\"; then\n if test -f NULL; then\n git_branch=$(&2 \"No file NULL in $rd\"\n git_branch=master\n fi\n fi\n $GIT checkout $git_branch\n else\n $HG up $hg_branch\n fi\n exit $?\n ) 2>&1 | tee -a \"$LOGFILE\"\n final \"$rd\"\n set +o pipefail\n done\n ;;\nupdate)\n for rd in ${params[@]}; do\n intro \"$rd: performing $MODE\"\n set -o pipefail\n ( cd \"$rd\" || exit 1\n if test \"$rd\" != \"${rd/.git}\"; then\n #$GIT pull -v --ff-only --stat --prune\n $GIT fetch --verbose --prune\n GITDID=1\n elif test \"$rd\" != \"${rd/.svn-git}\"; then\n $GIT svn rebase\n GITDID=1\n elif test \"$rd\" != \"${rd/.cvs-git}\"; then\n ldir='.git/.cvsps'\n tar xjf \"$ldir.tbz\" || {\n echo >&2 \"$rd: bail: tar xjf $ldir.tbz\"\n exit 1\n }\n hdir=\"$HOME/.cvsps\"\n test -d \"$hdir\" || mkdir \"$hdir\" || {\n echo >&2 \"$rd: failed to create $hdir directory\"\n exit 2\n }\n root=\"$ldir/CVSROOT\"\n repo=\"$ldir/MODULE\"\n cache=$(<$ldir/CVSPS_FILE)\n mv -f \"$ldir/cvs-revisions\" .git/\n mv -f \"$ldir/$cache\" \"$hdir/$cache\"\n $GIT cvsimport -aR -r origin -p '-u,--cvs-direct' \\\n -d $(<$root) $(<$repo)\n es=$?\n GITDID=1\n mv -f .git/cvs-revisions \"$ldir/\"\n mv -f \"$hdir/$cache\" \"$ldir/$cache\"\n tar cjf \"$ldir.tbz\" \"$ldir\" || {\n echo >&2 \"$rd: bail: tar cjf $ldir.tbz $ldir\"\n exit 3\n }\n rm -rf $ldir\n exit $es\n elif test \"$rd\" != \"${rd/.hg}\"; then\n $HG -v pull #-u\n elif test \"$rd\" != \"${rd/.svn}\"; then\n $SVN update\n elif test \"$rd\" != \"${rd/.cvs}\"; then\n $CVS\n else\n echo \"Unknown revision-control-system: $rd\"\n exit 1\n fi\n ) 2>&1 | tee -a \"$LOGFILE\"\n final \"$rd\"\n set +o pipefail\n done\n ;;\nfullgc|gc)\n gct=\n test \"$MODE\" == fullgc && gct=--aggressive\n for rd in ${params[@]}; do\n intro \"$rd: performing $MODE\"\n set -o pipefail\n ( cd \"$rd\" || exit 1\n test \"$rd\" != \"${rd/.git}\" && git gc $gct\n ) 2>&1 | tee -a \"$LOGFILE\"\n final \"$rd\"\n set +o pipefail\n done\n ;;\n*)\n echo 'USAGE: manager reduce|expand|update|gc|fullgc LIST-OF-DIRECTORIES'\n exit 1\n ;;\nesac\ntest x\"$GITDID\" != x && log 'git(1) fetched data - do arena-manager [full]gc ..'\n\ntest $ESTAT -ne 0 && log 'Errors occurred!'\nexit $ESTAT\n# vim:set fenc=utf-8 filetype=sh syntax=sh ts=4 sts=4 sw=4 et tw=79:\n"},{"id":"174392","messageId":"c70a246a601bec0ce973b177bf5553d3fee47014.1314472512.git.sdaoden@gmail.com","threadId":"27720","inReplyTo":"cover.1314472512.git.sdaoden@gmail.com","subject":"[PATCH] checkout: be quiet if not on isatty()","fromName":"Steffen Daode Nurpmeso","fromEmail":"sdaoden@googlemail.com","sentAt":"2011-08-27T19:45:52Z","receivedAt":"2011-08-27T19:45:52Z","isPatch":true,"sender":{"key":"sdaoden@googlemail.com","avatar":null},"body":"Signed-off-by: Steffen Daode Nurpmeso \n---\n builtin/checkout.c | 1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 4eaedff..6fb6d48 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -958,6 +958,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n \tgitmodules_config();\n \tgit_config(git_checkout_config, &opts);\n \n+\topts.quiet = !isatty(2);\n \topts.track = BRANCH_TRACK_UNSPECIFIED;\n \n \targc = parse_options(argc, argv, prefix, options, checkout_usage,\n-- \n1.7.6.537.ga80e5.dirty\n"},{"id":"174404","messageId":"7vaaau9hso.fsf@alter.siamese.dyndns.org","threadId":"27720","inReplyTo":"c70a246a601bec0ce973b177bf5553d3fee47014.1314472512.git.sdaoden@gmail.com","subject":"Re: [PATCH] checkout: be quiet if not on isatty()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-08-28T06:22:31Z","receivedAt":"2011-08-28T06:22:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Steffen Daode Nurpmeso writes:\n\n> Signed-off-by: Steffen Daode Nurpmeso \n\nJustification is necessary why this is a good change.\n\nPerhaps you meant something like this:\n\n\tIn general, the progress output should not be given unless the\n output is sent to a tty (i.e. an interactive session).\n\nBut this patch may be squelching the output a bit too much. The opts.quiet\nfield is used not just to set verbose_update in the unpack_trees_options\nused in reset_tree() and merge_working_tree(), but also used to report the\nlocal changes at the end of merge_working_tree(), report tracking\ninformation, and report where the detached HEAD is at, among other things.\n\nIndependently, it might make sense to squelch advice messages in a\nnon-interactive session, but I think that should probably be done by\nflipping advice_* variables in advice.c, I think.\n\n> ---\n> builtin/checkout.c | 1 +\n> 1 files changed, 1 insertions(+), 0 deletions(-)\n>\n> diff --git a/builtin/checkout.c b/builtin/checkout.c\n> index 4eaedff..6fb6d48 100644\n> --- a/builtin/checkout.c\n> +++ b/builtin/checkout.c\n> @@ -958,6 +958,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n> \tgitmodules_config();\n> \tgit_config(git_checkout_config, &opts);\n> \n> +\topts.quiet = !isatty(2);\n> \topts.track = BRANCH_TRACK_UNSPECIFIED;\n> \n> \targc = parse_options(argc, argv, prefix, options, checkout_usage,\n"},{"id":"174407","messageId":"20110828062848.GA22182@albatross.gern.madduck.net","threadId":"27720","inReplyTo":"7vaaau9hso.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] checkout: be quiet if not on isatty()","fromName":"martin f krafft","fromEmail":"madduck@madduck.net","sentAt":"2011-08-28T06:28:48Z","receivedAt":"2011-08-28T06:28:48Z","isPatch":true,"sender":{"key":"madduck@madduck.net","avatar":null},"body":"also sprach Junio C Hamano [2011.08.28.0822 +0200]:\n> \tIn general, the progress output should not be given unless the\n> output is sent to a tty (i.e. an interactive session).\n\nJust as a note — around Unix, it's generally \"output should not be\ngiven unless there was an unexpected condition, or --verbose was\npassed. If a tool did successfully what it was asked to do, it\nshould just be quiet about it.\"\n\n-- \nmartin | http://madduck.net/ | http://two.sentenc.es/\n \n/.ing an issue is like asking an infinite number of monkeys for advice\n -- in #debian-devel\n \nspamtraps: madduck.bogus@madduck.net\n"},{"id":"174426","messageId":"20110828173644.GA1553@sherwood.local","threadId":"27720","inReplyTo":"7vaaau9hso.fsf@alter.siamese.dyndns.org","subject":"[PATCH] checkout: add --verbose, and restrict progress reporting (was: Re: [PATCH] checkout: be quiet if not on isatty())","fromName":"Steffen Daode Nurpmeso","fromEmail":"sdaoden@googlemail.com","sentAt":"2011-08-28T17:37:27Z","receivedAt":"2011-08-28T17:37:27Z","isPatch":true,"sender":{"key":"sdaoden@googlemail.com","avatar":null},"body":"This commit adds support for the -v/--verbose pair of options,\nand thus offers the possibility to be more specific in deciding\nwhich purely informational feedback message is displayed or not.\n\nWithout any of --verbose and --quiet involved, the progress\nreporting is now restricted to interactive sessions, i.e. only\nshown if the output is send to a terminal.\n\nAnalyzed-by: Junio C Hamano \nInspired-by: martin f krafft \nSigned-off-by: Steffen Daode Nurpmeso \n---\n\nWell i was stepping down from my hill, actually singing my\nsunday's song (was it Elvis..), but i didn't dare to implement the\nbehaviour Martin suggested. But isn't he right?\nThis thing here was also tested a bit.\n\n Documentation/git-checkout.txt | 13 +++++++++----\n builtin/checkout.c | 12 +++++++++---\n 2 files changed, 18 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt\nindex c0a96e6..77ad4f3 100644\n--- a/Documentation/git-checkout.txt\n+++ b/Documentation/git-checkout.txt\n@@ -8,9 +8,9 @@ git-checkout - Checkout a branch or paths to the working tree\n SYNOPSIS\n --------\n [verse]\n-'git checkout' [-q] [-f] [-m] []\n-'git checkout' [-q] [-f] [-m] [--detach] []\n-'git checkout' [-q] [-f] [-m] [[-b|-B|--orphan] ] []\n+'git checkout' [-v] [-q] [-f] [-m] []\n+'git checkout' [-v] [-q] [-f] [-m] [--detach] []\n+'git checkout' [-v] [-q] [-f] [-m] [[-b|-B|--orphan] ] []\n 'git checkout' [-f|--ours|--theirs|-m|--conflict=