diff mbox series

[v3,07/13] graph: example of graph output that can be simplified

Message ID 631ee3cecb68d9f776d4a8fb30c1bca70797ba14.1571183279.git.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series Improve the readability of log --graph output | expand

Commit Message

John Passaro via GitGitGadget Oct. 15, 2019, 11:47 p.m. UTC 
From: James Coglan <jcoglan@gmail.com>

The commits following this one introduce a series of improvements to the
layout of graphs, tidying up a few edge cases, namely:

- merge whose first parent fuses with an existing column to the left
- merge whose last parent fuses with its immediate neighbor on the right
- edges that collapse to the left above and below a commit line

This test case exemplifies these cases and provides a motivating example
of the kind of history I'm aiming to clear up.

The first parent of merge E is the same as the parent of H, so those
edges fuse together.

        * H
        |
        | *-.   E
        | |\ \
        |/ / /
        |
        * B

We can "skew" the display of this merge so that it doesn't introduce
additional columns that immediately collapse:

        * H
        |
        | *   E
        |/|\
        |
        * B

The last parent of E is D, the same as the parent of F which is the edge
to the right of the merge.

            * F
            |
             \
          *-. \   E
          |\ \ \
         / / / /
            | /
            |/
            * D

The two edges leading to D could be fused sooner: rather than expanding
the F edge around the merge and then letting the edges collapse, the F
edge could fuse with the E edge in the post-merge line:

            * F
            |
             \
          *-. | E
          |\ \|
         / / /
            |
            * D

If this is combined with the "skew" effect above, we get a much cleaner
graph display for these edges:

            * F
            |
          * | E
         /|\|
            |
            * D

Finally, the edge leading from C to A appears jagged as it passes
through the commit line for B:

        | * | C
        | |/
        * | B
        |/
        * A

This can be smoothed out so that such edges are easier to read:

        | * | C
        | |/
        * / B
        |/
        * A

Signed-off-by: James Coglan <jcoglan@gmail.com>
---
 t/t4215-log-skewed-merges.sh | 43 ++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100755 t/t4215-log-skewed-merges.sh

Comments

Derrick Stolee Oct. 17, 2019, 12:30 p.m. UTC | #1 
On 10/15/2019 7:47 PM, James Coglan via GitGitGadget wrote:
> diff --git a/t/t4215-log-skewed-merges.sh b/t/t4215-log-skewed-merges.sh
> new file mode 100755
> index 0000000000..4582ba066a
> --- /dev/null
> +++ b/t/t4215-log-skewed-merges.sh
> @@ -0,0 +1,43 @@
> +#!/bin/sh
> +
> +test_description='git log --graph of skewed merges'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'log --graph with merge fusing with its left and right neighbors' '
> +	cat >expect <<-\EOF &&
> +	*   H
> +	|\
> +	| *   G
> +	| |\
> +	| | * F
> +	| | |
> +	| |  \
> +	| *-. \   E
> +	| |\ \ \
> +	|/ / / /
> +	| | | /
> +	| | |/
> +	| | * D
> +	| * | C
> +	| |/
> +	* | B
> +	|/
> +	* A
> +	EOF

Thanks for adding this test! I really think it helps show some
of your improvements later as this test is mutated.

-Stolee

> +
> +	git checkout --orphan _p &&
> +	test_commit A &&
> +	test_commit B &&
> +	git checkout -b _q @^ && test_commit C &&
> +	git checkout -b _r @^ && test_commit D &&
> +	git checkout _p && git merge --no-ff _q _r -m E &&
> +	git checkout _r && test_commit F &&
> +	git checkout _p && git merge --no-ff _r -m G &&
> +	git checkout @^^ && git merge --no-ff _p -m H &&
> +
> +	git log --graph --pretty=tformat:%s | sed "s/ *$//" >actual &&
> +	test_cmp expect actual
> +'
> +
> +test_done
>
SZEDER Gábor Oct. 18, 2019, 3:21 p.m. UTC | #2 
On Tue, Oct 15, 2019 at 11:47:53PM +0000, James Coglan via GitGitGadget wrote:
> diff --git a/t/t4215-log-skewed-merges.sh b/t/t4215-log-skewed-merges.sh
> new file mode 100755
> index 0000000000..4582ba066a
> --- /dev/null
> +++ b/t/t4215-log-skewed-merges.sh
> @@ -0,0 +1,43 @@
> +#!/bin/sh
> +
> +test_description='git log --graph of skewed merges'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'log --graph with merge fusing with its left and right neighbors' '
> +	cat >expect <<-\EOF &&
> +	*   H
> +	|\
> +	| *   G
> +	| |\
> +	| | * F
> +	| | |
> +	| |  \
> +	| *-. \   E
> +	| |\ \ \
> +	|/ / / /
> +	| | | /
> +	| | |/
> +	| | * D
> +	| * | C
> +	| |/
> +	* | B
> +	|/
> +	* A
> +	EOF
> +
> +	git checkout --orphan _p &&
> +	test_commit A &&
> +	test_commit B &&
> +	git checkout -b _q @^ && test_commit C &&
> +	git checkout -b _r @^ && test_commit D &&
> +	git checkout _p && git merge --no-ff _q _r -m E &&
> +	git checkout _r && test_commit F &&
> +	git checkout _p && git merge --no-ff _r -m G &&
> +	git checkout @^^ && git merge --no-ff _p -m H &&
> +
> +	git log --graph --pretty=tformat:%s | sed "s/ *$//" >actual &&

Please don't pipe 'git log --graph's output, but use an intermediate
file instead:

  git log --graph ... >out &&
  sed s/// out >actual &&
  test_cmp expect actual

The exit code of a pipeline is the exit code of the last process in
the pipeline, and the exit codes of processes upstream of a pipe are
ignored.  Consequently, if 'git log --graph' produced the expected
output but were to fail during housekeeping before exiting (segfault,
double free(), whatever), then that failure would go unnoticed.

This applies to several (all?) new tests added in this patch series as
well.


I'd like to join the praises from others: this is one excellent
first-time submission, thanks.
diff mbox series

Patch

diff --git a/t/t4215-log-skewed-merges.sh b/t/t4215-log-skewed-merges.sh
new file mode 100755
index 0000000000..4582ba066a
--- /dev/null
+++ b/t/t4215-log-skewed-merges.sh
@@ -0,0 +1,43 @@ 
+#!/bin/sh
+
+test_description='git log --graph of skewed merges'
+
+. ./test-lib.sh
+
+test_expect_success 'log --graph with merge fusing with its left and right neighbors' '
+	cat >expect <<-\EOF &&
+	*   H
+	|\
+	| *   G
+	| |\
+	| | * F
+	| | |
+	| |  \
+	| *-. \   E
+	| |\ \ \
+	|/ / / /
+	| | | /
+	| | |/
+	| | * D
+	| * | C
+	| |/
+	* | B
+	|/
+	* A
+	EOF
+
+	git checkout --orphan _p &&
+	test_commit A &&
+	test_commit B &&
+	git checkout -b _q @^ && test_commit C &&
+	git checkout -b _r @^ && test_commit D &&
+	git checkout _p && git merge --no-ff _q _r -m E &&
+	git checkout _r && test_commit F &&
+	git checkout _p && git merge --no-ff _r -m G &&
+	git checkout @^^ && git merge --no-ff _p -m H &&
+
+	git log --graph --pretty=tformat:%s | sed "s/ *$//" >actual &&
+	test_cmp expect actual
+'
+
+test_done