{"thread":{"id":"5713","subject":"[PATCH 3/3] diff --stat: sometimes use non-linear scaling.","startedAt":"2006-09-27T02:40:53Z","lastAt":"2006-10-06T15:53:31Z","messageCount":18,"participants":["Junio C Hamano","David Rientjes","Johannes Schindelin","Sean","Martin Waitz","Linus Torvalds","Andreas Ericsson","Petr Baudis"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"27738","messageId":"7vfyeejakq.fsf@assigned-by-dhcp.cox.net","threadId":"5713","inReplyTo":null,"subject":"[PATCH 3/3] diff --stat: sometimes use non-linear scaling.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-09-27T02:40:53Z","receivedAt":"2006-09-27T02:40:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When some files have big changes and others are touched only\nslightly, diffstat graph did not show differences among smaller\nchanges that well. This changes the graph scaling to non-linear\nalgorithm in such a case.\n\nWithout this, \"git show --stat fd88d9c\" gives:\n\n .gitignore | 1\n Documentation/git-tar-tree.txt | 3 +\n Documentation/git-upload-tar.txt | 39 -----------\n Documentation/git.txt | 4 -\n Makefile | 1\n builtin-tar-tree.c | 130 +++++++++++++++-----------------------\n builtin-upload-tar.c | 74 ----------------------\n git.c | 1\n 8 files changed, 53 insertions(+), 200 deletions(-)\n\nwhile with this, it shows:\n\n .gitignore | 1\n Documentation/git-tar-tree.txt | 3 +++++++++\n Documentation/git-upload-tar.txt | 39 -----------------------------\n Documentation/git.txt | 4 -----------\n Makefile | 1\n builtin-tar-tree.c | 130 +++++++++++++++-----------------------\n builtin-upload-tar.c | 74 ----------------------------------\n git.c | 1\n 8 files changed, 53 insertions(+), 200 deletions(-)\n\nSigned-off-by: Junio C Hamano \n---\n\n * Jan Engelhardt wondered about doing non-linear scaling on the\n kernel list and this is an experimental patch to do so. I do\n not seriously consider this for inclusion but it is more of a\n \"see if people like it\" patch.\n\n Makefile | 2 +-\n diff.c | 29 +++++++++++++++++++++++++++--\n 2 files changed, 28 insertions(+), 3 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 28091d6..0fc59c4 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -304,7 +304,7 @@ BUILTIN_OBJS = \\\n \tbuiltin-write-tree.o\n \n GITLIBS = $(LIB_FILE) $(XDIFF_LIB)\n-LIBS = $(GITLIBS) -lz\n+LIBS = $(GITLIBS) -lz -lm\n \n #\n # Platform specific tweaks\ndiff --git a/diff.c b/diff.c\nindex 13aac2d..163ef48 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -4,6 +4,7 @@\n #include \n #include \n #include \n+#include \n #include \"cache.h\"\n #include \"quote.h\"\n #include \"diff.h\"\n@@ -555,6 +556,16 @@ static int scale_linear(int it, int widt\n \treturn (it * width * 2 + max_change) / (max_change * 2);\n }\n \n+static int scale_non_linear(int it, int width, int max_change)\n+{\n+\t/*\n+\t * round(width * log(it)/log(max_change))\n+\t */\n+\tif (!it || !max_change)\n+\t\treturn 0;\n+\treturn (int)(0.5 + width * log(it) / log(max_change));\n+}\n+\n static void show_name(const char *prefix, const char *name, int len,\n \t\t const char *reset, const char *set)\n {\n@@ -574,10 +585,11 @@ static void show_graph(char ch, int cnt,\n static void show_stats(struct diffstat_t* data, struct diff_options *options)\n {\n \tint i, len, add, del, total, adds = 0, dels = 0;\n-\tint max_change = 0, max_len = 0;\n+\tint max_change = 0, max_len = 0, min_change = 0;\n \tint total_files = data->nr;\n \tint width, name_width;\n \tconst char *reset, *set, *add_c, *del_c;\n+\tint non_linear_scale = 0;\n \n \tif (data->nr == 0)\n \t\treturn;\n@@ -595,12 +607,12 @@ static void show_stats(struct diffstat_t\n \t\t\twidth = name_width + 15;\n \t}\n \n-\t/* Find the longest filename and max number of changes */\n \treset = diff_get_color(options->color_diff, DIFF_RESET);\n \tset = diff_get_color(options->color_diff, DIFF_PLAIN);\n \tadd_c = diff_get_color(options->color_diff, DIFF_FILE_NEW);\n \tdel_c = diff_get_color(options->color_diff, DIFF_FILE_OLD);\n \n+\t/* Find the longest filename and max/min number of changes */\n \tfor (i = 0; i < data->nr; i++) {\n \t\tstruct diffstat_file *file = data->files[i];\n \t\tint change = file->added + file->deleted;\n@@ -620,6 +632,8 @@ static void show_stats(struct diffstat_t\n \t\t\tcontinue;\n \t\tif (max_change < change)\n \t\t\tmax_change = change;\n+\t\tif (0 < change && (!min_change || change < min_change))\n+\t\t\tmin_change = change;\n \t}\n \n \t/* Compute the width of the graph part;\n@@ -635,6 +649,12 @@ static void show_stats(struct diffstat_t\n \telse\n \t\twidth = max_change;\n \n+\t/* See if the minimum change is shown with the normal scale\n+\t * and if not switch to non-linear scale\n+\t */\n+\tif (min_change && !scale_linear(min_change, width, max_change))\n+\t\tnon_linear_scale = 1;\n+\n \tfor (i = 0; i < data->nr; i++) {\n \t\tconst char *prefix = \"\";\n \t\tchar *name = data->files[i]->name;\n@@ -684,6 +704,11 @@ static void show_stats(struct diffstat_t\n \n \t\tif (max_change < width)\n \t\t\t;\n+\t\telse if (non_linear_scale) {\n+\t\t\ttotal = scale_non_linear(total, width, max_change);\n+\t\t\tadd = scale_linear(add, total, add + del);\n+\t\t\tdel = total - add;\n+\t\t}\n \t\telse {\n \t\t\ttotal = scale_linear(total, width, max_change);\n \t\t\tadd = scale_linear(add, width, max_change);\n-- \n1.4.2.1.gf80a\n"},{"id":"27748","messageId":"Pine.LNX.4.64N.0609262005150.520@attu4.cs.washington.edu","threadId":"5713","inReplyTo":"7vfyeejakq.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 3/3] diff --stat: sometimes use non-linear scaling.","fromName":"David Rientjes","fromEmail":"rientjes@cs.washington.edu","sentAt":"2006-09-27T03:11:32Z","receivedAt":"2006-09-27T03:11:32Z","isPatch":true,"sender":{"key":"rientjes@cs.washington.edu","avatar":null},"body":"On Tue, 26 Sep 2006, Junio C Hamano wrote:\n\n> @@ -574,10 +585,11 @@ static void show_graph(char ch, int cnt,\n> static void show_stats(struct diffstat_t* data, struct diff_options *options)\n> {\n> \tint i, len, add, del, total, adds = 0, dels = 0;\n> -\tint max_change = 0, max_len = 0;\n> +\tint max_change = 0, max_len = 0, min_change = 0;\n> \tint total_files = data->nr;\n> \tint width, name_width;\n> \tconst char *reset, *set, *add_c, *del_c;\n> +\tint non_linear_scale = 0;\n> \n> \tif (data->nr == 0)\n> \t\treturn;\n> @@ -620,6 +632,8 @@ static void show_stats(struct diffstat_t\n> \t\t\tcontinue;\n> \t\tif (max_change < change)\n> \t\t\tmax_change = change;\n> +\t\tif (0 < change && (!min_change || change < min_change))\n> +\t\t\tmin_change = change;\n> \t}\n\nAgain with the constant placement in a comparison expression.\n\n> @@ -684,6 +704,11 @@ static void show_stats(struct diffstat_t\n> \n> \t\tif (max_change < width)\n> \t\t\t;\n> +\t\telse if (non_linear_scale) {\n> +\t\t\ttotal = scale_non_linear(total, width, max_change);\n> +\t\t\tadd = scale_linear(add, total, add + del);\n> +\t\t\tdel = total - add;\n> +\t\t}\n> \t\telse {\n> \t\t\ttotal = scale_linear(total, width, max_change);\n> \t\t\tadd = scale_linear(add, width, max_change);\n> \n\nif (...)\n\t;\nelse if {\n\t...\n}\n\nis _never_ necessary.\n\n\t\tDavid\n"},{"id":"27757","messageId":"7vmz8lj3pl.fsf@assigned-by-dhcp.cox.net","threadId":"5713","inReplyTo":"Pine.LNX.4.64N.0609262005150.520@attu4.cs.washington.edu","subject":"Re: [PATCH 3/3] diff --stat: sometimes use non-linear scaling.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-09-27T05:09:10Z","receivedAt":"2006-09-27T05:09:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Rientjes writes:\n\n> Again with the constant placement in a comparison expression.\n\nI won't comment on this one. See list archives ;-).\n\n>> \t\tif (max_change < width)\n>> \t\t\t;\n>> +\t\telse if (non_linear_scale) {\n>> +\t\t\ttotal = scale_non_linear(total, width, max_change);\n>> +\t\t\tadd = scale_linear(add, total, add + del);\n>> +\t\t\tdel = total - add;\n>> +\t\t}\n>> \t\telse {\n>> \t\t\ttotal = scale_linear(total, width, max_change);\n>> \t\t\tadd = scale_linear(add, width, max_change);\n>> \n>\n> if (...)\n> \t;\n> else if {\n> \t...\n> }\n>\n> is _never_ necessary.\n\nWhat's happening here in this particular case is:\n\n\tif the changes fits within the alloted width\n\t\t; /* we do not have to do anything */\n\telse if we are using non-linear scale {\n \tscale it like this\n\t}\n\telse {\n \tscale it like that\n\t}\n\nso the code actually matches the flow of thought perfectly well.\n\nI first tried to write it without \"if () ;/*empty*/ else\" chain\nlike this:\n\n\tif given width is narrower than changes we have {\n \tif we are doing non-linear scale {\n \tscale it like this\n }\n else {\n \tscale it like that\n\t\t}\n\t}\n\n\nIt made the indentation unnecessarily deep.\n"},{"id":"27759","messageId":"Pine.LNX.4.64N.0609262216390.12560@attu2.cs.washington.edu","threadId":"5713","inReplyTo":"7vmz8lj3pl.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 3/3] diff --stat: sometimes use non-linear scaling.","fromName":"David Rientjes","fromEmail":"rientjes@cs.washington.edu","sentAt":"2006-09-27T05:32:47Z","receivedAt":"2006-09-27T05:32:47Z","isPatch":true,"sender":{"key":"rientjes@cs.washington.edu","avatar":null},"body":"On Tue, 26 Sep 2006, Junio C Hamano wrote:\n\n> David Rientjes writes:\n> \n> > Again with the constant placement in a comparison expression.\n> \n> I won't comment on this one. See list archives ;-).\n> \n\nI'm very familiar with the list archives and your support of writing \nrelationals like 0 < x. It's a matter of taste. And since the large \nmajority of programmers in any language write x > 0 instead, I think it's \npreferrable to write code that is in the style and taste of the majority.\n\nLarge software projects require a conformity in the style in which the \ncode is written. Granted the git developer community is small, there is \nstill a need for this confomity so that developers don't have to put up \nwith the subtleties in the style of which individuals decide to code.\n\nWhen I read \"x > 0\", my mind parses that very easily. When I read \"0 < \nx\", it takes me a few cycles longer. I think the goal of any software \nproject is to not only emit efficient and quality code, but also code that \ncan be read and deciphered with ease unless it's impossible otherwise.\n\n> What's happening here in this particular case is:\n> \n> \tif the changes fits within the alloted width\n> \t\t; /* we do not have to do anything */\n> \telse if we are using non-linear scale {\n> \tscale it like this\n> \t}\n> \telse {\n> \tscale it like that\n> \t}\n> \n> so the code actually matches the flow of thought perfectly well.\n> \n> I first tried to write it without \"if () ;/*empty*/ else\" chain\n> like this:\n> \n> \tif given width is narrower than changes we have {\n> \tif we are doing non-linear scale {\n> \tscale it like this\n> }\n> else {\n> \tscale it like that\n> \t\t}\n> \t}\n> \n> \n> It made the indentation unnecessarily deep.\n> \n\nTo change the code itself because of a hard 80-column limit or because \nyou're tired of hitting the tab key is poor style. The idents are there \nfor a purpose: it tells the reader that the code is inside a block. So \nwhen this conditional becomes a screen wide, I can understand it on the \nsecond screen and remember that I'm inside a conditional and not rely on \nthe previous 'else' to jog my memory. C is not a whitespace-dependent \nlanguage like Python, but since when did idents (which are there _solely_ \nfor the purpose of helping the reader) become deprecated?\n\n\t\tDavid\n"},{"id":"27763","messageId":"7vejtxhlv6.fsf@assigned-by-dhcp.cox.net","threadId":"5713","inReplyTo":"Pine.LNX.4.64N.0609262216390.12560@attu2.cs.washington.edu","subject":"Re: [PATCH 3/3] diff --stat: sometimes use non-linear scaling.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-09-27T06:19:57Z","receivedAt":"2006-09-27T06:19:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Rientjes writes:\n\n> When I read \"x > 0\", my mind parses that very easily. When I read \"0 < \n> x\", it takes me a few cycles longer. I think the goal of any software \n> project is to not only emit efficient and quality code, but also code that \n> can be read and deciphered with ease unless it's impossible otherwise.\n\nWell, the thing is, I end up being the guy who needs to stare at\ngit code longer than you do ;-).\n\nBefore --stat-width was introduced there was code like this:\n\n\tif (max + len > 70)\n\t\tmax = 70 - len;\n\nHere \"len\" is the width of the filename part, and \"max\" is the\nnumber of changes we need to express. The code is saying \"if we\nuse one column for each changed line, does graph and name exceed\n70 columns -- if so use the remainder of the line after we write\nname for the graph\". Your \"constant at right\" rule makes this\nkosher.\n\nIf we make that to a variable, say line_width, we can still\nwrite:\n\n\tif (max + len > line_width)\n \t...\n\nI however tend to think \"if line_width cannot fit (max + len)\nthen we do this\", which would be more naturally expressed with:\n\n\tif (line_width < max + len)\n \t...\n\nNow, at this point, it is really the matter of taste and there\nis no real reason to prefer one over the other. Textual\nordering lets my eyes coast while reading the code without\ntaxing the brain. I can see that the expression compares two\nquantities, \"line_width\" and \"max + len\", and the boolean holds\ntrue if line_width _comes_ _before_ \"max + len\" on the number\nline (having number line in your head helps visualizing what is\ncompared with what). If you write the comparison the wrong way,\nit forces me to stop and think -- because on my number line\nsmaller numbers appear left, and cannot help me reading the\ncomparison written in \"a > b\" order.\n\nI could try writing constants on the right hand side when\nconstants are involved, but I do not think it makes much sense.\nIt means that I would end up doing:\n\n-\tif (max + len > 70)\n-\t\tmax = 70 - len;\n+\tif (line_width < max + len)\n+\t\tmax = line_width - len;\n\nConsistency counts not only while reading the finished code, but\nalso it helps reviewing the diff between the earlier version\nthat used constant (hence forced to have it on the right hand\nside by your rule) and the version that made it into a variable.\n\n> To change the code itself because of a hard 80-column limit or because \n> you're tired of hitting the tab key is poor style.\n\nWell, the program _firstly_ matches the logic flow better, and\n_in_ _addition_ if you write it another way it becomes\nunnecessarily too deeply indented. So while I agree with you as a\ngeneral principle that indentation depth should not dictate how\nwe code it does not apply to this particular example.\n"},{"id":"27766","messageId":"Pine.LNX.4.64N.0609262320260.9088@attu4.cs.washington.edu","threadId":"5713","inReplyTo":"7vejtxhlv6.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 3/3] diff --stat: sometimes use non-linear scaling.","fromName":"David Rientjes","fromEmail":"rientjes@cs.washington.edu","sentAt":"2006-09-27T06:49:20Z","receivedAt":"2006-09-27T06:49:20Z","isPatch":true,"sender":{"key":"rientjes@cs.washington.edu","avatar":null},"body":"On Tue, 26 Sep 2006, Junio C Hamano wrote:\n\n> Well, the thing is, I end up being the guy who needs to stare at\n> git code longer than you do ;-).\n> \n\nReally? This is the only community that hacks git? There _are_ people \nout there that make their own changes specifically tailored to their \npurposes or that of their organization.\n\n> Before --stat-width was introduced there was code like this:\n> \n> \tif (max + len > 70)\n> \t\tmax = 70 - len;\n> \n> Here \"len\" is the width of the filename part, and \"max\" is the\n> number of changes we need to express. The code is saying \"if we\n> use one column for each changed line, does graph and name exceed\n> 70 columns -- if so use the remainder of the line after we write\n> name for the graph\". Your \"constant at right\" rule makes this\n> kosher.\n> \n> If we make that to a variable, say line_width, we can still\n> write:\n> \n> \tif (max + len > line_width)\n> \t...\n> \n> I however tend to think \"if line_width cannot fit (max + len)\n> then we do this\", which would be more naturally expressed with:\n> \n> \tif (line_width < max + len)\n> \t...\n> \n\nFirst of all, it's not my \"constant at right\" rule, it's a preference that \nthe _majority_ of computer programmers have used in virtually every \nlanguage that you see source code for.\n\nThe grammar for C is\n\n relational-expression:\n\tshift-expression\n\trelational-expression < shift-expression\n\nin this case. Now while this supports both your variations above, it \n_suggests_ that the higher degree of computation is associated on the left \nside because the less-than operator associates that way.\n\nWhat happens here:\n\ta < b < c\n\nit turns out that this is equivalent to:\n\t(a < b) < c\n\nso if you want your entire code base to conform to a particular style, \nit's _preferable_ to place the constant on the right. And that's what the \nmajority of programmers do. Your taste is in the minority and out of \nrespect to the code base you should make your code conform to what is most \npopular in the surrounding code.\n\nYour argument of saying to yourself \"if line_width cannot fit max + len \nthen we do this\" has no relevance at all. I can say \"if max + len is too \nbig for line_width we do this\" just the same.\n\nIf we're going by what sounds better in your head, then I expect _no_ \nargument when I write a function called conseguir_la_linea_longitud \ninstead of get_line_length because Spanish is my first language.\n\nPlease respect what the majority of computer programmers write and unify \nthe code base so that it's a similar style everywhere.\n\n> > To change the code itself because of a hard 80-column limit or because \n> > you're tired of hitting the tab key is poor style.\n> \n> Well, the program _firstly_ matches the logic flow better, and\n> _in_ _addition_ if you write it another way it becomes\n> unnecessarily too deeply indented. So while I agree with you as a\n> general principle that indentation depth should not dictate how\n> we code it does not apply to this particular example.\n> \n\nThis is a ridiculous argument. The C code will emit the exact same \nassembly regardless of how you write it. You say that you wrote it that \nway to avoid idents which is an absolutely horrible way to dictate the \ncode you use. There are tons of opportunities where you can write cryptic \nsource code that functions great with the least number of tokens and least \nnumber of lines to get the job done in every large project. But, given \nthat there are no assembler or performance tradeoffs, it should be written \nas clearly and nicely as possible for the reader. I assert again what I \ndid previously: if that if clause runs the length of my screen the indents \nwill help me later to remember we're still in a conditional. That's the \nSOLE purpose of indents: to make it easy for the reader to tell you're \ninside a block. \n\nAnd in one of your patches you had:\n\tif (...)\n\t\t;\n\telse {\n\t\t...\n\t}\n\nwithout any other if statements. If you're supporting that type of code, \nI'll simply consider this entire thread a lost cause.\n\n\t\tDavid\n"},{"id":"27767","messageId":"7vfyedg56m.fsf@assigned-by-dhcp.cox.net","threadId":"5713","inReplyTo":"Pine.LNX.4.64N.0609262320260.9088@attu4.cs.washington.edu","subject":"Re: [PATCH 3/3] diff --stat: sometimes use non-linear scaling.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-09-27T07:05:37Z","receivedAt":"2006-09-27T07:05:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Rientjes writes:\n\n> Your argument of saying to yourself \"if line_width cannot fit max + len \n> then we do this\" has no relevance at all. I can say \"if max + len is too \n> big for line_width we do this\" just the same.\n\nActually that is exactly my point. \"Just the same\". There is\nno reason to choose one way or the other from purely logical or\nmathematical point of view.\n\nComparisons written always in textual order, when one gets used\nto, takes less thinking to parse and understand, and that is\nwhat I'm used to. Have number line handy in your head and you\nwill hopefully like it too ;-).\n\n>> Well, the program _firstly_ matches the logic flow better, and\n>> _in_ _addition_ if you write it another way it becomes\n>> unnecessarily too deeply indented. So while I agree with you as a\n>> general principle that indentation depth should not dictate how\n>> we code it does not apply to this particular example.\n>\n> This is a ridiculous argument. The C code will emit the exact same \n> assembly regardless of how you write it. You say that you wrote it that \n> way to avoid idents which is an absolutely horrible way to dictate the \n> code you use.\n\nI guess probably I was unclear (I did not talk anything about\ncode generation -- where did it come from?). I say I wrote it\nthat way _firstly_ because the flow of the program matches\nexactly what I saw the code needed to do -- if A I do not have\nto do anything else if B I do this else I do that. In addition\nnot having that \"do nothing\" made the code indent unnecessarily\ndeep but that is \"in addition\" and not the primary cause. It\nwas an added bonus.\n\n> And in one of your patches you had:\n> \tif (...)\n> \t\t;\n> \telse {\n> \t\t...\n> \t}\n>\n> without any other if statements.\n\nYes, indeed that was very funny looking.\n\nIt was refactored from the final one that had \"else if\" in the\nmiddle (else if was to add the non-linear scaling). I agree\nthat any sane would not have done that if that was the real\nfirst version.\n"},{"id":"27768","messageId":"Pine.LNX.4.64N.0609270006020.9602@attu4.cs.washington.edu","threadId":"5713","inReplyTo":"7vfyedg56m.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 3/3] diff --stat: sometimes use non-linear scaling.","fromName":"David Rientjes","fromEmail":"rientjes@cs.washington.edu","sentAt":"2006-09-27T07:19:02Z","receivedAt":"2006-09-27T07:19:02Z","isPatch":true,"sender":{"key":"rientjes@cs.washington.edu","avatar":null},"body":"On Wed, 27 Sep 2006, Junio C Hamano wrote:\n\n> David Rientjes writes:\n> \n> > Your argument of saying to yourself \"if line_width cannot fit max + len \n> > then we do this\" has no relevance at all. I can say \"if max + len is too \n> > big for line_width we do this\" just the same.\n> \n> Actually that is exactly my point. \"Just the same\". There is\n> no reason to choose one way or the other from purely logical or\n> mathematical point of view.\n> \n\nNothing about this is \"mathematical\" at all and I never claimed it was. \nBut there _is_ a reason to choose one way over the other and that is \nbecause the MAJORITY of programmers do it one way and YOU do it another \nway. Why is it so hard to write all the code in the same style so that \nthere is as little variation in the code as possible?\n\n> Comparisons written always in textual order, when one gets used\n> to, takes less thinking to parse and understand, and that is\n> what I'm used to. Have number line handy in your head and you\n> will hopefully like it too ;-).\n> \n\nDoing what you prefer and not what the majority of your developers do is \nthe first step to a stagnant source tree.\n\n> >> Well, the program _firstly_ matches the logic flow better, and\n> >> _in_ _addition_ if you write it another way it becomes\n> >> unnecessarily too deeply indented. So while I agree with you as a\n> >> general principle that indentation depth should not dictate how\n> >> we code it does not apply to this particular example.\n> >\n> > This is a ridiculous argument. The C code will emit the exact same \n> > assembly regardless of how you write it. You say that you wrote it that \n> > way to avoid idents which is an absolutely horrible way to dictate the \n> > code you use.\n> \n> I guess probably I was unclear (I did not talk anything about\n> code generation -- where did it come from?). I say I wrote it\n> that way _firstly_ because the flow of the program matches\n> exactly what I saw the code needed to do -- if A I do not have\n> to do anything else if B I do this else I do that. In addition\n> not having that \"do nothing\" made the code indent unnecessarily\n> deep but that is \"in addition\" and not the primary cause. It\n> was an added bonus.\n> \n\nThe concept of code generation is the whole point. Both of our styles \nemits the same assembly code so by definition there is no difference since \nit achieves the exact same goal. But there's a reason git is written in C \nand not in assembly and that reason is not just for portability. A \n.c source file is the bridge between most programmers and assembly and \nsince our coding styles differ but emit the same assembly, then it \ninherently comes with a freedom in how it's written. And on a \ncollaborative project such as git, that freedom should be confined to \nresembling the surrounding source code.\n\n\t\tDavid\n"},{"id":"27770","messageId":"Pine.LNX.4.63.0609270929590.14200@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"5713","inReplyTo":"7vfyeejakq.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 3/3] diff --stat: sometimes use non-linear scaling.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-09-27T07:36:51Z","receivedAt":"2006-09-27T07:36:51Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 26 Sep 2006, Junio C Hamano wrote:\n\n> When some files have big changes and others are touched only\n> slightly, diffstat graph did not show differences among smaller\n> changes that well. This changes the graph scaling to non-linear\n> algorithm in such a case.\n\nI want to say something about the purpose of the patch, not some totally \nunimportant superficialities.\n\nIn your example, a three line change has more than three plusses, and I \nfind that wrong.\n\nBut I would actually find another change very useful: still linear, but \nsuch that if lines were added, at least one plus should be shown, and \nlikewise with minus. (Often I ask myself, was this file removed, or just \ndramatically reduced, when I only see minusses).\n\nCiao,\nDscho\n"},{"id":"27771","messageId":"Pine.LNX.4.63.0609270948140.14200@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"5713","inReplyTo":"Pine.LNX.4.64N.0609270006020.9602@attu4.cs.washington.edu","subject":"Re: [PATCH 3/3] diff --stat: sometimes use non-linear scaling.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-09-27T07:50:11Z","receivedAt":"2006-09-27T07:50:11Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 27 Sep 2006, David Rientjes wrote:\n\n> On Wed, 27 Sep 2006, Junio C Hamano wrote:\n> \n> > David Rientjes writes:\n> > \n> > > Your argument of saying to yourself \"if line_width cannot fit max + len \n> > > then we do this\" has no relevance at all. I can say \"if max + len is too \n> > > big for line_width we do this\" just the same.\n> > \n> > Actually that is exactly my point. \"Just the same\". There is\n> > no reason to choose one way or the other from purely logical or\n> > mathematical point of view.\n> > \n> \n> Nothing about this is \"mathematical\" at all and I never claimed it was. \n> But there _is_ a reason to choose one way over the other and that is \n> because the MAJORITY of programmers do it one way and YOU do it another \n> way. Why is it so hard to write all the code in the same style so that \n> there is as little variation in the code as possible?\n\nCould you stop it already?\n\nGit's source code is very clean and readable, even if there are inversions \nyou might not be used to.\n\nBesides, always doing it the same way is boring. _Boring_. Or do you make \nlove to your girl-friend the same way over and over again?\n\nCiao,\nDscho\n"},{"id":"27774","messageId":"Pine.LNX.4.63.0609271030180.14200@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"5713","inReplyTo":"BAYC1-PASMTP024D1DA4730F9DF93F857FAE1A0@CEZ.ICE","subject":"Re: [PATCH 3/3] diff --stat: sometimes use non-linear scaling.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-09-27T08:35:16Z","receivedAt":"2006-09-27T08:35:16Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 27 Sep 2006, Sean wrote:\n\n> On Wed, 27 Sep 2006 09:50:11 +0200 (CEST)\n> Johannes Schindelin wrote:\n> \n> > Could you stop it already?\n> \n> Well i'd like to offer some support for David.\n> \n> In English you'd never say \"if 10 is less than the number of girls\"\n> you'd always say \"if the number of girls is greater than 10\".\n> \n> Why on earth would you ever write C code different than the way you'd\n> express the same question in natural language? Maybe this is only common\n> in English and other languages are different; that would explain why this\n> seems more natural to some.\n\nIn this case, though, \"English\" is utterly, totally irrelevant. The \nquestion is a mathematical one, and thus, the solution is a mathematical \none.\n\nSo, in essence, if you do not understand a conditional with a constant on \nthe left side, just because it happens to honour the mathematical view of \n\"left is small, right is large\", you do not stand a chance of \nunderstanding the formula, right?\n\n> > Git's source code is very clean and readable, even if there are inversions \n> > you might not be used to.\n> \n> Not to me. I find it very annoying to have to figure out what\n> \"if ( 10 < x ) ...\" is really trying to do.\n\nOh, come on! You cannot possibly spend even _seconds_ on this particular \nconstruct!\n\n'nough said.\n\nCiao,\nDscho\n"},{"id":"27775","messageId":"BAYC1-PASMTP059765F6CE0979DC2F8D5AAE1A0@CEZ.ICE","threadId":"5713","inReplyTo":"Pine.LNX.4.63.0609271030180.14200@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH 3/3] diff --stat: sometimes use non-linear scaling.","fromName":"Sean","fromEmail":"seanlkml@sympatico.ca","sentAt":"2006-09-27T08:41:12Z","receivedAt":"2006-09-27T08:41:12Z","isPatch":true,"sender":{"key":"seanlkml@sympatico.ca","avatar":"https://gravatar.com/avatar/f92923f54fc08c401fc59b71829d4b89e9b8087fbba45ff87c82e6a83aee02ae?d=mp&s=160"},"body":"On Wed, 27 Sep 2006 10:35:16 +0200 (CEST)\nJohannes Schindelin wrote:\n\n> In this case, though, \"English\" is utterly, totally irrelevant. The \n> question is a mathematical one, and thus, the solution is a mathematical \n> one.\n\nWell, no.. At least for me, I \"think\" in english, not mathematics. And\nthus I have to understand each condition in my native language. And i'm\nbeing honest with you when I tell you that my parser hiccups every time\nI see such a construct.\n\n> So, in essence, if you do not understand a conditional with a constant on \n> the left side, just because it happens to honour the mathematical view of \n> \"left is small, right is large\", you do not stand a chance of \n> understanding the formula, right?\n\nIt's not a matter of being able to understand, it's being able to digest\nat a glance, almost without a thought as opposed to consciously having\nto rearrange the arguments into something that \"feels\" right.\n\n> Oh, come on! You cannot possibly spend even _seconds_ on this particular \n> construct!\n> \n> 'nough said.\n\nI'm telling you that it is disconcerting and annoying to have to rejig such\na construct. Whereas when expressed in the opposite format it makes reading\nsimple and natural. Making the code easier and more pleasurable to read.\n\nAnd if you find it so easy to read either way, then why not bend for those\nof us who have trouble reading it your way instead of just telling us to get\nstuffed?\n\nSean\n"},{"id":"27777","messageId":"20060927091620.GB8056@admingilde.org","threadId":"5713","inReplyTo":"7vfyeejakq.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 3/3] diff --stat: sometimes use non-linear scaling.","fromName":"Martin Waitz","fromEmail":"tali@admingilde.org","sentAt":"2006-09-27T09:16:20Z","receivedAt":"2006-09-27T09:16:20Z","isPatch":true,"sender":{"key":"tali@admingilde.org","avatar":"https://gravatar.com/avatar/3f89b03eee362187effabe257898735b475673a12265c398ea9161259ae91553?d=mp&s=160"},"body":"hoi :)\n\nOn Tue, Sep 26, 2006 at 07:40:53PM -0700, Junio C Hamano wrote:\n> .gitignore | 1\n> Documentation/git-tar-tree.txt | 3 +++++++++\n> Documentation/git-upload-tar.txt | 39 -----------------------------\n> Documentation/git.txt | 4 -----------\n> Makefile | 1\n> builtin-tar-tree.c | 130 +++++++++++++++-----------------------\n> builtin-upload-tar.c | 74 ----------------------------------\n> git.c | 1\n> 8 files changed, 53 insertions(+), 200 deletions(-)\n\nhmm, the small changes (1 line) are still not shown :-(.\nI like the idea of non-linear display, but we have to fine-tune the\nalgorithm a little bit more.\n\n-- \nMartin Waitz\n"},{"id":"27792","messageId":"Pine.LNX.4.64.0609270810470.3952@g5.osdl.org","threadId":"5713","inReplyTo":"7vfyeejakq.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 3/3] diff --stat: sometimes use non-linear scaling.","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-09-27T15:12:49Z","receivedAt":"2006-09-27T15:12:49Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 26 Sep 2006, Junio C Hamano wrote:\n>\n> When some files have big changes and others are touched only\n> slightly, diffstat graph did not show differences among smaller\n> changes that well. This changes the graph scaling to non-linear\n> algorithm in such a case.\n\nOk, this is just _strange_.\n\n> while with this, it shows:\n> \n> .gitignore | 1\n> Documentation/git-tar-tree.txt | 3 +++++++++\n\nNo _way_ is it correct to show more than three characters if there were \nthree lines of changes.\n\nI think \"nonlinear\" is fine, but this is something that is \"superlinear\" \nin small changes, and then sublinear in bigger ones (and then apparently \ntotally wrong for one-line changes).\n\nIt should at least never be superlinear, I believe.\n\n\t\tLinus\n"},{"id":"27879","messageId":"20060928081757.GF8056@admingilde.org","threadId":"5713","inReplyTo":"Pine.LNX.4.64.0609270810470.3952@g5.osdl.org","subject":"Re: [PATCH 3/3] diff --stat: sometimes use non-linear scaling.","fromName":"Martin Waitz","fromEmail":"tali@admingilde.org","sentAt":"2006-09-28T08:17:57Z","receivedAt":"2006-09-28T08:17:57Z","isPatch":true,"sender":{"key":"tali@admingilde.org","avatar":"https://gravatar.com/avatar/3f89b03eee362187effabe257898735b475673a12265c398ea9161259ae91553?d=mp&s=160"},"body":"hoi :)\n\nOn Wed, Sep 27, 2006 at 08:12:49AM -0700, Linus Torvalds wrote:\n> No _way_ is it correct to show more than three characters if there were \n> three lines of changes.\n> \n> I think \"nonlinear\" is fine, but this is something that is \"superlinear\" \n> in small changes, and then sublinear in bigger ones (and then apparently \n> totally wrong for one-line changes).\n> \n> It should at least never be superlinear, I believe.\n\nSo if we want to keep the logarithmic scale we can do some maths:\n\nAssume we use a formula ala\n\n\tlength = a log(change + b) + c\n\nwith three invariants a, b, and c.\n\nWe want to scale linearly at first, but want to reach width at\nmax_change:\n\n\t0 = a log(b) + c\n\t1 = a log(b + 1) + c\n\twidth = a log(max_change + b) + c\n\nBut only I have not succeeded in solving these equations, I always stop\nat the last invariant :-(\n\n-- \nMartin Waitz\n"},{"id":"27880","messageId":"7v64f8xs7p.fsf@assigned-by-dhcp.cox.net","threadId":"5713","inReplyTo":"20060928081757.GF8056@admingilde.org","subject":"Re: [PATCH 3/3] diff --stat: sometimes use non-linear scaling.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-09-28T09:20:42Z","receivedAt":"2006-09-28T09:20:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Waitz writes:\n\n>> It should at least never be superlinear, I believe.\n>\n> So if we want to keep the logarithmic scale we can do some maths:\n>...\n> But only I have not succeeded in solving these equations, I always stop\n> at the last invariant :-(\n\nThere is another constraint you did not mention. Here is the\noutput from my another failed experiment:\n\n .gitignore | 1 -\n Documentation/git-tar-tree.txt | 3 +++\n Documentation/git-upload-tar.txt | 39 -----------------------------\n Documentation/git.txt | 4 ----\n Makefile | 1 -\n builtin-tar-tree.c | 130 +++++++++++++++-----------------------\n builtin-upload-tar.c | 74 ----------------------------------\n git.c | 1 -\n 8 files changed, 53 insertions(+), 200 deletions(-)\n\nThe deletion from Documentation/git-upload-tar.txt looks much\nlarger than addition to builtin-tar-tree.c in the above, but\nthere are 50 lines added to builtin-tar-tree.c (which is why\nthis experiment is a failure).\n\nBecause we are dealing with non-linear scaling, the total of\nscaled adds and scaled deletes does not equal to scaled total.\nWe can deal with this in two ways. Scale the total and\ndistribute it, or scale adds and deletes individually and make\nsure the sum of scaled adds and deletes never exceed the width.\nObviously the former is easier to implement but it was _wrong_.\n\nThe fitting algorithm in the posted patch scales the total to\nfit the alloted width and then distributes the result to adds\nand deletes.\n"},{"id":"27991","messageId":"451CFBC5.3020006@op5.se","threadId":"5713","inReplyTo":"7v64f8xs7p.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 3/3] diff --stat: sometimes use non-linear scaling.","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2006-09-29T10:56:05Z","receivedAt":"2006-09-29T10:56:05Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Junio C Hamano wrote:\n> Martin Waitz writes:\n> \n>>> It should at least never be superlinear, I believe.\n>> So if we want to keep the logarithmic scale we can do some maths:\n>> ...\n>> But only I have not succeeded in solving these equations, I always stop\n>> at the last invariant :-(\n> \n> There is another constraint you did not mention. Here is the\n> output from my another failed experiment:\n> \n> .gitignore | 1 -\n> Documentation/git-tar-tree.txt | 3 +++\n> Documentation/git-upload-tar.txt | 39 -----------------------------\n> Documentation/git.txt | 4 ----\n> Makefile | 1 -\n> builtin-tar-tree.c | 130 +++++++++++++++-----------------------\n> builtin-upload-tar.c | 74 ----------------------------------\n> git.c | 1 -\n> 8 files changed, 53 insertions(+), 200 deletions(-)\n> \n> The deletion from Documentation/git-upload-tar.txt looks much\n> larger than addition to builtin-tar-tree.c in the above, but\n> there are 50 lines added to builtin-tar-tree.c (which is why\n> this experiment is a failure).\n> \n> Because we are dealing with non-linear scaling, the total of\n> scaled adds and scaled deletes does not equal to scaled total.\n> We can deal with this in two ways. Scale the total and\n> distribute it, or scale adds and deletes individually and make\n> sure the sum of scaled adds and deletes never exceed the width.\n> Obviously the former is easier to implement but it was _wrong_.\n> \n> The fitting algorithm in the posted patch scales the total to\n> fit the alloted width and then distributes the result to adds\n> and deletes.\n> \n\nWhy not just take the stupid and simple solution and make it:\n\nfile1 | +31,-19 +++\nfile2 | +19,-106 ---\nfile3 | +10,-10 ###\n\nThat is, show the number of lines that actually changed, and print a \nfixed number of plusses or minuses after the numbers to make it easy to, \nat a glance, check if more lines were added than deleted or vice versa.\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"28293","messageId":"20061006155331.GR20017@pasky.or.cz","threadId":"5713","inReplyTo":"Pine.LNX.4.63.0609270948140.14200@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH 3/3] diff --stat: sometimes use non-linear scaling.","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2006-10-06T15:53:31Z","receivedAt":"2006-10-06T15:53:31Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Wed, Sep 27, 2006 at 09:50:11AM CEST, I got a letter\nwhere Johannes Schindelin said that...\n> Git's source code is very clean and readable, even if there are inversions \n> you might not be used to.\n\nI think it's a good sign that _this_ is what we argue about. ;-)\n\n-- \n\t\t\t\tPetr \"Pasky the 10 > x Loather\" Baudis\nStuff: http://pasky.or.cz/\n#!/bin/perl -sp0777i