# [PATCH v3 0/7] diff stat calculation adjustments 15 messages from 2012-04-16 to 2012-04-18. Participants: Lucian Poston, Junio C Hamano. Thread: https://gitlist.dev/t/30258 ## Lucian Poston, 2012-04-16 10:44 Subject: [PATCH v3 0/7] diff stat calculation adjustments Message-ID: <1334573095-32286-1-git-send-email-lucian.poston@gmail.com> URL: https://gitlist.dev/e/1334573095-32286-1-git-send-email-lucian.poston%40gmail.com ``` I broke the patch series up into smaller patches. Hopefully this makes the intent of the changes easier to follow. Zbigniew's tests were added as PATCH 1/7. The order of the options->stat_graph_width test was reverted back to the original order as per Zbigniew's feedback. There remains an issue where name_width (or graph_width) could be set to a value greater than stat_name_width (or stat_graph_width). This patch doesn't address the issue -- v2 had a partial fix, but it resulted in other problems anyway. Lucian Poston (6): Add output_prefix_length to diff_options Adjust stat width calculations to take --graph output into account t4052: Adjust --graph --stat output for prefixes t4052: Add tests to illustrate issues with restrictive COLUMNS Prevent graph_width of stat width from falling below min t4052: Adjust --stat output for minimum graph_width Zbigniew Jędrzejewski-Szmek (1): t4052: test --stat output with --graph diff.c | 8 ++- diff.h | 1 + graph.c | 3 + t/t4052-stat-output.sh | 122 ++++++++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 128 insertions(+), 6 deletions(-) -- 1.7.3.4 ``` ## Lucian Poston, 2012-04-16 10:44 Subject: [PATCH v3 1/7] t4052: test --stat output with --graph Message-ID: <1334573095-32286-2-git-send-email-lucian.poston@gmail.com> URL: https://gitlist.dev/e/1334573095-32286-2-git-send-email-lucian.poston%40gmail.com In-Reply-To: <1334573095-32286-1-git-send-email-lucian.poston@gmail.com> ``` From: Zbigniew Jędrzejewski-Szmek Add tests which show that the width of the --prefix added by --graph is not taken into consideration when the diff stat output width is calculated. Signed-off-by: Zbigniew Jędrzejewski-Szmek Signed-off-by: Lucian Poston --- t/t4052-stat-output.sh | 78 +++++++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 74 insertions(+), 4 deletions(-) diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh index 328aa8f..da14984 100755 --- a/t/t4052-stat-output.sh +++ b/t/t4052-stat-output.sh @@ -82,11 +82,15 @@ test_expect_success 'preparation for big change tests' ' cat >expect80 <<'EOF' abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ EOF - +cat >expect80-graph <<'EOF' +| abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ +EOF cat >expect200 <<'EOF' abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ EOF - +cat >expect200-graph <<'EOF' +| abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ +EOF while read verb expect cmd args do test_expect_success "$cmd $verb COLUMNS (big change)" ' @@ -94,6 +98,14 @@ do grep " | " output >actual && test_cmp "$expect" actual ' + + test "$cmd" != diff || continue + + test_expect_success "$cmd --graph $verb COLUMNS (big change)" ' + COLUMNS=200 git $cmd $args --graph >output + grep " | " output >actual && + test_cmp "$expect-graph" actual + ' done <<\EOF ignores expect80 format-patch -1 --stdout respects expect200 diff HEAD^ HEAD --stat @@ -104,7 +116,9 @@ EOF cat >expect40 <<'EOF' abcd | 1000 ++++++++++++++++++++++++++ EOF - +cat >expect40-graph <<'EOF' +| abcd | 1000 ++++++++++++++++++++++++++ +EOF while read verb expect cmd args do test_expect_success "$cmd $verb not enough COLUMNS (big change)" ' @@ -118,6 +132,20 @@ do grep " | " output >actual && test_cmp "$expect" actual ' + + test "$cmd" != diff || continue + + test_expect_success "$cmd --graph $verb not enough COLUMNS (big change)" ' + COLUMNS=40 git $cmd $args --graph >output + grep " | " output >actual && + test_cmp "$expect-graph" actual + ' + + test_expect_success "$cmd --graph $verb statGraphWidth config" ' + git -c diff.statGraphWidth=26 $cmd $args --graph >output + grep " | " output >actual && + test_cmp "$expect-graph" actual + ' done <<\EOF ignores expect80 format-patch -1 --stdout respects expect40 diff HEAD^ HEAD --stat @@ -129,6 +157,9 @@ EOF cat >expect <<'EOF' abcd | 1000 ++++++++++++++++++++++++++ EOF +cat >expect-graph <<'EOF' +| abcd | 1000 ++++++++++++++++++++++++++ +EOF while read cmd args do test_expect_success "$cmd --stat=width with big change" ' @@ -143,11 +174,25 @@ do test_cmp expect actual ' - test_expect_success "$cmd --stat-graph--width with big change" ' + test_expect_success "$cmd --stat-graph-width with big change" ' git $cmd $args --stat-graph-width=26 >output grep " | " output >actual && test_cmp expect actual ' + + test "$cmd" != diff || continue + + test_expect_success "$cmd --stat-width=width --graph with big change" ' + git $cmd $args --stat-width=40 --graph >output + grep " | " output >actual && + test_cmp expect-graph actual + ' + + test_expect_success "$cmd --stat-graph-width --graph with big change" ' + git $cmd $args --stat-graph-width=26 --graph >output + grep " | " output >actual && + test_cmp expect-graph actual + ' done <<\EOF format-patch -1 --stdout diff HEAD^ HEAD --stat @@ -164,6 +209,9 @@ test_expect_success 'preparation for long filename tests' ' cat >expect <<'EOF' ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 ++++++++++++ EOF +cat >expect-graph <<'EOF' +| ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 ++++++++++++ +EOF while read cmd args do test_expect_success "$cmd --stat=width with big change is more balanced" ' @@ -171,6 +219,14 @@ do grep " | " output >actual && test_cmp expect actual ' + + test "$cmd" != diff || continue + + test_expect_success "$cmd --stat=width --graph with big change is balanced" ' + git $cmd $args --stat-width=60 --graph >output && + grep " | " output >actual && + test_cmp expect-graph actual + ' done <<\EOF format-patch -1 --stdout diff HEAD^ HEAD --stat @@ -181,9 +237,15 @@ EOF cat >expect80 <<'EOF' ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 ++++++++++++++++++++ EOF +cat >expect80-graph <<'EOF' +| ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 ++++++++++++++++++++ +EOF cat >expect200 <<'EOF' aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ EOF +cat >expect200-graph <<'EOF' +| aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ +EOF while read verb expect cmd args do test_expect_success "$cmd $verb COLUMNS (long filename)" ' @@ -191,6 +253,14 @@ do grep " | " output >actual && test_cmp "$expect" actual ' + + test "$cmd" != diff || continue + + test_expect_success "$cmd --graph $verb COLUMNS (long filename)" ' + COLUMNS=200 git $cmd $args --graph >output + grep " | " output >actual && + test_cmp "$expect-graph" actual + ' done <<\EOF ignores expect80 format-patch -1 --stdout respects expect200 diff HEAD^ HEAD --stat -- 1.7.3.4 ``` ## Lucian Poston, 2012-04-16 10:44 Subject: [PATCH v3 2/7] Add output_prefix_length to diff_options Message-ID: <1334573095-32286-3-git-send-email-lucian.poston@gmail.com> URL: https://gitlist.dev/e/1334573095-32286-3-git-send-email-lucian.poston%40gmail.com In-Reply-To: <1334573095-32286-1-git-send-email-lucian.poston@gmail.com> ``` Add output_prefix_length to diff_options. Initialize the value to 0 and only set it when graph.c:diff_output_prefix_callback() is called. Signed-off-by: Lucian Poston --- diff.h | 1 + graph.c | 3 +++ 2 files changed, 4 insertions(+), 0 deletions(-) diff --git a/diff.h b/diff.h index cb68743..19d762f 100644 --- a/diff.h +++ b/diff.h @@ -150,6 +150,7 @@ struct diff_options { diff_format_fn_t format_callback; void *format_callback_data; diff_prefix_fn_t output_prefix; + int output_prefix_length; void *output_prefix_data; }; diff --git a/graph.c b/graph.c index 7358416..7e0a099 100644 --- a/graph.c +++ b/graph.c @@ -194,8 +194,10 @@ static struct strbuf *diff_output_prefix_callback(struct diff_options *opt, void struct git_graph *graph = data; static struct strbuf msgbuf = STRBUF_INIT; + assert(opt); assert(graph); + opt->output_prefix_length = graph->width; strbuf_reset(&msgbuf); graph_padding_line(graph, &msgbuf); return &msgbuf; @@ -245,6 +247,7 @@ struct git_graph *graph_init(struct rev_info *opt) */ opt->diffopt.output_prefix = diff_output_prefix_callback; opt->diffopt.output_prefix_data = graph; + opt->diffopt.output_prefix_length = 0; return graph; } -- 1.7.3.4 ``` ## Lucian Poston, 2012-04-16 10:44 Subject: [PATCH v3 3/7] Adjust stat width calculations to take --graph output into account Message-ID: <1334573095-32286-4-git-send-email-lucian.poston@gmail.com> URL: https://gitlist.dev/e/1334573095-32286-4-git-send-email-lucian.poston%40gmail.com In-Reply-To: <1334573095-32286-1-git-send-email-lucian.poston@gmail.com> ``` The recent change to compute the width of diff --stat did not take into consideration the output from --graph. The consequence is that when both options are used, e.g. in 'log --stat --graph', the lines are too long. Adjust stat width calculations to take --graph output into account. Signed-off-by: Lucian Poston --- diff.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/diff.c b/diff.c index 377ec1e..0b61cf7 100644 --- a/diff.c +++ b/diff.c @@ -1445,7 +1445,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options) */ if (options->stat_width == -1) - width = term_columns(); + width = term_columns() - options->output_prefix_length; else width = options->stat_width ? options->stat_width : 80; -- 1.7.3.4 ``` ## Lucian Poston, 2012-04-16 10:44 Subject: [PATCH v3 4/7] t4052: Adjust --graph --stat output for prefixes Message-ID: <1334573095-32286-5-git-send-email-lucian.poston@gmail.com> URL: https://gitlist.dev/e/1334573095-32286-5-git-send-email-lucian.poston%40gmail.com In-Reply-To: <1334573095-32286-1-git-send-email-lucian.poston@gmail.com> ``` Adjust tests to verify that the commit history graph tree is taken into consideration when the diff stat output width is calculated. Signed-off-by: Lucian Poston --- t/t4052-stat-output.sh | 34 +++++++++++++++++++++++++--------- 1 files changed, 25 insertions(+), 9 deletions(-) diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh index da14984..d748e5e 100755 --- a/t/t4052-stat-output.sh +++ b/t/t4052-stat-output.sh @@ -89,7 +89,7 @@ cat >expect200 <<'EOF' abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ EOF cat >expect200-graph <<'EOF' -| abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ +| abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ EOF while read verb expect cmd args do @@ -117,7 +117,7 @@ cat >expect40 <<'EOF' abcd | 1000 ++++++++++++++++++++++++++ EOF cat >expect40-graph <<'EOF' -| abcd | 1000 ++++++++++++++++++++++++++ +| abcd | 1000 ++++++++++++++++++++++++ EOF while read verb expect cmd args do @@ -127,12 +127,6 @@ do test_cmp "$expect" actual ' - test_expect_success "$cmd $verb statGraphWidth config" ' - git -c diff.statGraphWidth=26 $cmd $args >output - grep " | " output >actual && - test_cmp "$expect" actual - ' - test "$cmd" != diff || continue test_expect_success "$cmd --graph $verb not enough COLUMNS (big change)" ' @@ -140,6 +134,28 @@ do grep " | " output >actual && test_cmp "$expect-graph" actual ' +done <<\EOF +ignores expect80 format-patch -1 --stdout +respects expect40 diff HEAD^ HEAD --stat +respects expect40 show --stat +respects expect40 log -1 --stat +EOF + +cat >expect40 <<'EOF' + abcd | 1000 ++++++++++++++++++++++++++ +EOF +cat >expect40-graph <<'EOF' +| abcd | 1000 ++++++++++++++++++++++++++ +EOF +while read verb expect cmd args +do + test_expect_success "$cmd $verb statGraphWidth config" ' + git -c diff.statGraphWidth=26 $cmd $args >output + grep " | " output >actual && + test_cmp "$expect" actual + ' + + test "$cmd" != diff || continue test_expect_success "$cmd --graph $verb statGraphWidth config" ' git -c diff.statGraphWidth=26 $cmd $args --graph >output @@ -244,7 +260,7 @@ cat >expect200 <<'EOF' aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ EOF cat >expect200-graph <<'EOF' -| aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ +| aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ EOF while read verb expect cmd args do -- 1.7.3.4 ``` ## Lucian Poston, 2012-04-16 10:44 Subject: [PATCH v3 5/7] t4052: Add tests to illustrate issues with restrictive COLUMNS Message-ID: <1334573095-32286-6-git-send-email-lucian.poston@gmail.com> URL: https://gitlist.dev/e/1334573095-32286-6-git-send-email-lucian.poston%40gmail.com In-Reply-To: <1334573095-32286-1-git-send-email-lucian.poston@gmail.com> ``` Signed-off-by: Lucian Poston --- t/t4052-stat-output.sh | 28 ++++++++++++++++++++++++++++ 1 files changed, 28 insertions(+), 0 deletions(-) diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh index d748e5e..6cac95b 100755 --- a/t/t4052-stat-output.sh +++ b/t/t4052-stat-output.sh @@ -284,6 +284,34 @@ respects expect200 show --stat respects expect200 log -1 --stat EOF +cat >expect1 <<'EOF' + ...aaaaaaaaaaaaaa | 1000 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ +EOF +cat >expect1-graph <<'EOF' +| ...aaaaaaaaaaaaaa | 1000 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ +EOF +while read verb expect cmd args +do + test_expect_success "$cmd $verb prefix greater than COLUMNS (big change)" ' + COLUMNS=1 git $cmd $args >output + grep " | " output >actual && + test_cmp "$expect" actual + ' + + test "$cmd" != diff || continue + + test_expect_success "$cmd --graph $verb prefix greater than COLUMNS (big change)" ' + COLUMNS=1 git $cmd $args --graph >output + grep " | " output >actual && + test_cmp "$expect-graph" actual + ' +done <<\EOF +ignores expect80 format-patch -1 --stdout +respects expect1 diff HEAD^ HEAD --stat +respects expect1 show --stat +respects expect1 log -1 --stat +EOF + cat >expect <<'EOF' abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ EOF -- 1.7.3.4 ``` ## Lucian Poston, 2012-04-16 10:44 Subject: [PATCH v3 6/7] Prevent graph_width of diff stat from falling below min Message-ID: <1334573095-32286-7-git-send-email-lucian.poston@gmail.com> URL: https://gitlist.dev/e/1334573095-32286-7-git-send-email-lucian.poston%40gmail.com In-Reply-To: <1334573095-32286-1-git-send-email-lucian.poston@gmail.com> ``` Signed-off-by: Lucian Poston --- diff.c | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/diff.c b/diff.c index 0b61cf7..f08b41a 100644 --- a/diff.c +++ b/diff.c @@ -1473,8 +1473,12 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options) * Adjust adjustable widths not to exceed maximum width */ if (name_width + number_width + 6 + graph_width > width) { - if (graph_width > width * 3/8 - number_width - 6) + if (graph_width > width * 3/8 - number_width - 6) { graph_width = width * 3/8 - number_width - 6; + if (graph_width < 6) + graph_width = 6; + } + if (options->stat_graph_width && graph_width > options->stat_graph_width) graph_width = options->stat_graph_width; -- 1.7.3.4 ``` ## Lucian Poston, 2012-04-16 10:44 Subject: [PATCH v3 7/7] t4052: Adjust --stat output for minimum graph_width Message-ID: <1334573095-32286-8-git-send-email-lucian.poston@gmail.com> URL: https://gitlist.dev/e/1334573095-32286-8-git-send-email-lucian.poston%40gmail.com In-Reply-To: <1334573095-32286-1-git-send-email-lucian.poston@gmail.com> ``` Signed-off-by: Lucian Poston --- t/t4052-stat-output.sh | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh index 6cac95b..ced32db 100755 --- a/t/t4052-stat-output.sh +++ b/t/t4052-stat-output.sh @@ -285,10 +285,10 @@ respects expect200 log -1 --stat EOF cat >expect1 <<'EOF' - ...aaaaaaaaaaaaaa | 1000 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ + ...aaaaaaa | 1000 ++++++ EOF cat >expect1-graph <<'EOF' -| ...aaaaaaaaaaaaaa | 1000 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ +| ...aaaaaaa | 1000 ++++++ EOF while read verb expect cmd args do -- 1.7.3.4 ``` ## Junio C Hamano, 2012-04-16 18:48 Subject: Re: [PATCH v3 5/7] t4052: Add tests to illustrate issues with restrictive COLUMNS Message-ID: <7v7gxfv7h5.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7v7gxfv7h5.fsf%40alter.siamese.dyndns.org In-Reply-To: <1334573095-32286-6-git-send-email-lucian.poston@gmail.com> ``` Lucian Poston writes: > Signed-off-by: Lucian Poston > --- Sorry, the patch text itself does not tell me what it wants to "illustate" at all. I _guess_ that you are saying that the output for expect1 and expect1-graph should be limited to very narrow width, not like the humongous output like we see in the added test vector, and it will be fixed with your patch 6/7. If that is the case, please structure this part of the series like this: 5/7: add test vector to illustrate what the correct output should be, but mark the test with "test_expect_failure". 6/7: update the code to fix it, and flip the "test_expect_failure" in 5/7 to "test_expect_success". And 7/7 will become unneeded. Thanks. > t/t4052-stat-output.sh | 28 ++++++++++++++++++++++++++++ > 1 files changed, 28 insertions(+), 0 deletions(-) > > diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh > index d748e5e..6cac95b 100755 > --- a/t/t4052-stat-output.sh > +++ b/t/t4052-stat-output.sh > @@ -284,6 +284,34 @@ respects expect200 show --stat > respects expect200 log -1 --stat > EOF > > +cat >expect1 <<'EOF' > + ...aaaaaaaaaaaaaa | 1000 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ ++ > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > +EOF > +cat >expect1-graph <<'EOF' > +| ...aaaaaaaaaaaaaa | 1000 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ ++ > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > +EOF > +while read verb expect cmd args > +do > + test_expect_success "$cmd $verb prefix greater than COLUMNS (big change)" ' > + COLUMNS=1 git $cmd $args >output > + grep " | " output >actual && > + test_cmp "$expect" actual > + ' > + > + test "$cmd" != diff || continue > + > + test_expect_success "$cmd --graph $verb prefix greater than COLUMNS (big change)" ' > + COLUMNS=1 git $cmd $args --graph >output > + grep " | " output >actual && > + test_cmp "$expect-graph" actual > + ' > +done <<\EOF > +ignores expect80 format-patch -1 --stdout > +respects expect1 diff HEAD^ HEAD --stat > +respects expect1 show --stat > +respects expect1 log -1 --stat > +EOF > + > cat >expect <<'EOF' > abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > EOF ``` ## Junio C Hamano, 2012-04-16 18:48 Subject: Re: [PATCH v3 4/7] t4052: Adjust --graph --stat output for prefixes Message-ID: <7vzkabtsw9.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vzkabtsw9.fsf%40alter.siamese.dyndns.org In-Reply-To: <1334573095-32286-5-git-send-email-lucian.poston@gmail.com> ``` Lucian Poston writes: > Adjust tests to verify that the commit history graph tree is taken into > consideration when the diff stat output width is calculated. > > Signed-off-by: Lucian Poston > --- > t/t4052-stat-output.sh | 34 +++++++++++++++++++++++++--------- > 1 files changed, 25 insertions(+), 9 deletions(-) > > diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh > index da14984..d748e5e 100755 > --- a/t/t4052-stat-output.sh > +++ b/t/t4052-stat-output.sh > @@ -89,7 +89,7 @@ cat >expect200 <<'EOF' > abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > EOF > cat >expect200-graph <<'EOF' > -| abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > +| abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > EOF This change is easily explainable. Earlier, the code did not take the "| " part at the very left end (i.e. ancestry graph) and drew the --stat output too wide by two columns, but now the code is updated to compensate for that, because "200" is coming from COLUMNS to clip the whole width, not just the stat-width part. > while read verb expect cmd args > do > @@ -117,7 +117,7 @@ cat >expect40 <<'EOF' > abcd | 1000 ++++++++++++++++++++++++++ > EOF > cat >expect40-graph <<'EOF' > -| abcd | 1000 ++++++++++++++++++++++++++ > +| abcd | 1000 ++++++++++++++++++++++++ > EOF Likewise. > while read verb expect cmd args > do > @@ -127,12 +127,6 @@ do > test_cmp "$expect" actual > ' > > - test_expect_success "$cmd $verb statGraphWidth config" ' > - git -c diff.statGraphWidth=26 $cmd $args >output > - grep " | " output >actual && > - test_cmp "$expect" actual > - ' > - This is unclear. Why does this need to be removed? ``` ## Junio C Hamano, 2012-04-16 18:48 Subject: Re: [PATCH v3 2/7] Add output_prefix_length to diff_options Message-ID: <7vsjg3tsvv.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vsjg3tsvv.fsf%40alter.siamese.dyndns.org In-Reply-To: <1334573095-32286-3-git-send-email-lucian.poston@gmail.com> ``` Lucian Poston writes: > Add output_prefix_length to diff_options. Initialize the value to 0 and only > set it when graph.c:diff_output_prefix_callback() is called. > > Signed-off-by: Lucian Poston > --- > diff.h | 1 + > graph.c | 3 +++ > 2 files changed, 4 insertions(+), 0 deletions(-) This patch is a no-op, as nobody pays attention to the value this field contains. I think it is better to squash this with 3/7 where you introduce the only user of this field. Also when that happens, you would need to adjust the test vector, which may mean that you need to squash the result with 4/7 as well. > diff --git a/diff.h b/diff.h > index cb68743..19d762f 100644 > --- a/diff.h > +++ b/diff.h > @@ -150,6 +150,7 @@ struct diff_options { > diff_format_fn_t format_callback; > void *format_callback_data; > diff_prefix_fn_t output_prefix; > + int output_prefix_length; > void *output_prefix_data; > }; > > diff --git a/graph.c b/graph.c > index 7358416..7e0a099 100644 > --- a/graph.c > +++ b/graph.c > @@ -194,8 +194,10 @@ static struct strbuf *diff_output_prefix_callback(struct diff_options *opt, void > struct git_graph *graph = data; > static struct strbuf msgbuf = STRBUF_INIT; > > + assert(opt); > assert(graph); > > + opt->output_prefix_length = graph->width; > strbuf_reset(&msgbuf); > graph_padding_line(graph, &msgbuf); > return &msgbuf; > @@ -245,6 +247,7 @@ struct git_graph *graph_init(struct rev_info *opt) > */ > opt->diffopt.output_prefix = diff_output_prefix_callback; > opt->diffopt.output_prefix_data = graph; > + opt->diffopt.output_prefix_length = 0; > > return graph; > } ``` ## Junio C Hamano, 2012-04-16 19:30 Subject: Re: [PATCH v3 0/7] diff stat calculation adjustments Message-ID: <7vobqrtqxj.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vobqrtqxj.fsf%40alter.siamese.dyndns.org In-Reply-To: <1334573095-32286-1-git-send-email-lucian.poston@gmail.com> ``` Lucian Poston writes: > I broke the patch series up into smaller patches. Hopefully this makes the > intent of the changes easier to follow. > > Zbigniew's tests were added as PATCH 1/7. Thanks. Overall it looks nicely done, except that the structure of the series may want a few squashing, moving and use of test_expect_failure that later turns into test_expect_success. When I learned from you that you will be rerolling, I was expecting to see the other series, i.e. * lp/maint-diff-three-dash-with-graph (2012-03-20) 3 commits - t4202: add test for "log --graph --stat -p" separator lines - log --graph: fix break in graph lines - log --graph --stat: three-dash separator should come after graph lines but I think this was $gmane/193490 split into three and didn't have anything controversial. I'll merge it to 'next'. Thanks. ``` ## Lucian Poston, 2012-04-18 02:23 Subject: Re: [PATCH v3 4/7] t4052: Adjust --graph --stat output for prefixes Message-ID: URL: https://gitlist.dev/e/CACz_eyfvC9OWwBscCTHVvs%2BudRW5_QUReZnxpPVWSrL1MAC6rA%40mail.gmail.com In-Reply-To: <7vzkabtsw9.fsf@alter.siamese.dyndns.org> ``` On Mon, Apr 16, 2012 at 11:48, Junio C Hamano wrote: >> @@ -117,7 +117,7 @@ cat >expect40 <<'EOF' >>   abcd | 1000 ++++++++++++++++++++++++++ >>  EOF >>  cat >expect40-graph <<'EOF' >> -|  abcd | 1000 ++++++++++++++++++++++++++ >> +|  abcd | 1000 ++++++++++++++++++++++++ >>  EOF >>  while read verb expect cmd args >>  do >> @@ -127,12 +127,6 @@ do >>               test_cmp "$expect" actual >>       ' >> >> -     test_expect_success "$cmd $verb statGraphWidth config" ' >> -             git -c diff.statGraphWidth=26 $cmd $args >output >> -             grep " | " output >actual && >> -             test_cmp "$expect" actual >> -     ' >> - > > This is unclear.  Why does this need to be removed? This test was actually only moved. There were 4 test_expect_success in a while loop -- 2 tested COLUMNS=40, and 2 tested diff.statGraphWidth=26. I moved the two tests for diff.statGraphWidth to a new while loop. expect40-graph varies between these two sets of tests, so I thought it cleaner to separate them so that the expectation could be defined separately. ``` ## Lucian Poston, 2012-04-18 02:31 Subject: Re: [PATCH v3 2/7] Add output_prefix_length to diff_options Message-ID: URL: https://gitlist.dev/e/CACz_eyftA6GQhSMscoucEbzKagjTSU3Psd8z%2Bd9_oWWrKD--%2Bw%40mail.gmail.com In-Reply-To: <7vsjg3tsvv.fsf@alter.siamese.dyndns.org> ``` On Mon, Apr 16, 2012 at 11:48, Junio C Hamano wrote: > Lucian Poston writes: > >> Add output_prefix_length to diff_options. Initialize the value to 0 and only >> set it when graph.c:diff_output_prefix_callback() is called. >> >> Signed-off-by: Lucian Poston >> --- >>  diff.h  |    1 + >>  graph.c |    3 +++ >>  2 files changed, 4 insertions(+), 0 deletions(-) > > This patch is a no-op, as nobody pays attention to the value this field > contains.  I think it is better to squash this with 3/7 where you > introduce the only user of this field. > > Also when that happens, you would need to adjust the test vector, which > may mean that you need to squash the result with 4/7 as well. Squashed. ``` ## Lucian Poston, 2012-04-18 02:31 Subject: Re: [PATCH v3 5/7] t4052: Add tests to illustrate issues with restrictive COLUMNS Message-ID: URL: https://gitlist.dev/e/CACz_eydRcYNs1TOpRjM91tzmBrXjs4sKzBrFFvXGk0hEQ9zEMw%40mail.gmail.com In-Reply-To: <7v7gxfv7h5.fsf@alter.siamese.dyndns.org> ``` On Mon, Apr 16, 2012 at 11:48, Junio C Hamano wrote: > Lucian Poston writes: > >> Signed-off-by: Lucian Poston >> --- > > Sorry, the patch text itself does not tell me what it wants to "illustate" > at all. > > I _guess_ that you are saying that the output for expect1 and > expect1-graph should be limited to very narrow width, not like the > humongous output like we see in the added test vector, and it will be > fixed with your patch 6/7. > > If that is the case, please structure this part of the series like this: > >        5/7: add test vector to illustrate what the correct output should >             be, but mark the test with "test_expect_failure". > >        6/7: update the code to fix it, and flip the "test_expect_failure" >             in 5/7 to "test_expect_success". > > And 7/7 will become unneeded. Done. Thanks. ```