[PATCH v4 1/4] t4052: test --stat output with --graph

[PATCH v4 1/4] t4052: test --stat output with --graph

[Lucian Poston]

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(-)

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

[PATCH v4 2/4] Adjust stat width calculations to take --graph output into account

[Lucian Poston]

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.

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.

Using output_prefix_length, adjust stat width calculations to take --graph
output into account.

t4052: Modify tests to verify that diff stat output is adjusted to compenstate
for the history graph tree.

Signed-off-by: Lucian Poston <lucian.poston@gmail.com>
---
 diff.c                 |    2 +-
 diff.h                 |    1 +
 graph.c                |    3 +++
 t/t4052-stat-output.sh |   34 +++++++++++++++++++++++++---------
 4 files changed, 30 insertions(+), 10 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;
 
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;
 }
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

[PATCH v4 3/4] t4052: Test diff-stat output with minimum columns

[Lucian Poston]

When COLUMNS or --stat-width restricts the diff-stat width to near the
minimum, 26 columns, the graph_width value becomes negative. Consequently, the
graph part of diff-stat is not resized properly.

Signed-off-by: Lucian Poston <lucian.poston@gmail.com>
---
 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..6305c99 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'
+ ...aaaaaaa | 1000 ++++++
+EOF
+cat >expect1-graph <<'EOF'
+|  ...aaaaaaa | 1000 ++++++
+EOF
+while read verb expect cmd args
+do
+	test_expect_failure "$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_failure "$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

[PATCH v4 4/4] Prevent graph_width of stat width from falling below min

[Lucian Poston]

Update tests in t4052 fixed by this change.

Signed-off-by: Lucian Poston <lucian.poston@gmail.com>
---
 diff.c                 |    6 +++++-
 t/t4052-stat-output.sh |    4 ++--
 2 files changed, 7 insertions(+), 3 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;
diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh
index 6305c99..ced32db 100755
--- a/t/t4052-stat-output.sh
+++ b/t/t4052-stat-output.sh
@@ -292,7 +292,7 @@ cat >expect1-graph <<'EOF'
 EOF
 while read verb expect cmd args
 do
-	test_expect_failure "$cmd $verb prefix greater than COLUMNS (big change)" '
+	test_expect_success "$cmd $verb prefix greater than COLUMNS (big change)" '
 		COLUMNS=1 git $cmd $args >output
 		grep " | " output >actual &&
 		test_cmp "$expect" actual
@@ -300,7 +300,7 @@ do
 
 	test "$cmd" != diff || continue
 
-	test_expect_failure "$cmd --graph $verb prefix greater than COLUMNS (big change)" '
+	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
-- 
1.7.3.4

Re: [PATCH v4 1/4] t4052: test --stat output with --graph

[Zbigniew Jędrzejewski-Szmek]

On 04/18/2012 04:29 AM, Lucian Poston wrote:
> 1/4 Prevent graph_width of stat width from falling below min
> 2/4 t4052: Test diff-stat output with minimum columns
> 3/4 Adjust stat width calculations to take --graph output into account
> 4/4 t4052: test --stat output with --graph

Looks nice.

Zbyszek

Re: [PATCH v4 3/4] t4052: Test diff-stat output with minimum columns

[Junio C Hamano]

Lucian Poston <lucian.poston@gmail.com> writes:

> When COLUMNS or --stat-width restricts the diff-stat width to near the
> minimum, 26 columns, the graph_width value becomes negative. Consequently, the
> graph part of diff-stat is not resized properly.
>
> Signed-off-by: Lucian Poston <lucian.poston@gmail.com>
> ---
>  t/t4052-stat-output.sh |   28 ++++++++++++++++++++++++++++
>  1 files changed, 28 insertions(+), 0 deletions(-)

Running t4052 with this patch applied reports

# fixed 2 known breakage(s)
# still have 5 known breakage(s)
# passed all remaining 81 test(s)

