threads / patch / 30258

v3, 7 partsdiff stat calculation adjustments

Subject: [PATCH v3 0/7] diff stat calculation adjustments

## tl;dr

15 messages between Apr 16, 2012 and Apr 18, 2012. Diffs are folded; open one to read it.

replies: 14people: 2as markdown or json

Lucian Poston· Apr 16, 2012, 10:44 UTC · lore

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· Apr 16, 2012, 10:44 UTC · re: Lucian Poston · lore

[PATCH v3 1/7] t4052: test --stat output with --graph

From: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>

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 <zbyszek@in.waw.pl>
Signed-off-by: Lucian Poston <lucian.poston@gmail.com>
---
 t/t4052-stat-output.sh |   78 +++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 74 insertions(+), 4 deletions(-)
Show changes to t/t4052-stat-output.sh +74 −4
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· Apr 16, 2012, 10:44 UTC · re: Lucian Poston · lore

[PATCH v3 2/7] Add output_prefix_length to diff_options

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 <lucian.poston@gmail.com>
---
 diff.h  |    1 +
 graph.c |    3 +++
 2 files changed, 4 insertions(+), 0 deletions(-)
Show changes to 2 files +4 −0

diff.h, graph.c

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
Junio C Hamano· Apr 16, 2012, 18:48 UTC · re: Lucian Poston · lore

Re: [PATCH v3 2/7] Add output_prefix_length to diff_options

Lucian Poston <lucian.poston@gmail.com> writes:
Show 8 quoted lines
> 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 <lucian.poston@gmail.com>
> ---
>  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.

Show 35 quoted lines
> 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;
>  }
Lucian Poston· Apr 18, 2012, 02:31 UTC · re: Junio C Hamano · lore

Re: [PATCH v3 2/7] Add output_prefix_length to diff_options

On Mon, Apr 16, 2012 at 11:48, Junio C Hamano <gitster@pobox.com> wrote:
Show 17 quoted lines
> Lucian Poston <lucian.poston@gmail.com> 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 <lucian.poston@gmail.com>
>> ---
>>  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· Apr 16, 2012, 10:44 UTC · re: Lucian Poston · lore

[PATCH v3 3/7] Adjust stat width calculations to take --graph output into account

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 <lucian.poston@gmail.com>
---
 diff.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Show changes to diff.c +1 −1
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· Apr 16, 2012, 10:44 UTC · re: Lucian Poston · lore

[PATCH v3 4/7] t4052: Adjust --graph --stat output for prefixes

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 <lucian.poston@gmail.com>
---
 t/t4052-stat-output.sh |   34 +++++++++++++++++++++++++---------
 1 files changed, 25 insertions(+), 9 deletions(-)
Show changes to t/t4052-stat-output.sh +25 −9
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
Junio C Hamano· Apr 16, 2012, 18:48 UTC · re: Lucian Poston · lore

Re: [PATCH v3 4/7] t4052: Adjust --graph --stat output for prefixes

Lucian Poston <lucian.poston@gmail.com> writes:
Show 19 quoted lines
> 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 <lucian.poston@gmail.com>
> ---
>  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.

Show 9 quoted lines
>  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.
Show 12 quoted lines
>  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?
Lucian Poston· Apr 18, 2012, 02:23 UTC · re: Junio C Hamano · lore

Re: [PATCH v3 4/7] t4052: Adjust --graph --stat output for prefixes

On Mon, Apr 16, 2012 at 11:48, Junio C Hamano <gitster@pobox.com> wrote:
Show 21 quoted lines
>> @@ -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· Apr 16, 2012, 10:44 UTC · re: Lucian Poston · lore

[PATCH v3 5/7] t4052: Add tests to illustrate issues with restrictive COLUMNS

Signed-off-by: Lucian Poston <lucian.poston@gmail.com>
---
 t/t4052-stat-output.sh |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)
Show changes to t/t4052-stat-output.sh +28 −0
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
Junio C Hamano· Apr 16, 2012, 18:48 UTC · re: Lucian Poston · lore

Re: [PATCH v3 5/7] t4052: Add tests to illustrate issues with restrictive COLUMNS

Lucian Poston <lucian.poston@gmail.com> writes:
> Signed-off-by: Lucian Poston <lucian.poston@gmail.com>
> ---

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.
Show 13 quoted lines
>  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 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 ++
Show 27 quoted lines
>  +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +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
Lucian Poston· Apr 18, 2012, 02:31 UTC · re: Junio C Hamano · lore

Re: [PATCH v3 5/7] t4052: Add tests to illustrate issues with restrictive COLUMNS

On Mon, Apr 16, 2012 at 11:48, Junio C Hamano <gitster@pobox.com> wrote:
Show 22 quoted lines
> Lucian Poston <lucian.poston@gmail.com> writes:
>
>> Signed-off-by: Lucian Poston <lucian.poston@gmail.com>
>> ---
>
> 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.
Lucian Poston· Apr 16, 2012, 10:44 UTC · re: Lucian Poston · lore

[PATCH v3 6/7] Prevent graph_width of diff stat from falling below min

Signed-off-by: Lucian Poston <lucian.poston@gmail.com>
---
 diff.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)
Show changes to diff.c +5 −1
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· Apr 16, 2012, 10:44 UTC · re: Lucian Poston · lore

[PATCH v3 7/7] t4052: Adjust --stat output for minimum graph_width

Signed-off-by: Lucian Poston <lucian.poston@gmail.com>
---
 t/t4052-stat-output.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
Show changes to t/t4052-stat-output.sh +2 −2
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· Apr 16, 2012, 19:30 UTC · re: Lucian Poston · lore

Re: [PATCH v3 0/7] diff stat calculation adjustments

Lucian Poston <lucian.poston@gmail.com> 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.

← back to recent threads