Please mark the ones that already pass as "test_expect_success", leaving
the only broken ones as "test_expect_failure".  Otherwise the patch to the
test that contains the fix would not illustrate what it fixed.

Thanks.

Re: [PATCH v4 3/4] t4052: Test diff-stat output with minimum columns

[Lucian Poston]

When COLUMNS or --stat-width restricts the diff-stat width to near the
minimum, 26 columns, the graph_width value becomes negative. Consequently, the
graph part of diff-stat is not resized properly.

Signed-off-by: Lucian Poston <lucian.poston@gmail.com>
---
 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..b8eec8f 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'
+ ...aaaaaaa | 1000 ++++++
+EOF
+cat >expect1-graph <<'EOF'
+|  ...aaaaaaa | 1000 ++++++
+EOF
+while read teststate verb expect cmd args
+do
+	test_expect_$teststate "$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_$teststate "$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
+success ignores expect80 format-patch -1 --stdout
+failure respects expect1 diff HEAD^ HEAD --stat
+failure respects expect1 show --stat
+failure respects expect1 log -1 --stat
+EOF
+
 cat >expect <<'EOF'
  abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 EOF
-- 
1.7.3.4

Re: [PATCH v4 4/4] Prevent graph_width of stat width from falling below min

[Lucian Poston]

Update tests in t4052 fixed by this change.

Signed-off-by: Lucian Poston <lucian.poston@gmail.com>
---
 diff.c                 |    6 +++++-
 t/t4052-stat-output.sh |   14 +++++++-------
 2 files changed, 12 insertions(+), 8 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;
diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh
index b8eec8f..ced32db 100755
--- a/t/t4052-stat-output.sh
+++ b/t/t4052-stat-output.sh
@@ -290,9 +290,9 @@ EOF
 cat >expect1-graph <<'EOF'
 |  ...aaaaaaa | 1000 ++++++
 EOF
-while read teststate verb expect cmd args
+while read verb expect cmd args
 do
-	test_expect_$teststate "$cmd $verb prefix greater than COLUMNS (big change)" '
+	test_expect_success "$cmd $verb prefix greater than COLUMNS (big change)" '
 		COLUMNS=1 git $cmd $args >output
 		grep " | " output >actual &&
 		test_cmp "$expect" actual
@@ -300,16 +300,16 @@ do
 
 	test "$cmd" != diff || continue
 
-	test_expect_$teststate "$cmd --graph $verb prefix greater than COLUMNS (big change)" '
+	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
-success ignores expect80 format-patch -1 --stdout
-failure respects expect1 diff HEAD^ HEAD --stat
-failure respects expect1 show --stat
-failure respects expect1 log -1 --stat
+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'
-- 
1.7.3.4

Re: [PATCH v4 3/4] t4052: Test diff-stat output with minimum columns

[Lucian Poston]

On Wed, Apr 18, 2012 at 13:15, Junio C Hamano <gitster@pobox.com> wrote:
> Lucian Poston <lucian.poston@gmail.com> writes:
>
>> When COLUMNS or --stat-width restricts the diff-stat width to near the
>> minimum, 26 columns, the graph_width value becomes negative. Consequently, the
>> graph part of diff-stat is not resized properly.
>>
>> Signed-off-by: Lucian Poston <lucian.poston@gmail.com>
>> ---
>>  t/t4052-stat-output.sh |   28 ++++++++++++++++++++++++++++
>>  1 files changed, 28 insertions(+), 0 deletions(-)
>
> Running t4052 with this patch applied reports
>
> # fixed 2 known breakage(s)
> # still have 5 known breakage(s)
> # passed all remaining 81 test(s)
>
> Please mark the ones that already pass as "test_expect_success", leaving
> the only broken ones as "test_expect_failure".  Otherwise the patch to the
> test that contains the fix would not illustrate what it fixed.

Sorry about that. Fixed in the recently emailed diffs.

Thanks!

Re: [PATCH v4 3/4] t4052: Test diff-stat output with minimum columns

[Kacper Kornet]

On Wed, Apr 18, 2012 at 02:09:33PM -0700, Lucian Poston wrote:
> When COLUMNS or --stat-width restricts the diff-stat width to near the
> minimum, 26 columns, the graph_width value becomes negative. Consequently, the
> graph part of diff-stat is not resized properly.

> Signed-off-by: Lucian Poston <lucian.poston@gmail.com>
> ---
>  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..b8eec8f 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'
> + ...aaaaaaa | 1000 ++++++
> +EOF
> +cat >expect1-graph <<'EOF'
> +|  ...aaaaaaa | 1000 ++++++
> +EOF
> +while read teststate verb expect cmd args
> +do
> +	test_expect_$teststate "$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_$teststate "$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
> +success ignores expect80 format-patch -1 --stdout
> +failure respects expect1 diff HEAD^ HEAD --stat
> +failure respects expect1 show --stat
> +failure respects expect1 log -1 --stat
> +EOF
> +
>  cat >expect <<'EOF'
>   abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  EOF

These tests fail when /bin/sh is mksh. The problem is, mksh set COLUMNS
to be at least 12.

-- 
  Kacper Kornet

[PATCH] test: skip test with COLUMNS=1 under mksh

[Zbigniew Jędrzejewski-Szmek]

mksh does not allow $COLUMNS to be set below 12. Quoting mksh(1)
$COLUMNS is "always set, defaults to 80, unless the value as reported
by stty(1) is non-zero and sane enough". This applies also to setting
it directly for one command:

$ COLUMNS=30 python -c 'import os; print os.environ["COLUMNS"]'
30
$ COLUMNS=20 python -c 'import os; print os.environ["COLUMNS"]'
20
$ COLUMNS=10 python -c 'import os; print os.environ["COLUMNS"]'
98

Let's test if we can set COLUMNS=1, and if not, skip the tests that
want to use that. A prereq is added, which is set if $COLUMNS can be
set.

In t9002-column.sh, file with expected output was shared between two
test cases, but set in the first one. Since the first test case can
now be skipped, setting up the expected output is moved outside of the
test case.

Signed-off-by: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
---

This fixes problems under /bin/sh=mksh in two newly added test files:
t4052 and t9002. I think that doing it with a prereq is fairly
minimalistic, and the output looks nice.

 t/t4052-stat-output.sh |    6 ++++--
 t/t9002-column.sh      |    5 +++--
 t/test-lib.sh          |    1 +
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh
index ced32db..ddd9497 100755
--- a/t/t4052-stat-output.sh
+++ b/t/t4052-stat-output.sh
@@ -292,7 +292,8 @@ cat >expect1-graph <<'EOF'
 EOF
 while read verb expect cmd args
 do
-	test_expect_success "$cmd $verb prefix greater than COLUMNS (big change)" '
+	test_expect_success COLUMNS_CAN_BE_1 \
+		"$cmd $verb prefix greater than COLUMNS (big change)" '
 		COLUMNS=1 git $cmd $args >output
 		grep " | " output >actual &&
 		test_cmp "$expect" actual
@@ -300,7 +301,8 @@ do
 
 	test "$cmd" != diff || continue
 
-	test_expect_success "$cmd --graph $verb prefix greater than COLUMNS (big change)" '
+	test_expect_success COLUMNS_CAN_BE_1 \
+		"$cmd --graph $verb prefix greater than COLUMNS (big change)" '
 		COLUMNS=1 git $cmd $args --graph >output
 		grep " | " output >actual &&
 		test_cmp "$expect-graph" actual
diff --git a/t/t9002-column.sh b/t/t9002-column.sh
index c7d7a65..8998352 100755
--- a/t/t9002-column.sh
+++ b/t/t9002-column.sh
@@ -50,8 +50,7 @@ EOF
 	test_cmp expected actual
 '
 
-test_expect_success 'COLUMNS = 1' '
-	cat >expected <<\EOF &&
+cat >expected <<\EOF
 one
 two
 three
@@ -64,6 +63,8 @@ nine
 ten
 eleven
 EOF
+
+test_expect_success COLUMNS_CAN_BE_1 'COLUMNS = 1' '
 	COLUMNS=1 git column --mode=column <lista >actual &&
 	test_cmp expected actual
 '
diff --git a/t/test-lib.sh b/t/test-lib.sh
index b7d7100..9e2b711 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -615,6 +615,7 @@ case $(uname -s) in
 	;;
 esac
 
+( COLUMNS=1 && test $COLUMNS = 1 ) && test_set_prereq COLUMNS_CAN_BE_1
 test -z "$NO_PERL" && test_set_prereq PERL
 test -z "$NO_PYTHON" && test_set_prereq PYTHON
 test -n "$USE_LIBPCRE" && test_set_prereq LIBPCRE
-- 
1.7.10.433.g48a19

Re: [PATCH] test: skip test with COLUMNS=1 under mksh

[Junio C Hamano]

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

> mksh does not allow $COLUMNS to be set below 12. Quoting mksh(1)
> $COLUMNS is "always set, defaults to 80, unless the value as reported
> by stty(1) is non-zero and sane enough". This applies also to setting
> it directly for one command:
>
> $ COLUMNS=30 python -c 'import os; print os.environ["COLUMNS"]'
> 30
> $ COLUMNS=20 python -c 'import os; print os.environ["COLUMNS"]'
> 20
> $ COLUMNS=10 python -c 'import os; print os.environ["COLUMNS"]'
> 98

I'd hate to say this, but that shell is broken.

> Let's test if we can set COLUMNS=1, and if not, skip the tests that
> want to use that. A prereq is added, which is set if $COLUMNS can be
> set.

And I think that is a sensible workaround, even though COLUMNS_CAN_BE_1
might not be the best name for it.

> +( COLUMNS=1 && test $COLUMNS = 1 ) && test_set_prereq COLUMNS_CAN_BE_1

This is different from what you wrote in your log message, but I trust
that it will fail under the problematic shell?

Will queue; thanks.

Re: [PATCH] test: skip test with COLUMNS=1 under mksh

[Junio C Hamano]

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

> mksh does not allow $COLUMNS to be set below 12. Quoting mksh(1)
> $COLUMNS is "always set, defaults to 80, unless the value as reported
> by stty(1) is non-zero and sane enough". This applies also to setting
> it directly for one command:
>
> $ COLUMNS=30 python -c 'import os; print os.environ["COLUMNS"]'
> 30
> $ COLUMNS=20 python -c 'import os; print os.environ["COLUMNS"]'
> 20
> $ COLUMNS=10 python -c 'import os; print os.environ["COLUMNS"]'
> 98

I wonder if this is an easier workaround, though.

	env COLUMNS=10 $cmd

In any case, I'd split this patch to three, as these tests belong to two
independent topics.

Re: [PATCH] test: skip test with COLUMNS=1 under mksh

[Stefano Lattarini]

Hi Junio, Zbigniew.

On 04/27/2012 06:21 PM, Junio C Hamano wrote:
> Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> writes:
> 
>> mksh does not allow $COLUMNS to be set below 12. Quoting mksh(1)
>> $COLUMNS is "always set, defaults to 80, unless the value as reported
>> by stty(1) is non-zero and sane enough". This applies also to setting
>> it directly for one command:
>>
>> $ COLUMNS=30 python -c 'import os; print os.environ["COLUMNS"]'
>> 30
>> $ COLUMNS=20 python -c 'import os; print os.environ["COLUMNS"]'
>> 20
>> $ COLUMNS=10 python -c 'import os; print os.environ["COLUMNS"]'
>> 98
> 
> I wonder if this is an easier workaround, though.
> 
> 	env COLUMNS=10 $cmd
> 
It works for me:

  $ mksh -c 'COLUMNS=10 env | grep COLUMNS'
  COLUMNS=113

  $ mksh -c 'env COLUMNS=10 env | grep COLUMNS'
  COLUMNS=10

  $ dpkg -l mksh
  ...
  ii  mksh  40.2-2  MirBSD Korn Shell

HTH,
  Stefano

Re: [PATCH] test: skip test with COLUMNS=1 under mksh

[Zbigniew Jędrzejewski-Szmek]

On 04/28/2012 12:35 AM, Stefano Lattarini wrote:
> Hi Junio, Zbigniew.
> 
> On 04/27/2012 06:21 PM, Junio C Hamano wrote:
>> Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> writes:
>>
>>> mksh does not allow $COLUMNS to be set below 12. Quoting mksh(1)
>>> $COLUMNS is "always set, defaults to 80, unless the value as reported
>>> by stty(1) is non-zero and sane enough". This applies also to setting
>>> it directly for one command:
>>>
>>> $ COLUMNS=30 python -c 'import os; print os.environ["COLUMNS"]'
>>> 30
>>> $ COLUMNS=20 python -c 'import os; print os.environ["COLUMNS"]'
>>> 20
>>> $ COLUMNS=10 python -c 'import os; print os.environ["COLUMNS"]'
>>> 98
>>
>> I wonder if this is an easier workaround, though.
>>
>> 	env COLUMNS=10 $cmd
>>
> It works for me:
> 
>   $ mksh -c 'COLUMNS=10 env | grep COLUMNS'
>   COLUMNS=113
> 
>   $ mksh -c 'env COLUMNS=10 env | grep COLUMNS'
>   COLUMNS=10
> 
>   $ dpkg -l mksh
>   ...
>   ii  mksh  40.2-2  MirBSD Korn Shell
> 
> HTH,
>   Stefano
> 
This approach with 'env COLUMNS=1 git ...' looks nicer, and works when
invoked directly, but doesn't work when run under the test harness,
because bin-wrappers/git uses !/bin/sh, which resets COLUMNS. So this
simpler approach doesn't look feasible.

Zbyszek

Re: [PATCH] test: skip test with COLUMNS=1 under mksh

[Zbigniew Jędrzejewski-Szmek]

On 04/27/2012 06:08 PM, Junio C Hamano wrote:
> Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> writes:
> 
>> mksh does not allow $COLUMNS to be set below 12. Quoting mksh(1)
>> $COLUMNS is "always set, defaults to 80, unless the value as reported
>> by stty(1) is non-zero and sane enough". This applies also to setting
>> it directly for one command:
>>
>> $ COLUMNS=30 python -c 'import os; print os.environ["COLUMNS"]'
>> 30
>> $ COLUMNS=20 python -c 'import os; print os.environ["COLUMNS"]'
>> 20
>> $ COLUMNS=10 python -c 'import os; print os.environ["COLUMNS"]'
>> 98
> 
> I'd hate to say this, but that shell is broken.
Yes.

>> Let's test if we can set COLUMNS=1, and if not, skip the tests that
>> want to use that. A prereq is added, which is set if $COLUMNS can be
>> set.
> 
> And I think that is a sensible workaround, even though COLUMNS_CAN_BE_1
> might not be the best name for it.
> 
>> +( COLUMNS=1 && test $COLUMNS = 1 ) && test_set_prereq COLUMNS_CAN_BE_1
> 
> This is different from what you wrote in your log message, but I trust
> that it will fail under the problematic shell?
Yes.

> Will queue; thanks.
Cool.

Zbyszek

Re: [PATCH] test: skip test with COLUMNS=1 under mksh

[Junio C Hamano]

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

> This approach with 'env COLUMNS=1 git ...' looks nicer, and works when
> invoked directly, but doesn't work when run under the test harness,
> because bin-wrappers/git uses !/bin/sh, which resets COLUMNS. So this
> simpler approach doesn't look feasible.

Sigh... That shell seem to be infinitely broken X-<.