[GSoC RFC PATCH] graph: add --graph-max option to limit displayed columns

[GSoC RFC PATCH] graph: add --graph-max option to limit displayed columns

[Pablo Sabater]

When there are multiple branches, --graph-max modifies the maximum
amount of columns that will be displayed.

Add "--graph-max=<n>" option to cap how many columns will be shown,
columns after the limit are replaced with a single '.'. Changes only
the output rendering.

Define MINIMUM_GRAPH_COLUMNS constant to validate the option value.

The commit character '*' is always shown no matter what the limit is.

Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---

This addresses the TODO at graph.c:

  TODO:
      - Limit the number of columns, similar to the way gitk does.
        If we reach more than a specified number of columns, omit
        sections of some columns.

About the design of how this would have to be:

- Should '--graph-max' by itself be enough to implicitly work like '--graph' so 
  'git log --graph-max=3' works without needing to write '--graph'?
- graph_max_columns by default is set to 0, meaning no limit, and any other 
  positive value becomes a limit. Is this a good design? it cannot be negative,
  shouldn't it be a uint32_t instead, I left it as a int because of the other
  variables like this that are int. like skip_count, max_count, etc.
- Is '--graph-max' a good name?
- Is '.' a good char for truncation?
- Should '/' to outside branches be shown?
- What should it be done when a commit is in a column that is truncated?

known limitations:

- Post merge lines have some trouble with the padding.

I added two tests for example, but I will add better test coverage as design 
choices are more clear. testing on the Git repo itself is a good example also.

 graph.c                      | 52 +++++++++++++++++++++++++++------
 graph.h                      |  2 ++
 revision.c                   |  7 +++++
 revision.h                   |  1 +
 t/t4215-log-skewed-merges.sh | 56 ++++++++++++++++++++++++++++++++++++
 5 files changed, 109 insertions(+), 9 deletions(-)

diff --git a/graph.c b/graph.c
index 26f6fbf000..7ae0ab61b7 100644
--- a/graph.c
+++ b/graph.c
@@ -42,14 +42,6 @@ static void graph_padding_line(struct git_graph *graph, struct strbuf *sb);
 static void graph_show_strbuf(struct git_graph *graph,
 			      FILE *file,
 			      struct strbuf const *sb);
-
-/*
- * TODO:
- * - Limit the number of columns, similar to the way gitk does.
- *   If we reach more than a specified number of columns, omit
- *   sections of some columns.
- */
-
 struct column {
 	/*
 	 * The parent commit of this column.
@@ -317,6 +309,12 @@ struct git_graph {
 	struct strbuf prefix_buf;
 };
 
+static int graph_is_truncated(struct git_graph *graph, int col)
+{
+	int max = graph->revs->graph_max_columns;
+	return max > 0 && col >= max;
+}
+
 static const char *diff_output_prefix_callback(struct diff_options *opt, void *data)
 {
 	struct git_graph *graph = data;
@@ -846,6 +844,10 @@ static void graph_output_padding_line(struct git_graph *graph,
 	 * Output a padding row, that leaves all branch lines unchanged
 	 */
 	for (i = 0; i < graph->num_new_columns; i++) {
+		if (graph_is_truncated(graph, i)) {
+			graph_line_addstr(line, ". ");
+			break;
+		}
 		graph_line_write_column(line, &graph->new_columns[i], '|');
 		graph_line_addch(line, ' ');
 	}
@@ -903,6 +905,9 @@ static void graph_output_pre_commit_line(struct git_graph *graph,
 			seen_this = 1;
 			graph_line_write_column(line, col, '|');
 			graph_line_addchars(line, ' ', graph->expansion_row);
+		} else if (seen_this && graph_is_truncated(graph, i)) {
+			graph_line_addstr(line, ". ");
+			break;
 		} else if (seen_this && (graph->expansion_row == 0)) {
 			/*
 			 * This is the first line of the pre-commit output.
@@ -1013,6 +1018,7 @@ static void graph_output_commit_line(struct git_graph *graph, struct graph_line
 	 * children that we have already processed.)
 	 */
 	seen_this = 0;
+
 	for (i = 0; i <= graph->num_columns; i++) {
 		struct column *col = &graph->columns[i];
 		struct commit *col_commit;
@@ -1028,8 +1034,14 @@ static void graph_output_commit_line(struct git_graph *graph, struct graph_line
 			seen_this = 1;
 			graph_output_commit_char(graph, line);
 
+			if (graph_is_truncated(graph, i))
+				break;
+
 			if (graph->num_parents > 2)
 				graph_draw_octopus_merge(graph, line);
+		} else if (seen_this && graph_is_truncated(graph, i)) {
+			graph_line_addstr(line, ". ");
+			break;
 		} else if (seen_this && (graph->edges_added > 1)) {
 			graph_line_write_column(line, col, '\\');
 		} else if (seen_this && (graph->edges_added == 1)) {
@@ -1109,9 +1121,15 @@ static void graph_output_post_merge_line(struct git_graph *graph, struct graph_l
 			int par_column;
 			int idx = graph->merge_layout;
 			char c;
+			int truncated = 0;
 			seen_this = 1;
 
 			for (j = 0; j < graph->num_parents; j++) {
+				if (graph_is_truncated(graph, i + j)) {
+					graph_line_addstr(line, ". ");
+					truncated = 1;
+					break;
+				}
 				par_column = graph_find_new_column_by_commit(graph, parents->item);
 				assert(par_column >= 0);
 
@@ -1125,10 +1143,15 @@ static void graph_output_post_merge_line(struct git_graph *graph, struct graph_l
 				}
 				parents = next_interesting_parent(graph, parents);
 			}
+			if (truncated)
+				break;
 			if (graph->edges_added == 0)
 				graph_line_addch(line, ' ');
-
 		} else if (seen_this) {
+			if (graph_is_truncated(graph, i)) {
+				graph_line_addstr(line, ". ");
+				break;
+			}
 			if (graph->edges_added > 0)
 				graph_line_write_column(line, col, '\\');
 			else
@@ -1279,6 +1302,12 @@ static void graph_output_collapsing_line(struct git_graph *graph, struct graph_l
 	 */
 	for (i = 0; i < graph->mapping_size; i++) {
 		int target = graph->mapping[i];
+
+		if (graph_is_truncated(graph, i / 2)) {
+			graph_line_addstr(line, ". ");
+			break;
+		}
+
 		if (target < 0)
 			graph_line_addch(line, ' ');
 		else if (target * 2 == i)
@@ -1372,6 +1401,11 @@ static void graph_padding_line(struct git_graph *graph, struct strbuf *sb)
 	for (i = 0; i < graph->num_columns; i++) {
 		struct column *col = &graph->columns[i];
 
+		if (graph_is_truncated(graph, i)) {
+			graph_line_addch(&line, '.');
+			break;
+		}
+
 		graph_line_write_column(&line, col, '|');
 
 		if (col->commit == graph->commit && graph->num_parents > 2) {
diff --git a/graph.h b/graph.h
index 3fd1dcb2e9..9a4551dd29 100644
--- a/graph.h
+++ b/graph.h
@@ -262,4 +262,6 @@ void graph_show_commit_msg(struct git_graph *graph,
 			   FILE *file,
 			   struct strbuf const *sb);
 
+#define MINIMUM_GRAPH_COLUMNS 1
+
 #endif /* GRAPH_H */
diff --git a/revision.c b/revision.c
index 31808e3df0..ba5088be14 100644
--- a/revision.c
+++ b/revision.c
@@ -2605,6 +2605,13 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 	} else if (!strcmp(arg, "--no-graph")) {
 		graph_clear(revs->graph);
 		revs->graph = NULL;
+	} else if (skip_prefix(arg, "--graph-max=", &optarg)) {
+		revs->graph_max_columns = strtoul(optarg, NULL, 10);
+		if (revs->graph_max_columns < MINIMUM_GRAPH_COLUMNS) {
+			die(_("minimum columns is %d, unable to set below %d"),
+			MINIMUM_GRAPH_COLUMNS,
+			revs->graph_max_columns);
+		}
 	} else if (!strcmp(arg, "--encode-email-headers")) {
 		revs->encode_email_headers = 1;
 	} else if (!strcmp(arg, "--no-encode-email-headers")) {
diff --git a/revision.h b/revision.h
index 69242ecb18..6442129c14 100644
--- a/revision.h
+++ b/revision.h
@@ -304,6 +304,7 @@ struct rev_info {
 
 	/* Display history graph */
 	struct git_graph *graph;
+	int graph_max_columns;
 
 	/* special limits */
 	int skip_count;
diff --git a/t/t4215-log-skewed-merges.sh b/t/t4215-log-skewed-merges.sh
index 28d0779a8c..6266de4e2b 100755
--- a/t/t4215-log-skewed-merges.sh
+++ b/t/t4215-log-skewed-merges.sh
@@ -370,4 +370,60 @@ test_expect_success 'log --graph with multiple tips' '
 	EOF
 '
 
+test_expect_success 'log --graph --graph-max=2 only two columns' '
+	check_graph --graph-max=2 M_7 <<-\EOF
+	*-.   7_M4
+	|\ .
+	| | * 7_G
+	| | * 7_F
+	| * . 7_E
+	| * . 7_D
+	* | . 7_C
+	| |/
+	|/|
+	* | 7_B
+	|/
+	* 7_A
+	EOF
+'
+
+test_expect_success 'log --graph --graph-max=3 only three columns' '
+	check_graph --graph-max=3 M_1 M_3 M_5 M_7 <<-\EOF
+	*   7_M1
+	|\
+	| | *   7_M2
+	| | |.
+	| | | * 7_H
+	| | | | *   7_M3
+	| | | | .
+	| | | | | * 7_J
+	| | | | *   7_I
+	| | | | | | *   7_M4
+	| |_|_|_|_|.
+	|/| | .
+	| | |_.
+	| |/|_.
+	| |/|_.
+	| |/| .
+	| | |/.
+	| | * .     7_G
+	| | | .
+	| | |/.
+	| | |/.
+	| | * .   7_F
+	| * | .   7_E
+	| | |/.
+	| |/| .
+	| * | . 7_D
+	| | |/
+	| |/|
+	* | | 7_C
+	| |/
+	|/|
+	* | 7_B
+	|/
+	* 7_A
+	EOF
+'
+
 test_done
-- 
2.43.0

Re: [GSoC RFC PATCH] graph: add --graph-max option to limit displayed columns

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 13550 bytes --]

Pablo Sabater <pabloosabaterr@gmail.com> writes:

> When there are multiple branches, --graph-max modifies the maximum
> amount of columns that will be displayed.
>
> Add "--graph-max=<n>" option to cap how many columns will be shown,
> columns after the limit are replaced with a single '.'. Changes only
> the output rendering.
>

The first sentence seems to talk about the option like it already
exists, when the second para introduces it. It would be nice if the
first para explained the problem we're trying to solve and why and the
second para then dove into the solution space.

Do you think '--graph-max' signifies that we're talking about the
maximum columns to display?

> Define MINIMUM_GRAPH_COLUMNS constant to validate the option value.

What does this mean? Validate how?

> The commit character '*' is always shown no matter what the limit is.

I think overall a little more explanation in the commit message makes it
easier to understand the context and also helps reviewers!

Shouldn't we also talk about the todo and the commit (c12172d2ea (Add
history graph API, 2008-05-04)) in which it was added?

>
> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
> ---
>
> This addresses the TODO at graph.c:
>
>   TODO:
>       - Limit the number of columns, similar to the way gitk does.
>         If we reach more than a specified number of columns, omit
>         sections of some columns.
>

One question to ask is, is this even needed anymore and does it really
make sense to add it?

The TODO was added back in 2008, that's ~16 years ago and was not
touched till now. So perhaps no one needs it? If so, maybe the smarter
option is to simply remove the TODO?

Or do you see a usecase where this is useful? If so, it would be nice to
talk about that in the commit message.

> About the design of how this would have to be:
>
> - Should '--graph-max' by itself be enough to implicitly work like '--graph' so
>   'git log --graph-max=3' works without needing to write '--graph'?

My preference would be not to have implicit behavior but also at the
same time guide the user in the right path, so:

     $ git log --graph-max=4
     fatal: --graph-max used without --graph

> - graph_max_columns by default is set to 0, meaning no limit, and any other
>   positive value becomes a limit. Is this a good design? it cannot be negative,
>   shouldn't it be a uint32_t instead, I left it as a int because of the other
>   variables like this that are int. like skip_count, max_count, etc.
> - Is '--graph-max' a good name?

I would argue against it. Perhaps '--graph-col-limit'? It's a bit handy though.

> - Is '.' a good char for truncation?

Trying it out:

$ git log --graph --oneline --graph-max=3

| * ba1c21d343 odb: split `struct odb_source` into separate header
| *   b1af291b4a Merge branch 'ps/object-info-bits-cleanup' into ps/odb-sources
| |\
| * \   703c97519d Merge branch 'ps/odb-for-each-object' into ps/odb-sources
| |\ \
* | \ .   d0413b31dd Merge branch 'hn/status-compare-with-push'
|\ \ \ .
| * | .   68791d7506 status: clarify how status.compareBranches deduplicates
| * | .   3ea95ac9c5 (gitster/hn/status-compare-with-push) status: add
status.compareBranches config for multiple branch comparisons
| * | .   04f47265c1 refactor format_branch_comparison in preparation
| * | .     2aa9b75b43 Merge branch 'jk/remote-tracking-ref-leakfix'
into hn/status-compare-with-push
| |\ \ .
* | \ .       03161747b4 Merge branch 'ds/for-each-repo-w-worktree'
|\ \ \ .
| * | .       e87493b9b4 for-each-repo: simplify passing of parameters
| * | .       2ef539bcee for-each-repo: work correctly in a worktree
| * | .       5f031fe4f1 run-command: extract sanitize_repo_env helper
| * | .       c5e62e1aa0 for-each-repo: test outside of repo context
* | | .       67006b9db8 The 15th batch
* | | .         99da934835 Merge branch 'sp/send-email-validate-charset'
|\ \ \ .
| * | .         c52f085a47 (gitster/sp/send-email-validate-charset)
send-email: validate charset name in 8bit encoding prompt

vs

$ git log --graph --oneline

| * ba1c21d343 odb: split `struct odb_source` into separate header
| *   b1af291b4a Merge branch 'ps/object-info-bits-cleanup' into ps/odb-sources
| |\
| * \   703c97519d Merge branch 'ps/odb-for-each-object' into ps/odb-sources
| |\ \
* | \ \   d0413b31dd Merge branch 'hn/status-compare-with-push'
|\ \ \ \
| * | | | 68791d7506 status: clarify how status.compareBranches deduplicates
| * | | | 3ea95ac9c5 (gitster/hn/status-compare-with-push) status: add
status.compareBranches config for multiple branch comparisons
| * | | | 04f47265c1 refactor format_branch_comparison in preparation
| * | | |   2aa9b75b43 Merge branch 'jk/remote-tracking-ref-leakfix'
into hn/status-compare-with-push
| |\ \ \ \
* | \ \ \ \   03161747b4 Merge branch 'ds/for-each-repo-w-worktree'
|\ \ \ \ \ \
| * | | | | | e87493b9b4 for-each-repo: simplify passing of parameters
| * | | | | | 2ef539bcee for-each-repo: work correctly in a worktree
| * | | | | | 5f031fe4f1 run-command: extract sanitize_repo_env helper
| * | | | | | c5e62e1aa0 for-each-repo: test outside of repo context
* | | | | | | 67006b9db8 The 15th batch
* | | | | | |   99da934835 Merge branch 'sp/send-email-validate-charset'
|\ \ \ \ \ \ \
| * | | | | | | c52f085a47 (gitster/sp/send-email-validate-charset)
send-email: validate charset name in 8bit encoding prompt

So we still keep the spaces, but only remove the column indicator

> - Should '/' to outside branches be shown?
> - What should it be done when a commit is in a column that is truncated?
>
> known limitations:
>
> - Post merge lines have some trouble with the padding.
>
> I added two tests for example, but I will add better test coverage as design
> choices are more clear. testing on the Git repo itself is a good example also.
>
>  graph.c                      | 52 +++++++++++++++++++++++++++------
>  graph.h                      |  2 ++
>  revision.c                   |  7 +++++
>  revision.h                   |  1 +
>  t/t4215-log-skewed-merges.sh | 56 ++++++++++++++++++++++++++++++++++++
>  5 files changed, 109 insertions(+), 9 deletions(-)
>
> diff --git a/graph.c b/graph.c
> index 26f6fbf000..7ae0ab61b7 100644
> --- a/graph.c
> +++ b/graph.c
> @@ -42,14 +42,6 @@ static void graph_padding_line(struct git_graph *graph, struct strbuf *sb);
>  static void graph_show_strbuf(struct git_graph *graph,
>  			      FILE *file,
>  			      struct strbuf const *sb);
> -
> -/*
> - * TODO:
> - * - Limit the number of columns, similar to the way gitk does.
> - *   If we reach more than a specified number of columns, omit
> - *   sections of some columns.
> - */
> -
>  struct column {
>  	/*
>  	 * The parent commit of this column.
> @@ -317,6 +309,12 @@ struct git_graph {
>  	struct strbuf prefix_buf;
>  };
>
> +static int graph_is_truncated(struct git_graph *graph, int col)

Isn't this more of `graphs_needs_truncation()`?

> +{
> +	int max = graph->revs->graph_max_columns;
> +	return max > 0 && col >= max;
> +}
> +
>  static const char *diff_output_prefix_callback(struct diff_options *opt, void *data)
>  {
>  	struct git_graph *graph = data;
> @@ -846,6 +844,10 @@ static void graph_output_padding_line(struct git_graph *graph,
>  	 * Output a padding row, that leaves all branch lines unchanged
>  	 */
>  	for (i = 0; i < graph->num_new_columns; i++) {
> +		if (graph_is_truncated(graph, i)) {
> +			graph_line_addstr(line, ". ");
> +			break;
> +		}
>  		graph_line_write_column(line, &graph->new_columns[i], '|');
>  		graph_line_addch(line, ' ');
>  	}
> @@ -903,6 +905,9 @@ static void graph_output_pre_commit_line(struct git_graph *graph,
>  			seen_this = 1;
>  			graph_line_write_column(line, col, '|');
>  			graph_line_addchars(line, ' ', graph->expansion_row);
> +		} else if (seen_this && graph_is_truncated(graph, i)) {
> +			graph_line_addstr(line, ". ");
> +			break;
>  		} else if (seen_this && (graph->expansion_row == 0)) {
>  			/*
>  			 * This is the first line of the pre-commit output.
> @@ -1013,6 +1018,7 @@ static void graph_output_commit_line(struct git_graph *graph, struct graph_line
>  	 * children that we have already processed.)
>  	 */
>  	seen_this = 0;
> +
>  	for (i = 0; i <= graph->num_columns; i++) {
>  		struct column *col = &graph->columns[i];
>  		struct commit *col_commit;
> @@ -1028,8 +1034,14 @@ static void graph_output_commit_line(struct git_graph *graph, struct graph_line
>  			seen_this = 1;
>  			graph_output_commit_char(graph, line);
>
> +			if (graph_is_truncated(graph, i))
> +				break;
> +
>  			if (graph->num_parents > 2)
>  				graph_draw_octopus_merge(graph, line);
> +		} else if (seen_this && graph_is_truncated(graph, i)) {
> +			graph_line_addstr(line, ". ");
> +			break;
>  		} else if (seen_this && (graph->edges_added > 1)) {
>  			graph_line_write_column(line, col, '\\');
>  		} else if (seen_this && (graph->edges_added == 1)) {
> @@ -1109,9 +1121,15 @@ static void graph_output_post_merge_line(struct git_graph *graph, struct graph_l
>  			int par_column;
>  			int idx = graph->merge_layout;
>  			char c;
> +			int truncated = 0;
>  			seen_this = 1;
>
>  			for (j = 0; j < graph->num_parents; j++) {
> +				if (graph_is_truncated(graph, i + j)) {
> +					graph_line_addstr(line, ". ");
> +					truncated = 1;
> +					break;
> +				}
>  				par_column = graph_find_new_column_by_commit(graph, parents->item);
>  				assert(par_column >= 0);
>
> @@ -1125,10 +1143,15 @@ static void graph_output_post_merge_line(struct git_graph *graph, struct graph_l
>  				}
>  				parents = next_interesting_parent(graph, parents);
>  			}
> +			if (truncated)
> +				break;
>  			if (graph->edges_added == 0)
>  				graph_line_addch(line, ' ');
> -
>  		} else if (seen_this) {
> +			if (graph_is_truncated(graph, i)) {
> +				graph_line_addstr(line, ". ");
> +				break;
> +			}
>  			if (graph->edges_added > 0)
>  				graph_line_write_column(line, col, '\\');
>  			else
> @@ -1279,6 +1302,12 @@ static void graph_output_collapsing_line(struct git_graph *graph, struct graph_l
>  	 */
>  	for (i = 0; i < graph->mapping_size; i++) {
>  		int target = graph->mapping[i];
> +
> +		if (graph_is_truncated(graph, i / 2)) {
> +			graph_line_addstr(line, ". ");
> +			break;
> +		}
> +
>  		if (target < 0)
>  			graph_line_addch(line, ' ');
>  		else if (target * 2 == i)
> @@ -1372,6 +1401,11 @@ static void graph_padding_line(struct git_graph *graph, struct strbuf *sb)
>  	for (i = 0; i < graph->num_columns; i++) {
>  		struct column *col = &graph->columns[i];
>
> +		if (graph_is_truncated(graph, i)) {
> +			graph_line_addch(&line, '.');
> +			break;
> +		}
> +
>  		graph_line_write_column(&line, col, '|');
>
>  		if (col->commit == graph->commit && graph->num_parents > 2) {
> diff --git a/graph.h b/graph.h
> index 3fd1dcb2e9..9a4551dd29 100644
> --- a/graph.h
> +++ b/graph.h
> @@ -262,4 +262,6 @@ void graph_show_commit_msg(struct git_graph *graph,
>  			   FILE *file,
>  			   struct strbuf const *sb);
>
> +#define MINIMUM_GRAPH_COLUMNS 1
> +
>  #endif /* GRAPH_H */
> diff --git a/revision.c b/revision.c
> index 31808e3df0..ba5088be14 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -2605,6 +2605,13 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
>  	} else if (!strcmp(arg, "--no-graph")) {
>  		graph_clear(revs->graph);
>  		revs->graph = NULL;
> +	} else if (skip_prefix(arg, "--graph-max=", &optarg)) {
> +		revs->graph_max_columns = strtoul(optarg, NULL, 10);
> +		if (revs->graph_max_columns < MINIMUM_GRAPH_COLUMNS) {
> +			die(_("minimum columns is %d, unable to set below %d"),
> +			MINIMUM_GRAPH_COLUMNS,
> +			revs->graph_max_columns);

Shouldn't we allow users to set 0? That combined with an unsigned int
would:
1. remove the need for MINIMUM_GRAPH_COLUMNS
2. allow users to specify that they do not want a column limit

> +		}
>  	} else if (!strcmp(arg, "--encode-email-headers")) {
>  		revs->encode_email_headers = 1;
>  	} else if (!strcmp(arg, "--no-encode-email-headers")) {
> diff --git a/revision.h b/revision.h
> index 69242ecb18..6442129c14 100644
> --- a/revision.h
> +++ b/revision.h
> @@ -304,6 +304,7 @@ struct rev_info {
>
>  	/* Display history graph */
>  	struct git_graph *graph;
> +	int graph_max_columns;
>

I think it makes sense to make this an unsigned int.

>  	/* special limits */
>  	int skip_count;
> diff --git a/t/t4215-log-skewed-merges.sh b/t/t4215-log-skewed-merges.sh
> index 28d0779a8c..6266de4e2b 100755
> --- a/t/t4215-log-skewed-merges.sh
> +++ b/t/t4215-log-skewed-merges.sh
> @@ -370,4 +370,60 @@ test_expect_success 'log --graph with multiple tips' '
>  	EOF
>  '
>
> +test_expect_success 'log --graph --graph-max=2 only two columns' '
> +	check_graph --graph-max=2 M_7 <<-\EOF
> +	*-.   7_M4
> +	|\ .
> +	| | * 7_G
> +	| | * 7_F
> +	| * . 7_E
> +	| * . 7_D
> +	* | . 7_C
> +	| |/
> +	|/|
> +	* | 7_B
> +	|/
> +	* 7_A
> +	EOF
> +'
> +
> +test_expect_success 'log --graph --graph-max=3 only three columns' '
> +	check_graph --graph-max=3 M_1 M_3 M_5 M_7 <<-\EOF
> +	*   7_M1
> +	|\
> +	| | *   7_M2
> +	| | |.
> +	| | | * 7_H
> +	| | | | *   7_M3
> +	| | | | .
> +	| | | | | * 7_J
> +	| | | | *   7_I
> +	| | | | | | *   7_M4
> +	| |_|_|_|_|.
> +	|/| | .
> +	| | |_.
> +	| |/|_.
> +	| |/|_.
> +	| |/| .
> +	| | |/.
> +	| | * .     7_G
> +	| | | .
> +	| | |/.
> +	| | |/.
> +	| | * .   7_F
> +	| * | .   7_E
> +	| | |/.
> +	| |/| .
> +	| * | . 7_D
> +	| | |/
> +	| |/|
> +	* | | 7_C
> +	| |/
> +	|/|
> +	* | 7_B
> +	|/
> +	* 7_A
> +	EOF
> +'
> +
>  test_done
> --
> 2.43.0
v

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [GSoC RFC PATCH] graph: add --graph-max option to limit displayed columns

[Pablo]

Thanks for the feedback,

> Do you think '--graph-max' signifies that we're talking about the
> maximum columns to display?

Yeah, doesn't  seem clear enough, but --graph-col-limit seems too
verbose. What do you think about --graph-max-cols ? It's just one char
less but I feel it is more readable.

> One question to ask is, is this even needed anymore and does it really
> make sense to add it?

> The TODO was added back in 2008, that's ~16 years ago and was not
> touched till now. So perhaps no one needs it? If so, maybe the smarter
> option is to simply remove the TODO?

> Or do you see a usecase where this is useful? If so, it would be nice to
> talk about that in the commit message.

I've run 'git log --graph --all' on the Git repo itself  on 'next' and
just scrolling a bit
down up to March 5 there are already +35 branches, which isn't very readable.

It's been 16 years but I believe that is still a good thing to add, even if
a lot of graph viewing happens with third parties, adding this makes
Git a bit more self sufficient. This is even more useful in a case where the
third party is not an option.

That said, if this still doesn't seem useful I would remove the TODO
to avoid further confusion in the future.

> My preference would be not to have implicit behavior but also at the
> same time guide the user in the right path, so:

>      $ git log --graph-max=4
>      fatal: --graph-max used without --graph

Ok, will add this to v2.

> Trying it out:
> ...
> So we still keep the spaces, but only remove the column indicator

Padding needs to be adjusted to the columns truncated. Will add it to v2.

> Isn't this more of `graphs_needs_truncation()`?

Yes, I'll rename it in v2.

> Shouldn't we allow users to set 0? That combined with an unsigned int
> would:
> 1. remove the need for MINIMUM_GRAPH_COLUMNS
> 2. allow users to specify that they do not want a column limit

I think it's better not to let 0 be a good input, it's the same as not using it
making it redundant. Is set to 0 by default from the memset().
Other "max" options I've tried that allow 0, don't share the "no limit"
behaviour.
  git log --graph --all --max-parents=0
  git log --max-count=0
That's why I wouldn't let it be valid.
The unsigned int is better for clarity.

> I think overall a little more explanation in the commit message makes it
> easier to understand the context and also helps reviewers!
>
> Shouldn't we also talk about the todo and the commit (c12172d2ea (Add
> history graph API, 2008-05-04)) in which it was added?

> The first sentence seems to talk about the option like it already
> exists, when the second para introduces it. It would be nice if the
> first para explained the problem we're trying to solve and why and the
> second para then dove into the solution space.

My bad, I'll improve the commit msg on v2 to be more clear about the
problem, what it does and where it comes from.

> What does this mean? Validate how?

It validated user input to avoid them to set 0. In case of letting them
set 0 (no limit) this would be removed.

I'll wait for the decision about whether this TODO from 2008 is worth
doing before a v2.

Thanks,
Pablo

[GSoC RFC PATCH v2] graph: add --max-columns option to limit displayed columns

[Pablo Sabater]

Repositories that have many active branches produce very wide
outputs with 'git log --graph --all', makes it difficult to read.

Add '--max-columns=<n>' to limit the columns shown. Columns over
the limit are replaced with a '.' truncation indicator. This only
affects the visual rendering.

The commit mark '*' is only shown in two cases:
- The commit is in a branch inside the limit.
- The commit is in the first hidden branch, in this case '.'
  is replaced by '*'.

Commits on deeper hidden branches do not show '*' but the commit
subject is still shown, so no information is lost.

The original idea to limit columns was noted as a TODO in
c12172d2ea (Add history graph API, 2008-05-04), which mentions
gitk's behavior. This does not implement gitk-style column
rearrangement; it only truncates the visual output.

Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---

> I'll wait for the decision about whether this TODO from 2008 is worth
> doing before a v2.

I believe that it is better to send an improved version instead of waiting
to show more clearly what I want to do, sorry for the confusion.

> It's been 16 years but I believe that is still a good thing to add, even if
> a lot of graph viewing happens with third parties, adding this makes
> Git a bit more self sufficient. This is even more useful in a case where the
> third party is not an option.

> That said, if this still doesn't seem useful I would remove the TODO
> to avoid further confusion in the future.

I revert what I've said here. This doesn't actually reflects gitk behaviour, 
it only truncates the visual output and prettifies it. To actually fullfill the
TODO, it should rearrange the columns what would mean to change the whole
rendering algorithm of --graph. This prob why it has been a TODO for so long.

I don't think the TODO should be removed with what I bring here. Therefore, I
wont't remove it.

The actual TODO seems still a good project and a good improvement.

example:

git log --oneline --graph --all

*   b81d099 (M_1) 7_M1
|\
| | *   2d96ba0 (M_3) 7_M2
| | |\
| | | * 993fbcd (7_4) 7_H
| | | | *   d5b56db (M_5) 7_M3
| | | | |\
| | | | | * b45d68d (7_6) 7_J
| | | | * | 3f9ab79 (7_5) 7_I
| | | | | | *   f89bbdc (HEAD -> M_7) 7_M4
| |_|_|_|_|/|\
|/| | | | |/ /
| | |_|_|/| /
| |/| | | |/
| | | |_|/|
| | |/| | |
| | * | | | f5cbb18 (7_3) 7_G
| | | |_|/
| | |/| |
| | * | | 6ee728a 7_F
| * | | | d0c5a67 (7_2) 7_E
| | |/ /
| |/| |
| * | | c3bffce 7_D
| | |/
| |/|
* | | df1a24f (7_1) 7_C
| |/
|/|
* | 5d3f777 7_B
|/
* d309d64 7_A

vs 

git log --graph --all --oneline --max-columns=2

*   b81d099 (M_1) 7_M1
|\
| | * 2d96ba0 (M_3) 7_M2
| | .
| | . 993fbcd (7_4) 7_H
| | . d5b56db (M_5) 7_M3
| | .
| | . b45d68d (7_6) 7_J
| | . 3f9ab79 (7_5) 7_I
| | . f89bbdc (HEAD -> M_7) 7_M4
| |_.
|/| .
| | .
| |/.
| |/.
| |/.
| | .
| | * f5cbb18 (7_3) 7_G
| | .
| | .
| | .
| | * 6ee728a 7_F
| * . d0c5a67 (7_2) 7_E
| | .
| |/.
| * . c3bffce 7_D
| | .
| |/.
* | . df1a24f (7_1) 7_C
| |/
|/|
* | 5d3f777 7_B
|/
* d309d64 7_A

Changes since v1:
- Renamed option from --graph-max to --max-columns
- Require --graph for --max-columns, die without it
- Fixed padding so commit text aligns correctly after truncation
- Added pre-commit truncation so lines before the commit column
  are also truncated when they exceed the limit
- Fixed post-merge line spacing inconsistency
- Used parse_long_opt/parse_count for input validation, matching
  existing revision.c patterns
- Reject negative and zero values with clear error messages
- Renamed graph_is_truncated to graph_needs_truncation

RFC :
- I think --max-columns is a good name, but it does not have --graph
  prefix, bcs it can only be on --graph I think it's clear enough.
  I added checks in setup_revisions().
- Is '.' fine as delimiter ? other options could be "~" or "-".

 graph.c                      | 57 +++++++++++++++++++++++++++++++++++-
 graph.h                      |  2 ++
 revision.c                   | 11 +++++++
 revision.h                   |  1 +
 t/t4215-log-skewed-merges.sh | 56 +++++++++++++++++++++++++++++++++++
 5 files changed, 126 insertions(+), 1 deletion(-)

diff --git a/graph.c b/graph.c
index 26f6fbf000..6227c4f22f 100644
--- a/graph.c
+++ b/graph.c
@@ -317,6 +317,12 @@ struct git_graph {
 	struct strbuf prefix_buf;
 };
 
+static int graph_needs_truncation(struct git_graph *graph, int col)
+{
+	int max = graph->revs->graph_max_columns;
+	return max > 0 && col >= max;
+}
+
 static const char *diff_output_prefix_callback(struct diff_options *opt, void *data)
 {
 	struct git_graph *graph = data;
@@ -696,6 +702,15 @@ static void graph_update_columns(struct git_graph *graph)
 		}
 	}
 
+	/*
+	 * If graph_max_columns is set, cap the padding from the branches
+	 */
+	if (graph->revs->graph_max_columns > 0) {
+		int truncation = graph->revs->graph_max_columns * 2 + 2;
+		if (graph->width > truncation)
+			graph->width = truncation;
+	}
+
 	/*
 	 * Shrink mapping_size to be the minimum necessary
 	 */
@@ -846,6 +861,10 @@ static void graph_output_padding_line(struct git_graph *graph,
 	 * Output a padding row, that leaves all branch lines unchanged
 	 */
 	for (i = 0; i < graph->num_new_columns; i++) {
+		if (graph_needs_truncation(graph, i)) {
+			graph_line_addstr(line, ". ");
+			break;
+		}
 		graph_line_write_column(line, &graph->new_columns[i], '|');
 		graph_line_addch(line, ' ');
 	}
@@ -903,6 +922,9 @@ static void graph_output_pre_commit_line(struct git_graph *graph,
 			seen_this = 1;
 			graph_line_write_column(line, col, '|');
 			graph_line_addchars(line, ' ', graph->expansion_row);
+		} else if (seen_this && graph_needs_truncation(graph, i)) {
+			graph_line_addstr(line, ". ");
+			break;
 		} else if (seen_this && (graph->expansion_row == 0)) {
 			/*
 			 * This is the first line of the pre-commit output.
@@ -1013,6 +1035,7 @@ static void graph_output_commit_line(struct git_graph *graph, struct graph_line
 	 * children that we have already processed.)
 	 */
 	seen_this = 0;
+
 	for (i = 0; i <= graph->num_columns; i++) {
 		struct column *col = &graph->columns[i];
 		struct commit *col_commit;
@@ -1028,8 +1051,17 @@ static void graph_output_commit_line(struct git_graph *graph, struct graph_line
 			seen_this = 1;
 			graph_output_commit_char(graph, line);
 
+			if (graph_needs_truncation(graph, i)) {
+				graph_line_addch(line, ' ');
+				break;
+			}
+
 			if (graph->num_parents > 2)
 				graph_draw_octopus_merge(graph, line);
+		} else if (graph_needs_truncation(graph, i)) {
+			graph_line_addstr(line, ". ");
+			seen_this = 1;
+			break;
 		} else if (seen_this && (graph->edges_added > 1)) {
 			graph_line_write_column(line, col, '\\');
 		} else if (seen_this && (graph->edges_added == 1)) {
@@ -1109,9 +1141,17 @@ static void graph_output_post_merge_line(struct git_graph *graph, struct graph_l
 			int par_column;
 			int idx = graph->merge_layout;
 			char c;
+			int truncated = 0;
 			seen_this = 1;
 
 			for (j = 0; j < graph->num_parents; j++) {
+				if (graph_needs_truncation(graph, i + j)) {
+					if (j > 0)
+						graph_line_addch(line, ' ');
+					graph_line_addstr(line, ". ");
+					truncated = 1;
+					break;
+				}
 				par_column = graph_find_new_column_by_commit(graph, parents->item);
 				assert(par_column >= 0);
 
@@ -1125,9 +1165,13 @@ static void graph_output_post_merge_line(struct git_graph *graph, struct graph_l
 				}
 				parents = next_interesting_parent(graph, parents);
 			}
+			if (truncated)
+				break;
 			if (graph->edges_added == 0)
 				graph_line_addch(line, ' ');
-
+		} else if (graph_needs_truncation(graph, i)) {
+			graph_line_addstr(line, ". ");
+			break;
 		} else if (seen_this) {
 			if (graph->edges_added > 0)
 				graph_line_write_column(line, col, '\\');
@@ -1279,6 +1323,12 @@ static void graph_output_collapsing_line(struct git_graph *graph, struct graph_l
 	 */
 	for (i = 0; i < graph->mapping_size; i++) {
 		int target = graph->mapping[i];
+
+		if (graph_needs_truncation(graph, i / 2)) {
+			graph_line_addstr(line, ". ");
+			break;
+		}
+
 		if (target < 0)
 			graph_line_addch(line, ' ');
 		else if (target * 2 == i)
@@ -1372,6 +1422,11 @@ static void graph_padding_line(struct git_graph *graph, struct strbuf *sb)
 	for (i = 0; i < graph->num_columns; i++) {
 		struct column *col = &graph->columns[i];
 
+		if (graph_needs_truncation(graph, i)) {
+			graph_line_addch(&line, '.');
+			break;
+		}
+
 		graph_line_write_column(&line, col, '|');
 
 		if (col->commit == graph->commit && graph->num_parents > 2) {
diff --git a/graph.h b/graph.h
index 3fd1dcb2e9..9a4551dd29 100644
--- a/graph.h
+++ b/graph.h
@@ -262,4 +262,6 @@ void graph_show_commit_msg(struct git_graph *graph,
 			   FILE *file,
 			   struct strbuf const *sb);
 
+#define MINIMUM_GRAPH_COLUMNS 1
+
 #endif /* GRAPH_H */
diff --git a/revision.c b/revision.c
index 31808e3df0..e8d38cb2a1 100644
--- a/revision.c
+++ b/revision.c
@@ -2605,6 +2605,13 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 	} else if (!strcmp(arg, "--no-graph")) {
 		graph_clear(revs->graph);
 		revs->graph = NULL;
+	} else if ((argcount = parse_long_opt("max-columns", argv, &optarg))) {
+		int val = parse_count(optarg);
+		if (val < MINIMUM_GRAPH_COLUMNS)
+			die(_("minimum columns is %d, cannot be set to %d"),
+			    MINIMUM_GRAPH_COLUMNS, val);
+		revs->graph_max_columns = val;
+		return argcount;
 	} else if (!strcmp(arg, "--encode-email-headers")) {
 		revs->encode_email_headers = 1;
 	} else if (!strcmp(arg, "--no-encode-email-headers")) {
@@ -3172,6 +3179,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 
 	if (revs->no_walk && revs->graph)
 		die(_("options '%s' and '%s' cannot be used together"), "--no-walk", "--graph");
+
+	if (revs->graph_max_columns > 0 && !revs->graph)
+		die(_("option '%s' requires '%s'"), "--max-columns", "--graph");
+
 	if (!revs->reflog_info && revs->grep_filter.use_reflog_filter)
 		die(_("the option '%s' requires '%s'"), "--grep-reflog", "--walk-reflogs");
 
diff --git a/revision.h b/revision.h
index 69242ecb18..f15b390289 100644
--- a/revision.h
+++ b/revision.h
@@ -304,6 +304,7 @@ struct rev_info {
 
 	/* Display history graph */
 	struct git_graph *graph;
+	unsigned int graph_max_columns;
 
 	/* special limits */
 	int skip_count;
diff --git a/t/t4215-log-skewed-merges.sh b/t/t4215-log-skewed-merges.sh
index 28d0779a8c..ca2b224cc4 100755
--- a/t/t4215-log-skewed-merges.sh
+++ b/t/t4215-log-skewed-merges.sh
@@ -370,4 +370,60 @@ test_expect_success 'log --graph with multiple tips' '
 	EOF
 '
 
+test_expect_success 'log --graph --max-columns=2 only two columns' '
+	check_graph --max-columns=2 M_7 <<-\EOF
+	*-.   7_M4
+	|\  .
+	| | * 7_G
+	| | * 7_F
+	| * . 7_E
+	| * . 7_D
+	* | . 7_C
+	| |/
+	|/|
+	* | 7_B
+	|/
+	* 7_A
+	EOF
+'
+
+test_expect_success 'log --graph --max-columns=3 only three columns' '
+	check_graph --max-columns=3 M_1 M_3 M_5 M_7 <<-\EOF
+	*   7_M1
+	|\
+	| | *   7_M2
+	| | | .
+	| | | * 7_H
+	| | | . 7_M3
+	| | | .
+	| | | . 7_J
+	| | | . 7_I
+	| | | . 7_M4
+	| |_|_.
+	|/| | .
+	| | |_.
+	| |/|_.
+	| |/|_.
+	| |/| .
+	| | |/.
+	| | * . 7_G
+	| | | .
+	| | |/.
+	| | |/.
+	| | * . 7_F
+	| * | . 7_E
+	| | |/.
+	| |/| .
+	| * | . 7_D
+	| | |/
+	| |/|
+	* | | 7_C
+	| |/
+	|/|
+	* | 7_B
+	|/
+	* 7_A
+	EOF
+'
+
 test_done
-- 
2.43.0

Re: [GSoC RFC PATCH v2] graph: add --max-columns option to limit displayed columns

[Junio C Hamano]

Pablo Sabater <pabloosabaterr@gmail.com> writes:

> Repositories that have many active branches produce very wide
> outputs with 'git log --graph --all', makes it difficult to read.

"making it difficult"?

> Add '--max-columns=<n>' to limit the columns shown. Columns over
> the limit are replaced with a '.' truncation indicator. This only
> affects the visual rendering.

Being an option to "git log", I expect that readers would naturally
take "--max-columns=<n>" to refer to the total display width
consumed by the entire output including the log messages and
possibly patches when the command is run with the "-p" option,
whether the ancestry graph is shown or not.

But somehow I suspect that it is not what is going on here.

If this <n> refers to the width of graph part only, the option name
should hint that fact somehow.  Perhaps include the word "graph" in
it, or something.  As you use the verb "limit" below, perhaps
"--limit-graph-columns=<n>"?

> The commit mark '*' is only shown in two cases:
> - The commit is in a branch inside the limit.
> - The commit is in the first hidden branch, in this case '.'
>   is replaced by '*'.

> Commits on deeper hidden branches do not show '*' but the commit
> subject is still shown, so no information is lost.

Isn't "no information is lost" a huge exaggeration?  We are losing
the ancestry information by not drawing graph lines, aren't we?

> The original idea to limit columns was noted as a TODO in
> c12172d2ea (Add history graph API, 2008-05-04), which mentions
> gitk's behavior. This does not implement gitk-style column
> rearrangement; it only truncates the visual output.

True.  

IIRC, Gitk also allows you to click the chopped arrow-head to jump
to the other end of the omitted ancestry line, which is very useful
but is hard to do on a terminal output that is not interactive.

> git log --graph --all --oneline --max-columns=2
> *   b81d099 (M_1) 7_M1
> |\
> | | * 2d96ba0 (M_3) 7_M2
> | | .

I would call this output consuming 5 columns for graph part, not 2.
For end users, being able to specify 2, i.e., being able to say "I
tolerate wasting display columns to show up to two ancestry lines"
(or "two lanes of ancestry information"), is indeed a lot more
intuitive than having to say "You are allowed to use up to 5 display
columns", so I do not object to an option that takes "2" as its
value and produces the above output, but I am not sure if we want to
have "columns" in the name of such an option; "--limit-graph-lanes=2"?

> - I think --max-columns is a good name, but it does not have --graph
>   prefix, bcs it can only be on --graph I think it's clear enough.

If "git log --max-columns=77" ignores the option because "--graph"
is not given, it would be confusing to the users.

> +	/*
> +	 * If graph_max_columns is set, cap the padding from the branches
> +	 */
> +	if (graph->revs->graph_max_columns > 0) {
> +		int truncation = graph->revs->graph_max_columns * 2 + 2;

This needs a bit more commenting to explain where these magic
numbers come from; they are of the same value 2 but have different
meanings, right?  Like this (only to illustrate the shape, not
suggesting what the contents should read):

		/*
		 * Each ancestry "lane" occupies 2 columns, and
		 * we leave two columns before drawing the commit
		 * title and log message part.
		 */
		int max_column_width = 
			graph->revs->graph_limit_lanes * 2 + 2;

> +
> +	if (revs->graph_max_columns > 0 && !revs->graph)
> +		die(_("option '%s' requires '%s'"), "--max-columns", "--graph");

The naming is so selfish.  Among "git log" options that exists and
that will be added in the future, this design decision declares that
"--graph" is and will remain to be the only one that may want to
specify the maximum number of columns to spend.

If the option is named clearly to be related to the "--graph"
feature, another way to go is to make it imply "--graph".  If the
user says "I want to limit the graph output to consume no more than
10 leftmost columns", it is clear that the user expects the graph to
be shown.

Re: [GSoC RFC PATCH v2] graph: add --max-columns option to limit displayed columns

[Pablo]

Thanks for the feedback!

Junio C Hamano (<gitster@pobox.com>) writes:

> "making it difficult"?

I'll correct the typo

> If this <n> refers to the width of graph part only, the option name
> should hint that fact somehow.  Perhaps include the word "graph" in
> it, or something.  As you use the verb "limit" below, perhaps
> "--limit-graph-columns=<n>"?

> I would call this output consuming 5 columns for graph part, not 2.
> For end users, being able to specify 2, i.e., being able to say "I
> tolerate wasting display columns to show up to two ancestry lines"
> (or "two lanes of ancestry information"), is indeed a lot more
> intuitive than having to say "You are allowed to use up to 5 display
> columns", so I do not object to an option that takes "2" as its
> value and produces the above output, but I am not sure if we want to
> have "columns" in the name of such an option; "--limit-graph-lanes=2"?

Yes, it only affects the graph rendering, I'll change the name to be
more clear about it is for the graph only and that what is being
modified are the lanes, "|" + " " not actual single char columns.
What about --graph-limit-lanes and make it imply --graph as you said
if it's clear enough?

> Isn't "no information is lost" a huge exaggeration?  We are losing
> the ancestry information by not drawing graph lines, aren't we?

Yeah, I meant that most of the commit information is not lost, only
where they come from if it's a deeper hidden lane. But what I wanted
to say is that internally I'm not removing any information about the
graph and that every commit will show no matter the lane limit.

> IIRC, Gitk also allows you to click the chopped arrow-head to jump
> to the other end of the omitted ancestry line, which is very useful
> but is hard to do on a terminal output that is not interactive.

Yes, that's why it's been sooo long without being done prob. But I
think this approach is still useful, it can be extended later for more
customization like limiting the lines from the left side or ranges.
For lane rearrangement I would need to study graph.c more and look
forward to refactoring a lot in multiple patches.

> If "git log --max-columns=77" ignores the option because "--graph"
> is not given, it would be confusing to the users.

With the check on 'setup_revision()' users won't be able to
'max-columns=77' if there's no '--graph'

> This needs a bit more commenting to explain where these magic
> numbers come from; they are of the same value 2 but have different
> meanings, right?  Like this (only to illustrate the shape, not
> suggesting what the contents should read):
>
>                 /*
>                  * Each ancestry "lane" occupies 2 columns, and
>                  * we leave two columns before drawing the commit
>                  * title and log message part.
>                  */
>                 int max_column_width =
>                         graph->revs->graph_limit_lanes * 2 + 2;

the magic numbers are because, a lane is two columns '|' + ' ', and
the +2 comes from the truncation mark '.' + ' ', so for a 3 lanes
limit, the padding from the graph should be 3 * 2 + 2 = 8. I'll add a
comment to doc the magic numbers.

> > +
> > +     if (revs->graph_max_columns > 0 && !revs->graph)
> > +             die(_("option '%s' requires '%s'"), "--max-columns", "--graph");
>
> The naming is so selfish.  Among "git log" options that exists and
> that will be added in the future, this design decision declares that
> "--graph" is and will remain to be the only one that may want to
> specify the maximum number of columns to spend.
>
> If the option is named clearly to be related to the "--graph"
> feature, another way to go is to make it imply "--graph".  If the
> user says "I want to limit the graph output to consume no more than
> 10 leftmost columns", it is clear that the user expects the graph to
> be shown.

I'll make the name clearer about what it does and less selfish. I like
the idea about --graph-limit-lanes to imply --graph directly and not
force it to be explicit.
Making --graph-limit-lanes imply --graph removes the check at revision_setup()

I'll send a v3

Re: [GSoC RFC PATCH v2] graph: add --max-columns option to limit displayed columns

[Johannes Sixt]

Am 18.03.26 um 19:20 schrieb Pablo:
> Junio C Hamano (<gitster@pobox.com>) writes:>> If the option is named clearly to be related to the "--graph"
>> feature, another way to go is to make it imply "--graph".  If the
>> user says "I want to limit the graph output to consume no more than
>> 10 leftmost columns", it is clear that the user expects the graph to
>> be shown.
> 
> I'll make the name clearer about what it does and less selfish. I like
> the idea about --graph-limit-lanes to imply --graph directly and not
> force it to be explicit.
Don't let this option imply --graph. It specifies a parameter that could
also reasonably be specified via a configuration. But we don't want that
the existence of the hypothetical configuration implies --graph.

How about --edge-limit or --lane-limit?

-- Hannes

[GSoC PATCH WIP RFC v3 0/3] graph: add --graph-lane-limit option

[Pablo Sabater]

When viewing the history of a repository that has many branches, the graph output
can become very wide easily, making it difficult to read and sometimes pushing the 
commit messages to the right and breaking the format. 

Introduce --graph-lane-limit option to truncate the graph at n + 1/2 lanes (n lanes
plus the column between the lane and the truncation mark). Lanes over the limit
are replaced with '.' keeping the graph readable and shows all the information
about the visible lanes.

commit mark '*' is shown in two cases:

- When the commit lives on a visible lane.
- When the commit lives on the first hidden lane (n+1) this case instead of
  '.' it will show '*' to show that there is a commit in the first hidden lane.

Any commit on deeper branches won't show any mark, but the commit subject will 
still be shown.

Coming from graph_output_commit_line, merge-lanes with no relevant information
on the visible lanes (neither commit or parent lives on a visible lane) are
skipped making the graph more compact.

The original idea to limit columns was noted as a TODO in
c12172d2ea (Add history graph API, 2008-05-04), which mentions
gitk's behavior. This does not implement gitk-style column
rearrangement; it only truncates the visual output.

RFC and WIP:

1. On lane-limited graphs, there are collapsing, merge and padding lanes that don't
   add any information to the graph.

     - graph_output_collapsing_line mixes state handling and graph rendering, skipping
       this function entirely, as is done with post-merge, would corrupt the mapping
       so the function needs to be run but returning an empty buffer is not expected
       by the callers and produces blank lanes.

     - Suppressing redundant lanes like merges or collapses that come from graph_next_line().
       graph_show_remainder() and graph_show_commit_msg() emits newlines hoping that
       all lines will have content to print, returning an empty buffer to them
       results in blank lines because of newlines being duplicated. graph_show_remainder
       would be called from two places with different newlines needs, to make
       this work graph_show_remainder() needs to be refactored. Doing this will result
       in shorter graphs with less redundant lines on lane-limited graphs. Given that
       the graph.c code is 17 years old is this desired ?

2. To keep the most information rendered on lane-limited graphs, collapsing and
   merges are rendered as usual but when the collapse or merge to hidden lanes, 
   the information about from where and to where it goes it lost, are they still
   useful ? if not, it could be desirable to remove them like in point 1.

Pablo Sabater (3):
  graph: add --graph-lane-limit option
  graph: truncate graph visual output
  graph: add documentation and testing about --graph-lane-limit

 Documentation/rev-list-options.adoc |   5 ++
 graph.c                             | 130 ++++++++++++++++++++++++----
 graph.h                             |   2 +
 revision.c                          |  11 +++
 revision.h                          |   1 +
 t/t4215-log-skewed-merges.sh        |  53 ++++++++++++
 6 files changed, 185 insertions(+), 17 deletions(-)


base-commit: dc6ecd5354dca88d51b6d6562777fc8fc10d77e1
-- 
2.43.0

[GSoC PATCH WIP RFC v3 1/3] graph: add --graph-lane-limit option

[Pablo Sabater]

Repositories that have many active branches produce very wide outputs
with 'git log --graph --all' making it difficult to read.

Add MINIMUM_GRAPH_COLUMNS = 1

Add '--graph-lane-limit=<n>' to the revision options, this option
needs --graph explicitly and rejects values under MINIMUM_GRAPH_COLUMNS.

Add graph_max_lanes to rev_info and store what the user set

Add graph_needs_truncation() and teach it to know when a column is
over the limit following graph_max_lanes, if the limit is 0, treat it like
no limit.

Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
 graph.c    |  6 ++++++
 graph.h    |  2 ++
 revision.c | 11 +++++++++++
 revision.h |  1 +
 4 files changed, 20 insertions(+)

diff --git a/graph.c b/graph.c
index 26f6fbf000..a95c0a9a73 100644
--- a/graph.c
+++ b/graph.c
@@ -317,6 +317,12 @@ struct git_graph {
 	struct strbuf prefix_buf;
 };
 
+static int graph_needs_truncation(struct git_graph *graph, int col)
+{
+	int max = graph->revs->graph_max_lanes;
+	return max > 0 && col >= max;
+}
+
 static const char *diff_output_prefix_callback(struct diff_options *opt, void *data)
 {
 	struct git_graph *graph = data;
diff --git a/graph.h b/graph.h
index 3fd1dcb2e9..9a4551dd29 100644
--- a/graph.h
+++ b/graph.h
@@ -262,4 +262,6 @@ void graph_show_commit_msg(struct git_graph *graph,
 			   FILE *file,
 			   struct strbuf const *sb);
 
+#define MINIMUM_GRAPH_COLUMNS 1
+
 #endif /* GRAPH_H */
diff --git a/revision.c b/revision.c
index 31808e3df0..aeddf2d166 100644
--- a/revision.c
+++ b/revision.c
@@ -2605,6 +2605,13 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 	} else if (!strcmp(arg, "--no-graph")) {
 		graph_clear(revs->graph);
 		revs->graph = NULL;
+	} else if ((argcount = parse_long_opt("graph-lane-limit", argv, &optarg))) {
+		int max_lanes = parse_count(optarg);
+		if (max_lanes < MINIMUM_GRAPH_COLUMNS)
+			die(_("minimum lanes is %d, cannot be set to %d"),
+			    MINIMUM_GRAPH_COLUMNS, max_lanes);
+		revs->graph_max_lanes = max_lanes;
+		return argcount;
 	} else if (!strcmp(arg, "--encode-email-headers")) {
 		revs->encode_email_headers = 1;
 	} else if (!strcmp(arg, "--no-encode-email-headers")) {
@@ -3172,6 +3179,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 
 	if (revs->no_walk && revs->graph)
 		die(_("options '%s' and '%s' cannot be used together"), "--no-walk", "--graph");
+
+	if (revs->graph_max_lanes > 0 && !revs->graph)
+		die(_("option '%s' requires '%s'"), "--graph-lane-limit", "--graph");
+
 	if (!revs->reflog_info && revs->grep_filter.use_reflog_filter)
 		die(_("the option '%s' requires '%s'"), "--grep-reflog", "--walk-reflogs");
 
diff --git a/revision.h b/revision.h
index 69242ecb18..597116f885 100644
--- a/revision.h
+++ b/revision.h
@@ -304,6 +304,7 @@ struct rev_info {
 
 	/* Display history graph */
 	struct git_graph *graph;
+	unsigned int graph_max_lanes;
 
 	/* special limits */
 	int skip_count;
-- 
2.43.0

[GSoC PATCH WIP RFC v3 2/3] graph: truncate graph visual output

[Pablo Sabater]

Teach graph statuses to stop rendering and add the truncation mark
'.' once they are writing over the lane limit, following
graph_needs_truncation().

Teach graph_output_commit_line to skip the POST_MERGE status when
neither the commit nor the parent is on a visible lane but keep it
when either commit or parent lives in a visible lane.

Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
 graph.c | 124 ++++++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 107 insertions(+), 17 deletions(-)

diff --git a/graph.c b/graph.c
index a95c0a9a73..ab0a008af5 100644
--- a/graph.c
+++ b/graph.c
@@ -702,6 +702,20 @@ static void graph_update_columns(struct git_graph *graph)
 		}
 	}
 
+	/*
+	 * If graph_max_lanes is set, cap the padding from the branches
+	 */
+	if (graph->revs->graph_max_lanes > 0) {
+		/*
+		 * Get the maximum width by multiplying the maximum number of
+		 * lanes by the size of the lane "| " and adds the truncation
+		 * mark ". "
+		 */
+		int max_columns_width = graph->revs->graph_max_lanes * 2 + 2;
+		if (graph->width > max_columns_width)
+			graph->width = max_columns_width;
+	}
+
 	/*
 	 * Shrink mapping_size to be the minimum necessary
 	 */
@@ -852,6 +866,10 @@ static void graph_output_padding_line(struct git_graph *graph,
 	 * Output a padding row, that leaves all branch lines unchanged
 	 */
 	for (i = 0; i < graph->num_new_columns; i++) {
+		if (graph_needs_truncation(graph, i)) {
+			graph_line_addstr(line, ". ");
+			break;
+		}
 		graph_line_write_column(line, &graph->new_columns[i], '|');
 		graph_line_addch(line, ' ');
 	}
@@ -909,6 +927,9 @@ static void graph_output_pre_commit_line(struct git_graph *graph,
 			seen_this = 1;
 			graph_line_write_column(line, col, '|');
 			graph_line_addchars(line, ' ', graph->expansion_row);
+		} else if (seen_this && graph_needs_truncation(graph, i)) {
+			graph_line_addstr(line, ". ");
+			break;
 		} else if (seen_this && (graph->expansion_row == 0)) {
 			/*
 			 * This is the first line of the pre-commit output.
@@ -1019,6 +1040,7 @@ static void graph_output_commit_line(struct git_graph *graph, struct graph_line
 	 * children that we have already processed.)
 	 */
 	seen_this = 0;
+
 	for (i = 0; i <= graph->num_columns; i++) {
 		struct column *col = &graph->columns[i];
 		struct commit *col_commit;
@@ -1034,8 +1056,17 @@ static void graph_output_commit_line(struct git_graph *graph, struct graph_line
 			seen_this = 1;
 			graph_output_commit_char(graph, line);
 
+			if (graph_needs_truncation(graph, i)) {
+				graph_line_addch(line, ' ');
+				break;
+			}
+
 			if (graph->num_parents > 2)
 				graph_draw_octopus_merge(graph, line);
+		} else if (graph_needs_truncation(graph, i)) {
+			graph_line_addstr(line, ". ");
+			seen_this = 1;
+			break;
 		} else if (seen_this && (graph->edges_added > 1)) {
 			graph_line_write_column(line, col, '\\');
 		} else if (seen_this && (graph->edges_added == 1)) {
@@ -1071,10 +1102,32 @@ static void graph_output_commit_line(struct git_graph *graph, struct graph_line
 
 	/*
 	 * Update graph->state
-	 */
-	if (graph->num_parents > 1)
-		graph_update_state(graph, GRAPH_POST_MERGE);
-	else if (graph_is_mapping_correct(graph))
+	 *
+	 * If the commit is a merge and the first parent is in a visible lane,
+	 * then the GRAPH_POST_MERGE is needed to draw the merge lane.
+	 * 
+	 * If the commit is over the truncation limit, but the first parent is on
+	 * a visible lane, then we still need the merge lane but truncated.
+	 * 
+	 * If both commit and first parent are over the truncation limit, then
+	 * there's no need to draw the merge lane because it would work as a
+	 * padding lane.
+	 */
+	if (graph->num_parents > 1) {
+		if (!graph_needs_truncation(graph, graph->commit_index)) {
+			graph_update_state(graph, GRAPH_POST_MERGE);
+		} else {
+			struct commit_list *first_parent = first_interesting_parent(graph);
+			int first_parent_col = graph_find_new_column_by_commit(graph, first_parent->item);
+			
+			if (!graph_needs_truncation(graph, first_parent_col))
+				graph_update_state(graph, GRAPH_POST_MERGE);
+			else if (graph_is_mapping_correct(graph))
+				graph_update_state(graph, GRAPH_PADDING);
+			else
+				graph_update_state(graph, GRAPH_COLLAPSING);
+		}
+	} else if (graph_is_mapping_correct(graph))
 		graph_update_state(graph, GRAPH_PADDING);
 	else
 		graph_update_state(graph, GRAPH_COLLAPSING);
@@ -1115,14 +1168,28 @@ static void graph_output_post_merge_line(struct git_graph *graph, struct graph_l
 			int par_column;
 			int idx = graph->merge_layout;
 			char c;
+			int truncated = 0;
 			seen_this = 1;
 
 			for (j = 0; j < graph->num_parents; j++) {
+				unsigned int truncation_max = i + (j > 1 ? j - 1 : 0);
 				par_column = graph_find_new_column_by_commit(graph, parents->item);
 				assert(par_column >= 0);
 
 				c = merge_chars[idx];
 				graph_line_write_column(line, &graph->new_columns[par_column], c);
+
+				if (j >= 2)
+					truncation_max -= 1;
+
+				if (graph_needs_truncation(graph, truncation_max)) {
+					if (j > 0 && !(graph->edges_added > 0))
+						graph_line_addch(line, ' ');
+					graph_line_addstr(line, ". ");
+					truncated = 1;
+					break;
+				}
+
 				if (idx == 2) {
 					if (graph->edges_added > 0 || j < graph->num_parents - 1)
 						graph_line_addch(line, ' ');
@@ -1131,15 +1198,24 @@ static void graph_output_post_merge_line(struct git_graph *graph, struct graph_l
 				}
 				parents = next_interesting_parent(graph, parents);
 			}
+			if (truncated)
+				break;
 			if (graph->edges_added == 0)
 				graph_line_addch(line, ' ');
-
+		} else if (graph_needs_truncation(graph, i)) {
+			graph_line_addstr(line, ". ");
+			break;
 		} else if (seen_this) {
 			if (graph->edges_added > 0)
 				graph_line_write_column(line, col, '\\');
 			else
 				graph_line_write_column(line, col, '|');
-			graph_line_addch(line, ' ');
+			/*
+			 * If it's between two lanes and next would be truncated,
+			 * don't add space padding.
+			 */
+			if (!graph_needs_truncation(graph, i + 1)) 
+				graph_line_addch(line, ' ');
 		} else {
 			graph_line_write_column(line, col, '|');
 			if (graph->merge_layout != 0 || i != graph->commit_index - 1) {
@@ -1170,6 +1246,7 @@ static void graph_output_collapsing_line(struct git_graph *graph, struct graph_l
 	short used_horizontal = 0;
 	int horizontal_edge = -1;
 	int horizontal_edge_target = -1;
+	int truncated = 0;
 
 	/*
 	 * Swap the mapping and old_mapping arrays
@@ -1285,12 +1362,20 @@ static void graph_output_collapsing_line(struct git_graph *graph, struct graph_l
 	 */
 	for (i = 0; i < graph->mapping_size; i++) {
 		int target = graph->mapping[i];
-		if (target < 0)
-			graph_line_addch(line, ' ');
-		else if (target * 2 == i)
-			graph_line_write_column(line, &graph->new_columns[target], '|');
-		else if (target == horizontal_edge_target &&
-			 i != horizontal_edge - 1) {
+
+		if (!truncated && graph_needs_truncation(graph, i / 2)) {
+			graph_line_addstr(line, ". ");
+			truncated = 1;
+		}
+
+		if (target < 0) {
+			if (!truncated)
+				graph_line_addch(line, ' ');
+		} else if (target * 2 == i) {
+			if (!truncated)
+				graph_line_write_column(line, &graph->new_columns[target], '|');
+		} else if (target == horizontal_edge_target &&
+			   i != horizontal_edge - 1) {
 				/*
 				 * Set the mappings for all but the
 				 * first segment to -1 so that they
@@ -1298,13 +1383,14 @@ static void graph_output_collapsing_line(struct git_graph *graph, struct graph_l
 				 */
 				if (i != (target * 2)+3)
 					graph->mapping[i] = -1;
-				used_horizontal = 1;
-			graph_line_write_column(line, &graph->new_columns[target], '_');
+			used_horizontal = 1;
+			if (!truncated)
+				graph_line_write_column(line, &graph->new_columns[target], '_');
 		} else {
 			if (used_horizontal && i < horizontal_edge)
 				graph->mapping[i] = -1;
-			graph_line_write_column(line, &graph->new_columns[target], '/');
-
+			if (!truncated)
+				graph_line_write_column(line, &graph->new_columns[target], '/');
 		}
 	}
 
@@ -1353,7 +1439,6 @@ int graph_next_line(struct git_graph *graph, struct strbuf *sb)
 		graph_output_collapsing_line(graph, &line);
 		break;
 	}
-
 	graph_pad_horizontally(graph, &line);
 	return shown_commit_line;
 }
@@ -1378,6 +1463,11 @@ static void graph_padding_line(struct git_graph *graph, struct strbuf *sb)
 	for (i = 0; i < graph->num_columns; i++) {
 		struct column *col = &graph->columns[i];
 
+		if (graph_needs_truncation(graph, i)) {
+			graph_line_addch(&line, '.');
+			break;
+		}
+
 		graph_line_write_column(&line, col, '|');
 
 		if (col->commit == graph->commit && graph->num_parents > 2) {
-- 
2.43.0

[GSoC PATCH WIP RFC v3 3/3] graph: add documentation and testing about --graph-lane-limit

[Pablo Sabater]

Document --graph-lane-limit option in rev-list-options.adoc with
--graph option.

Add two tests in t4215 reusing last test graph structure.

- --graph-lane-limit=2 on one tip showing only two rendered
  lanes and the rest replaced with the truncation
  marker.

- --graph-lane-limit=3 with multiple tips, showing only three
  rendered lanes.

Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
 Documentation/rev-list-options.adoc |  5 +++
 t/t4215-log-skewed-merges.sh        | 53 +++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+)

diff --git a/Documentation/rev-list-options.adoc b/Documentation/rev-list-options.adoc
index 2d195a1474..1819228b60 100644
--- a/Documentation/rev-list-options.adoc
+++ b/Documentation/rev-list-options.adoc
@@ -1259,6 +1259,11 @@ This implies the `--topo-order` option by default, but the
 	in between them in that case. If _<barrier>_ is specified, it
 	is the string that will be shown instead of the default one.
 
+`graph-lane-limit=<n>`::
+	When `--graph` is used, limit the number of graph lanes to be shown.
+	Lanes over the limit are replaced with a truncation mark '.'. By default
+	there is no limit.
+
 ifdef::git-rev-list[]
 `--count`::
 	Print a number stating how many commits would have been
diff --git a/t/t4215-log-skewed-merges.sh b/t/t4215-log-skewed-merges.sh
index 28d0779a8c..657e3ff2a5 100755
--- a/t/t4215-log-skewed-merges.sh
+++ b/t/t4215-log-skewed-merges.sh
@@ -370,4 +370,57 @@ test_expect_success 'log --graph with multiple tips' '
 	EOF
 '
 
+test_expect_success 'log --graph --graph-lane-limit=2 limited to two columns' '
+	check_graph --graph-lane-limit=2 M_7 <<-\EOF
+	*-.   7_M4
+	|\ \
+	| | * 7_G
+	| | * 7_F
+	| * . 7_E
+	| * . 7_D
+	* | . 7_C
+	| |/
+	|/|
+	* | 7_B
+	|/
+	* 7_A
+	EOF
+'
+
+test_expect_success 'log --graph --graph-lane-limit=3 limited to three columns' '
+	check_graph --graph-lane-limit=3 M_1 M_3 M_5 M_7 <<-\EOF
+	*   7_M1
+	|\
+	| | *   7_M2
+	| | |\
+	| | | * 7_H
+	| | | . 7_M3
+	| | | . 7_J
+	| | | . 7_I
+	| | | . 7_M4
+	| |_|_.
+	|/| | .
+	| | |_.
+	| |/| .
+	| | | .
+	| | |/.
+	| | * . 7_G
+	| | | .
+	| | |/.
+	| | * . 7_F
+	| * | . 7_E
+	| | |/.
+	| |/| .
+	| * | . 7_D
+	| | |/
+	| |/|
+	* | | 7_C
+	| |/
+	|/|
+	* | 7_B
+	|/
+	* 7_A
+	EOF
+'
+
 test_done
-- 
2.43.0

Re: [GSoC PATCH WIP RFC v3 1/3] graph: add --graph-lane-limit option

[Junio C Hamano]

Pablo Sabater <pabloosabaterr@gmail.com> writes:

> Repositories that have many active branches produce very wide outputs
> with 'git log --graph --all' making it difficult to read.

If you have active branches, whether they are merged to some very
small number of integration branches or they are left updating
without getting merged for a long time, you'd end up getting very
wide output from "log --all --graph"?  Or is this a problem only
when these active branches are merged and having to show merge
commits create the need for wider output?  I cannot quite see which
from the above description.

>
> Add MINIMUM_GRAPH_COLUMNS = 1

This unfinished sentence looks a bit out of place.  It is unclear
how that variable about "columns" related to "lane" that is
introduced in the next paratraph.

> Add '--graph-lane-limit=<n>' to the revision options, this option
> needs --graph explicitly and rejects values under MINIMUM_GRAPH_COLUMNS.

"and rejects values under ..." -> "has to be at least 1".  And the
previous paragraph that consists of a single unfinished sentence can
be discarded.

The implementation detail that you happened to choose a C
proprocessor macro instead of hardcoded constatnt to write the lower
limit is not something readers of the log message needs to know.
They can see that in the "log -p" output easily.  

What is more helpful for readers to know is what you mean by
"graph-lane", what you are counting, and why you want its lower
limit to 1 (instead of 0 or 2).  These reasoning behind the design
is much more important to record to help future developers who want
to fix bugs in this code or who want to extend the feature this code
adds, without violating the underlying assumption and design goals
of the original author (i.e., you).

> Add graph_max_lanes to rev_info and store what the user set
>
> Add graph_needs_truncation() and teach it to know when a column is
> over the limit following graph_max_lanes, if the limit is 0, treat it like
> no limit.
>
> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
> ---
>  graph.c    |  6 ++++++
>  graph.h    |  2 ++
>  revision.c | 11 +++++++++++
>  revision.h |  1 +
>  4 files changed, 20 insertions(+)
>
> diff --git a/graph.c b/graph.c
> index 26f6fbf000..a95c0a9a73 100644
> --- a/graph.c
> +++ b/graph.c
> @@ -317,6 +317,12 @@ struct git_graph {
>  	struct strbuf prefix_buf;
>  };
>  
> +static int graph_needs_truncation(struct git_graph *graph, int col)
> +{
> +	int max = graph->revs->graph_max_lanes;
> +	return max > 0 && col >= max;
> +}
> +
>  static const char *diff_output_prefix_callback(struct diff_options *opt, void *data)
>  {
>  	struct git_graph *graph = data;
> diff --git a/graph.h b/graph.h
> index 3fd1dcb2e9..9a4551dd29 100644
> --- a/graph.h
> +++ b/graph.h
> @@ -262,4 +262,6 @@ void graph_show_commit_msg(struct git_graph *graph,
>  			   FILE *file,
>  			   struct strbuf const *sb);
>  
> +#define MINIMUM_GRAPH_COLUMNS 1
> +
>  #endif /* GRAPH_H */
> diff --git a/revision.c b/revision.c
> index 31808e3df0..aeddf2d166 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -2605,6 +2605,13 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
>  	} else if (!strcmp(arg, "--no-graph")) {
>  		graph_clear(revs->graph);
>  		revs->graph = NULL;
> +	} else if ((argcount = parse_long_opt("graph-lane-limit", argv, &optarg))) {
> +		int max_lanes = parse_count(optarg);
> +		if (max_lanes < MINIMUM_GRAPH_COLUMNS)
> +			die(_("minimum lanes is %d, cannot be set to %d"),
> +			    MINIMUM_GRAPH_COLUMNS, max_lanes);
> +		revs->graph_max_lanes = max_lanes;
> +		return argcount;
>  	} else if (!strcmp(arg, "--encode-email-headers")) {
>  		revs->encode_email_headers = 1;
>  	} else if (!strcmp(arg, "--no-encode-email-headers")) {
> @@ -3172,6 +3179,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
>  
>  	if (revs->no_walk && revs->graph)
>  		die(_("options '%s' and '%s' cannot be used together"), "--no-walk", "--graph");
> +
> +	if (revs->graph_max_lanes > 0 && !revs->graph)
> +		die(_("option '%s' requires '%s'"), "--graph-lane-limit", "--graph");
> +
>  	if (!revs->reflog_info && revs->grep_filter.use_reflog_filter)
>  		die(_("the option '%s' requires '%s'"), "--grep-reflog", "--walk-reflogs");
>  
> diff --git a/revision.h b/revision.h
> index 69242ecb18..597116f885 100644
> --- a/revision.h
> +++ b/revision.h
> @@ -304,6 +304,7 @@ struct rev_info {
>  
>  	/* Display history graph */
>  	struct git_graph *graph;
> +	unsigned int graph_max_lanes;

This is "unsigned int"; don't we want the other places (like the
on-stack local variable handle_revision_opt() uses to parse the
value from the command line) and the parameter used in
graph_needs_truncation() helper function all consistently use the
same type?

>  	/* special limits */
>  	int skip_count;

Re: [GSoC PATCH WIP RFC v3 1/3] graph: add --graph-lane-limit option

[Pablo]

Junio C Hamano (<gitster@pobox.com>) writes:

> If you have active branches, whether they are merged to some very
> small number of integration branches or they are left updating
> without getting merged for a long time, you'd end up getting very
> wide output from "log --all --graph"?  Or is this a problem only
> when these active branches are merged and having to show merge
> commits create the need for wider output?  I cannot quite see which
> from the above description.

The graph gets wide based on the branches active at the same time,
each one occupies two columns, merges don't create new lanes.
I'll improve the problem description.

> The implementation detail that you happened to choose a C
> proprocessor macro instead of hardcoded constatnt to write the lower
> limit is not something readers of the log message needs to know.
> They can see that in the "log -p" output easily.
>
> What is more helpful for readers to know is what you mean by
> "graph-lane", what you are counting, and why you want its lower
> limit to 1 (instead of 0 or 2).  These reasoning behind the design
> is much more important to record to help future developers who want
> to fix bugs in this code or who want to extend the feature this code
> adds, without violating the underlying assumption and design goals
> of the original author (i.e., you).

Ok, I'll focus more on why, rather than how.
the minimum is set to 1 to have at least 1 visible lane, even though it could
accept 0 it's the same as not using this option, 0 it's treated as no limit
and I found it better to not give the users the option to place it
because no other option
seems to behave like this, so I thought the best would be to force the
input to be >= 1 to be valid.
in v4 I'll make sure that this is clear for others.

> This is "unsigned int"; don't we want the other places (like the
> on-stack local variable handle_revision_opt() uses to parse the
> value from the command line) and the parameter used in
> graph_needs_truncation() helper function all consistently use the
> same type?

I saw that other options like max_count, min/max_parents use int instead
of unsigned int, so yes the most consistent would be to have it as
int also, but it would make no sense to have -1 visible lanes. I thought
it would be a good idea to keep it explicit that it can't be negative.
this examples, max_count, etc were the closest examples I saw, but
parse_count does return an int so i can't cast it to unsigned without
checking if its neg that's why this

> int max_lanes = parse_count(optarg);
> if (max_lanes < MINIMUM_GRAPH_COLUMNS)
>         die(_("minimum lanes is %d, cannot be set to %d"),
>                   MINIMUM_GRAPH_COLUMNS, max_lanes);
> revs->graph_max_lanes = max_lanes;

where it checks if it's < 1, now max_lanes has to be > 1 and it fits in an
unsigned int. But i do understand to keep the consistency and
the coding guidelines, i'll make it an int.

Thanks for the feedback!

[GSoC PATCH v4 0/3] graph: add --graph-lane-limit option

[Pablo Sabater]

Repositories that have many active branches at the same time produce
wide graphs. A lane consists of two columns, the edge and the space
padding, each branch takes a lane in the graph and there is no way
to limit how many can be shown.

Add '--graph-lane-limit=<n>' option that caps the visible lanes to n. Lanes over
this limit are replaced with '.' truncation mark.

The '*' commit mark is visible when it lives on a visible lane or the first
hidden lane, any deeper lane doesn't show the commit mark but keeps the commit
message visible. 

Merges where neither the commit nor its parents live on a visible lane are skipped
because they dont carry any visible information.

The original idea to limit columns was noted as a TODO in c12172d2ea
(Add history graph API, 2008-05-04).  This does not implement
gitk-style column rearrangement, it only truncates the visual output.

Possible future improvements:

- When all branches involved in collapsing or padding are over the limit, the
  truncated lane doesn't show any information, this lane could be removed to
  make the graph more compact. Currently these lanes still appear because
  graph_output_collapsing_line() mixes state handling with rendering, so it can't
  be skipped but callers always expect a non empty buffer. Fixing it would need
  to refactor the callers to handle empty buffers instead of expecting them to
  always have content.

- Collapsing and merges lanes that start on visible lanes but end on hidden ones
  are kept to maintain the most information possible on the visible lanes, but
  the information about where they go is lost. They can be kept, removed or think
  of a way to show that information without showing the lanes.

Changes since v3:

- Rewrote commit messages to focus on reasoning rather than the
  implementation details.
- Changed graph_max_lanes to int for consistency with other
  rev_info fields like max_count.
- Zero and negative values are now accepted and silently treated
  as "no limit", following what --max-parents does with negative values.
- Removed the MINIMUM_GRAPH_COLUMNS macro.
- Fixed missing "--" prefix in the documentation.

Pablo Sabater (3):
  graph: add --graph-lane-limit option
  graph: truncate graph visual output
  graph: add documentation and tests about --graph-lane-limit

 Documentation/rev-list-options.adoc |   5 ++
 graph.c                             | 131 ++++++++++++++++++++++++----
 revision.c                          |   6 ++
 revision.h                          |   1 +
 t/t4215-log-skewed-merges.sh        |  53 +++++++++++
 5 files changed, 180 insertions(+), 16 deletions(-)


base-commit: dc6ecd5354dca88d51b6d6562777fc8fc10d77e1
-- 
2.43.0

[GSoC PATCH v4 1/3] graph: add --graph-lane-limit option

[Pablo Sabater]

Repositories that have many active branches at the same time produce
wide graphs. A lane consists of two columns, the edge and the space
padding, each branch takes a lane in the graph and there is no way
to limit how many can be shown.

Add '--graph-lane-limit=<n>' revision option that caps the number
of visible lanes to n. This option requires '--graph', without it
a limit to the graph has no meaning, in this case error out.

Zero and negative values are valid inputs but silently ignored
treating them as "no limit", the same as not using the option.
This follows what '--max-parents' does with negative values.

Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
 graph.c    | 9 +++++++++
 revision.c | 6 ++++++
 revision.h | 1 +
 3 files changed, 16 insertions(+)

diff --git a/graph.c b/graph.c
index 26f6fbf000..e7c1151ac0 100644
--- a/graph.c
+++ b/graph.c
@@ -317,6 +317,15 @@ struct git_graph {
 	struct strbuf prefix_buf;
 };
 
+static int graph_needs_truncation(struct git_graph *graph, int lane)
+{
+	int max = graph->revs->graph_max_lanes;
+	/*
+	 * Ignore values <= 0, meaning no limit.
+	 */
+	return max > 0 && lane >= max;
+}
+
 static const char *diff_output_prefix_callback(struct diff_options *opt, void *data)
 {
 	struct git_graph *graph = data;
diff --git a/revision.c b/revision.c
index 31808e3df0..952edb031e 100644
--- a/revision.c
+++ b/revision.c
@@ -2605,6 +2605,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 	} else if (!strcmp(arg, "--no-graph")) {
 		graph_clear(revs->graph);
 		revs->graph = NULL;
+	} else if (skip_prefix(arg, "--graph-lane-limit=", &optarg)) {
+		revs->graph_max_lanes = parse_count(optarg);
 	} else if (!strcmp(arg, "--encode-email-headers")) {
 		revs->encode_email_headers = 1;
 	} else if (!strcmp(arg, "--no-encode-email-headers")) {
@@ -3172,6 +3174,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 
 	if (revs->no_walk && revs->graph)
 		die(_("options '%s' and '%s' cannot be used together"), "--no-walk", "--graph");
+
+	if (revs->graph_max_lanes > 0 && !revs->graph)
+		die(_("option '%s' requires '%s'"), "--graph-lane-limit", "--graph");
+
 	if (!revs->reflog_info && revs->grep_filter.use_reflog_filter)
 		die(_("the option '%s' requires '%s'"), "--grep-reflog", "--walk-reflogs");
 
diff --git a/revision.h b/revision.h
index 69242ecb18..874ccce625 100644
--- a/revision.h
+++ b/revision.h
@@ -304,6 +304,7 @@ struct rev_info {
 
 	/* Display history graph */
 	struct git_graph *graph;
+	int graph_max_lanes;
 
 	/* special limits */
 	int skip_count;
-- 
2.43.0

Re: [GSoC PATCH v4 1/3] graph: add --graph-lane-limit option

[SZEDER Gábor]

On Mon, Mar 23, 2026 at 10:59:33PM +0100, Pablo Sabater wrote:
> Repositories that have many active branches at the same time produce
> wide graphs. A lane consists of two columns, the edge and the space
> padding, each branch takes a lane in the graph and there is no way
> to limit how many can be shown.
> 
> Add '--graph-lane-limit=<n>' revision option that caps the number
> of visible lanes to n. This option requires '--graph', without it
> a limit to the graph has no meaning, in this case error out.
> 
> Zero and negative values are valid inputs but silently ignored
> treating them as "no limit", the same as not using the option.
> This follows what '--max-parents' does with negative values.
> 
> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
> ---
>  graph.c    | 9 +++++++++
>  revision.c | 6 ++++++
>  revision.h | 1 +
>  3 files changed, 16 insertions(+)
> 
> diff --git a/graph.c b/graph.c
> index 26f6fbf000..e7c1151ac0 100644
> --- a/graph.c
> +++ b/graph.c
> @@ -317,6 +317,15 @@ struct git_graph {
>  	struct strbuf prefix_buf;
>  };
>  
> +static int graph_needs_truncation(struct git_graph *graph, int lane)
> +{
> +	int max = graph->revs->graph_max_lanes;
> +	/*
> +	 * Ignore values <= 0, meaning no limit.
> +	 */
> +	return max > 0 && lane >= max;
> +}

This patch adds this static function, but it doesn't add any callers.
This breaks the build with DEVELOPER=1:

  $ make DEVELOPER=1 graph.o
      CC graph.o
  graph.c:320:12: error: ‘graph_needs_truncation’ defined but not used [-Werror=unused-function]
    320 | static int graph_needs_truncation(struct git_graph *graph, int lane)
        |            ^~~~~~~~~~~~~~~~~~~~~~
  cc1: all warnings being treated as errors
  make: *** [Makefile:2923: graph.o] Error 1

>  static const char *diff_output_prefix_callback(struct diff_options *opt, void *data)
>  {
>  	struct git_graph *graph = data;
> diff --git a/revision.c b/revision.c
> index 31808e3df0..952edb031e 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -2605,6 +2605,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
>  	} else if (!strcmp(arg, "--no-graph")) {
>  		graph_clear(revs->graph);
>  		revs->graph = NULL;
> +	} else if (skip_prefix(arg, "--graph-lane-limit=", &optarg)) {
> +		revs->graph_max_lanes = parse_count(optarg);
>  	} else if (!strcmp(arg, "--encode-email-headers")) {
>  		revs->encode_email_headers = 1;
>  	} else if (!strcmp(arg, "--no-encode-email-headers")) {
> @@ -3172,6 +3174,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
>  
>  	if (revs->no_walk && revs->graph)
>  		die(_("options '%s' and '%s' cannot be used together"), "--no-walk", "--graph");
> +
> +	if (revs->graph_max_lanes > 0 && !revs->graph)
> +		die(_("option '%s' requires '%s'"), "--graph-lane-limit", "--graph");
> +
>  	if (!revs->reflog_info && revs->grep_filter.use_reflog_filter)
>  		die(_("the option '%s' requires '%s'"), "--grep-reflog", "--walk-reflogs");
>  
> diff --git a/revision.h b/revision.h
> index 69242ecb18..874ccce625 100644
> --- a/revision.h
> +++ b/revision.h
> @@ -304,6 +304,7 @@ struct rev_info {
>  
>  	/* Display history graph */
>  	struct git_graph *graph;
> +	int graph_max_lanes;
>  
>  	/* special limits */
>  	int skip_count;
> -- 
> 2.43.0
> 

Re: [GSoC PATCH v4 1/3] graph: add --graph-lane-limit option

[Johannes Sixt]

Am 23.03.26 um 22:59 schrieb Pablo Sabater:
> @@ -3172,6 +3174,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
>  
>  	if (revs->no_walk && revs->graph)
>  		die(_("options '%s' and '%s' cannot be used together"), "--no-walk", "--graph");
> +
> +	if (revs->graph_max_lanes > 0 && !revs->graph)
> +		die(_("option '%s' requires '%s'"), "--graph-lane-limit", "--graph");
> +
>  	if (!revs->reflog_info && revs->grep_filter.use_reflog_filter)
>  		die(_("the option '%s' requires '%s'"), "--grep-reflog", "--walk-reflogs");

You help translators if you make the new error message format string
exactly identical to the one that we see in the post-context.

-- Hannes

Re: [GSoC PATCH v4 1/3] graph: add --graph-lane-limit option

[Pablo]

Johannes Sixt (<j6t@kdbg.org>) writes:
>
> Am 23.03.26 um 22:59 schrieb Pablo Sabater:
> > @@ -3172,6 +3174,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
> >
> >       if (revs->no_walk && revs->graph)
> >               die(_("options '%s' and '%s' cannot be used together"), "--no-walk", "--graph");
> > +
> > +     if (revs->graph_max_lanes > 0 && !revs->graph)
> > +             die(_("option '%s' requires '%s'"), "--graph-lane-limit", "--graph");
> > +
> >       if (!revs->reflog_info && revs->grep_filter.use_reflog_filter)
> >               die(_("the option '%s' requires '%s'"), "--grep-reflog", "--walk-reflogs");
>
> You help translators if you make the new error message format string
> exactly identical to the one that we see in the post-context.
>

True, I'll make the messages the same in v5.

> -- Hannes
>

[GSoC PATCH v4 2/3] graph: truncate graph visual output

[Pablo Sabater]

When '--graph-lane-limit' is set, lanes over the limit should
not be drawn. Teach each graph state to stop rendering at the
lane limit and print a '.' truncation mark, so users know that
there are hidden lanes.

On the commit line, if the commit lives on a visible lane, show
the normal commit mark and truncate after it, if the commit lives
on the first hidden lane (the truncation mark lane) show the "*"
instead of the truncation mark so it is known that this commit sits
on the first hidden lane. Commits on deeper lanes don't leave
a mark.

For merges, the post-merge lane is only needed when the commit or
the first parent lives on a visible lane (to draw the connection
between them), when both are on hidden lanes, post-merge carries no
useful information, skip it and go to collapsing or padding state.

Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
 graph.c | 122 ++++++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 106 insertions(+), 16 deletions(-)

diff --git a/graph.c b/graph.c
index e7c1151ac0..886ef7cede 100644
--- a/graph.c
+++ b/graph.c
@@ -705,6 +705,20 @@ static void graph_update_columns(struct git_graph *graph)
 		}
 	}
 
+	/*
+	 * If graph_max_lanes is set, cap the padding from the branches
+	 */
+	if (graph->revs->graph_max_lanes > 0) {
+		/*
+		 * Get the maximum width by multiplying the maximum number of
+		 * lanes by the size of the lane "| " and adds the truncation
+		 * mark ". "
+		 */
+		int max_columns_width = graph->revs->graph_max_lanes * 2 + 2;
+		if (graph->width > max_columns_width)
+			graph->width = max_columns_width;
+	}
+
 	/*
 	 * Shrink mapping_size to be the minimum necessary
 	 */
@@ -855,6 +869,10 @@ static void graph_output_padding_line(struct git_graph *graph,
 	 * Output a padding row, that leaves all branch lines unchanged
 	 */
 	for (i = 0; i < graph->num_new_columns; i++) {
+		if (graph_needs_truncation(graph, i)) {
+			graph_line_addstr(line, ". ");
+			break;
+		}
 		graph_line_write_column(line, &graph->new_columns[i], '|');
 		graph_line_addch(line, ' ');
 	}
@@ -912,6 +930,9 @@ static void graph_output_pre_commit_line(struct git_graph *graph,
 			seen_this = 1;
 			graph_line_write_column(line, col, '|');
 			graph_line_addchars(line, ' ', graph->expansion_row);
+		} else if (seen_this && graph_needs_truncation(graph, i)) {
+			graph_line_addstr(line, ". ");
+			break;
 		} else if (seen_this && (graph->expansion_row == 0)) {
 			/*
 			 * This is the first line of the pre-commit output.
@@ -1022,6 +1043,7 @@ static void graph_output_commit_line(struct git_graph *graph, struct graph_line
 	 * children that we have already processed.)
 	 */
 	seen_this = 0;
+
 	for (i = 0; i <= graph->num_columns; i++) {
 		struct column *col = &graph->columns[i];
 		struct commit *col_commit;
@@ -1037,8 +1059,17 @@ static void graph_output_commit_line(struct git_graph *graph, struct graph_line
 			seen_this = 1;
 			graph_output_commit_char(graph, line);
 
+			if (graph_needs_truncation(graph, i)) {
+				graph_line_addch(line, ' ');
+				break;
+			}
+
 			if (graph->num_parents > 2)
 				graph_draw_octopus_merge(graph, line);
+		} else if (graph_needs_truncation(graph, i)) {
+			graph_line_addstr(line, ". ");
+			seen_this = 1;
+			break;
 		} else if (seen_this && (graph->edges_added > 1)) {
 			graph_line_write_column(line, col, '\\');
 		} else if (seen_this && (graph->edges_added == 1)) {
@@ -1074,10 +1105,32 @@ static void graph_output_commit_line(struct git_graph *graph, struct graph_line
 
 	/*
 	 * Update graph->state
+	 *
+	 * If the commit is a merge and the first parent is in a visible lane,
+	 * then the GRAPH_POST_MERGE is needed to draw the merge lane.
+	 *
+	 * If the commit is over the truncation limit, but the first parent is on
+	 * a visible lane, then we still need the merge lane but truncated.
+	 *
+	 * If both commit and first parent are over the truncation limit, then
+	 * there's no need to draw the merge lane because it would work as a
+	 * padding lane.
 	 */
-	if (graph->num_parents > 1)
-		graph_update_state(graph, GRAPH_POST_MERGE);
-	else if (graph_is_mapping_correct(graph))
+	if (graph->num_parents > 1) {
+		if (!graph_needs_truncation(graph, graph->commit_index)) {
+			graph_update_state(graph, GRAPH_POST_MERGE);
+		} else {
+			struct commit_list *first_parent = first_interesting_parent(graph);
+			int first_parent_col = graph_find_new_column_by_commit(graph, first_parent->item);
+
+			if (!graph_needs_truncation(graph, first_parent_col))
+				graph_update_state(graph, GRAPH_POST_MERGE);
+			else if (graph_is_mapping_correct(graph))
+				graph_update_state(graph, GRAPH_PADDING);
+			else
+				graph_update_state(graph, GRAPH_COLLAPSING);
+		}
+	} else if (graph_is_mapping_correct(graph))
 		graph_update_state(graph, GRAPH_PADDING);
 	else
 		graph_update_state(graph, GRAPH_COLLAPSING);
@@ -1118,14 +1171,28 @@ static void graph_output_post_merge_line(struct git_graph *graph, struct graph_l
 			int par_column;
 			int idx = graph->merge_layout;
 			char c;
+			int truncated = 0;
 			seen_this = 1;
 
 			for (j = 0; j < graph->num_parents; j++) {
+				unsigned int truncation_max = i + (j > 1 ? j - 1 : 0);
 				par_column = graph_find_new_column_by_commit(graph, parents->item);
 				assert(par_column >= 0);
 
 				c = merge_chars[idx];
 				graph_line_write_column(line, &graph->new_columns[par_column], c);
+
+				if (j >= 2)
+					truncation_max -= 1;
+
+				if (graph_needs_truncation(graph, truncation_max)) {
+					if (j > 0 && !(graph->edges_added > 0))
+						graph_line_addch(line, ' ');
+					graph_line_addstr(line, ". ");
+					truncated = 1;
+					break;
+				}
+
 				if (idx == 2) {
 					if (graph->edges_added > 0 || j < graph->num_parents - 1)
 						graph_line_addch(line, ' ');
@@ -1134,15 +1201,24 @@ static void graph_output_post_merge_line(struct git_graph *graph, struct graph_l
 				}
 				parents = next_interesting_parent(graph, parents);
 			}
+			if (truncated)
+				break;
 			if (graph->edges_added == 0)
 				graph_line_addch(line, ' ');
-
+		} else if (graph_needs_truncation(graph, i)) {
+			graph_line_addstr(line, ". ");
+			break;
 		} else if (seen_this) {
 			if (graph->edges_added > 0)
 				graph_line_write_column(line, col, '\\');
 			else
 				graph_line_write_column(line, col, '|');
-			graph_line_addch(line, ' ');
+			/*
+			 * If it's between two lanes and next would be truncated,
+			 * don't add space padding.
+			 */
+			if (!graph_needs_truncation(graph, i + 1))
+				graph_line_addch(line, ' ');
 		} else {
 			graph_line_write_column(line, col, '|');
 			if (graph->merge_layout != 0 || i != graph->commit_index - 1) {
@@ -1173,6 +1249,7 @@ static void graph_output_collapsing_line(struct git_graph *graph, struct graph_l
 	short used_horizontal = 0;
 	int horizontal_edge = -1;
 	int horizontal_edge_target = -1;
+	int truncated = 0;
 
 	/*
 	 * Swap the mapping and old_mapping arrays
@@ -1288,12 +1365,20 @@ static void graph_output_collapsing_line(struct git_graph *graph, struct graph_l
 	 */
 	for (i = 0; i < graph->mapping_size; i++) {
 		int target = graph->mapping[i];
-		if (target < 0)
-			graph_line_addch(line, ' ');
-		else if (target * 2 == i)
-			graph_line_write_column(line, &graph->new_columns[target], '|');
-		else if (target == horizontal_edge_target &&
-			 i != horizontal_edge - 1) {
+
+		if (!truncated && graph_needs_truncation(graph, i / 2)) {
+			graph_line_addstr(line, ". ");
+			truncated = 1;
+		}
+
+		if (target < 0) {
+			if (!truncated)
+				graph_line_addch(line, ' ');
+		} else if (target * 2 == i) {
+			if (!truncated)
+				graph_line_write_column(line, &graph->new_columns[target], '|');
+		} else if (target == horizontal_edge_target &&
+			   i != horizontal_edge - 1) {
 				/*
 				 * Set the mappings for all but the
 				 * first segment to -1 so that they
@@ -1301,13 +1386,14 @@ static void graph_output_collapsing_line(struct git_graph *graph, struct graph_l
 				 */
 				if (i != (target * 2)+3)
 					graph->mapping[i] = -1;
-				used_horizontal = 1;
-			graph_line_write_column(line, &graph->new_columns[target], '_');
+			used_horizontal = 1;
+			if (!truncated)
+				graph_line_write_column(line, &graph->new_columns[target], '_');
 		} else {
 			if (used_horizontal && i < horizontal_edge)
 				graph->mapping[i] = -1;
-			graph_line_write_column(line, &graph->new_columns[target], '/');
-
+			if (!truncated)
+				graph_line_write_column(line, &graph->new_columns[target], '/');
 		}
 	}
 
@@ -1356,7 +1442,6 @@ int graph_next_line(struct git_graph *graph, struct strbuf *sb)
 		graph_output_collapsing_line(graph, &line);
 		break;
 	}
-
 	graph_pad_horizontally(graph, &line);
 	return shown_commit_line;
 }
@@ -1381,6 +1466,11 @@ static void graph_padding_line(struct git_graph *graph, struct strbuf *sb)
 	for (i = 0; i < graph->num_columns; i++) {
 		struct column *col = &graph->columns[i];
 
+		if (graph_needs_truncation(graph, i)) {
+			graph_line_addch(&line, '.');
+			break;
+		}
+
 		graph_line_write_column(&line, col, '|');
 
 		if (col->commit == graph->commit && graph->num_parents > 2) {
-- 
2.43.0

Re: [GSoC PATCH v4 2/3] graph: truncate graph visual output

[Johannes Sixt]

Am 23.03.26 um 22:59 schrieb Pablo Sabater:
> +	/*
> +	 * If graph_max_lanes is set, cap the padding from the branches
> +	 */
> +	if (graph->revs->graph_max_lanes > 0) {
> +		/*
> +		 * Get the maximum width by multiplying the maximum number of
> +		 * lanes by the size of the lane "| " and adds the truncation
> +		 * mark ". "
> +		 */
> +		int max_columns_width = graph->revs->graph_max_lanes * 2 + 2;

Please be to the point in the code comments. That there is a
multiplication and an addition, we can see in the code. Perhaps:

		/* width of "| " per lane plus truncation mark ". " */

> +		if (graph->width > max_columns_width)
> +			graph->width = max_columns_width;
> +	}
> +
>  	/*
>  	 * Shrink mapping_size to be the minimum necessary
>  	 */

> @@ -1022,6 +1043,7 @@ static void graph_output_commit_line(struct git_graph *graph, struct graph_line
>  	 * children that we have already processed.)
>  	 */
>  	seen_this = 0;
> +
>  	for (i = 0; i <= graph->num_columns; i++) {
>  		struct column *col = &graph->columns[i];
>  		struct commit *col_commit;

Is this empty line really needed?

> +				if (j >= 2)
> +					truncation_max -= 1;

I think it is more idiomatic way to write this as truncation_max--.

> @@ -1288,12 +1365,20 @@ static void graph_output_collapsing_line(struct git_graph *graph, struct graph_l
>  	 */
>  	for (i = 0; i < graph->mapping_size; i++) {
>  		int target = graph->mapping[i];
> -		if (target < 0)
> -			graph_line_addch(line, ' ');
> -		else if (target * 2 == i)
> -			graph_line_write_column(line, &graph->new_columns[target], '|');
> -		else if (target == horizontal_edge_target &&
> -			 i != horizontal_edge - 1) {
> +
> +		if (!truncated && graph_needs_truncation(graph, i / 2)) {
> +			graph_line_addstr(line, ". ");
> +			truncated = 1;
> +		}
> +
> +		if (target < 0) {
> +			if (!truncated)
> +				graph_line_addch(line, ' ');
> +		} else if (target * 2 == i) {
> +			if (!truncated)
> +				graph_line_write_column(line, &graph->new_columns[target], '|');
> +		} else if (target == horizontal_edge_target &&
> +			   i != horizontal_edge - 1) {
>  				/*
>  				 * Set the mappings for all but the
>  				 * first segment to -1 so that they
> @@ -1301,13 +1386,14 @@ static void graph_output_collapsing_line(struct git_graph *graph, struct graph_l
>  				 */
>  				if (i != (target * 2)+3)
>  					graph->mapping[i] = -1;
> -				used_horizontal = 1;
> -			graph_line_write_column(line, &graph->new_columns[target], '_');
> +			used_horizontal = 1;
> +			if (!truncated)
> +				graph_line_write_column(line, &graph->new_columns[target], '_');

Huh? The indentation of "used_horizontal..." changed. The reason is that
this whole if-branch is indented too far by one tab. Perhaps an initial
clean-up commit that only fixes this indentation?

>  		} else {
>  			if (used_horizontal && i < horizontal_edge)
>  				graph->mapping[i] = -1;
> -			graph_line_write_column(line, &graph->new_columns[target], '/');
> -
> +			if (!truncated)
> +				graph_line_write_column(line, &graph->new_columns[target], '/');
>  		}
>  	}
>  
> @@ -1356,7 +1442,6 @@ int graph_next_line(struct git_graph *graph, struct strbuf *sb)
>  		graph_output_collapsing_line(graph, &line);
>  		break;
>  	}
> -
>  	graph_pad_horizontally(graph, &line);
>  	return shown_commit_line;
>  }

This removal of an empty line isn't warranted, I think.

-- Hannes

Re: [GSoC PATCH v4 2/3] graph: truncate graph visual output

[Pablo]

Johannes Sixt (<j6t@kdbg.org>) writes:
>
> Am 23.03.26 um 22:59 schrieb Pablo Sabater:
> > +     /*
> > +      * If graph_max_lanes is set, cap the padding from the branches
> > +      */
> > +     if (graph->revs->graph_max_lanes > 0) {
> > +             /*
> > +              * Get the maximum width by multiplying the maximum number of
> > +              * lanes by the size of the lane "| " and adds the truncation
> > +              * mark ". "
> > +              */
> > +             int max_columns_width = graph->revs->graph_max_lanes * 2 + 2;
>
> Please be to the point in the code comments. That there is a
> multiplication and an addition, we can see in the code. Perhaps:
>
>                 /* width of "| " per lane plus truncation mark ". " */

I'll use this one instead, thanks.

> > +             if (graph->width > max_columns_width)
> > +                     graph->width = max_columns_width;
> > +     }
> > +
> >       /*
> >        * Shrink mapping_size to be the minimum necessary
> >        */
>
> > @@ -1022,6 +1043,7 @@ static void graph_output_commit_line(struct git_graph *graph, struct graph_line
> >        * children that we have already processed.)
> >        */
> >       seen_this = 0;
> > +
> >       for (i = 0; i <= graph->num_columns; i++) {
> >               struct column *col = &graph->columns[i];
> >               struct commit *col_commit;
>
> Is this empty line really needed?

No, I'll drop it.

>
> > +                             if (j >= 2)
> > +                                     truncation_max -= 1;
>
> I think it is more idiomatic way to write this as truncation_max--.

I'll change it.

>
> > @@ -1288,12 +1365,20 @@ static void graph_output_collapsing_line(struct git_graph *graph, struct graph_l
> >        */
> >       for (i = 0; i < graph->mapping_size; i++) {
> >               int target = graph->mapping[i];
> > -             if (target < 0)
> > -                     graph_line_addch(line, ' ');
> > -             else if (target * 2 == i)
> > -                     graph_line_write_column(line, &graph->new_columns[target], '|');
> > -             else if (target == horizontal_edge_target &&
> > -                      i != horizontal_edge - 1) {
> > +
> > +             if (!truncated && graph_needs_truncation(graph, i / 2)) {
> > +                     graph_line_addstr(line, ". ");
> > +                     truncated = 1;
> > +             }
> > +
> > +             if (target < 0) {
> > +                     if (!truncated)
> > +                             graph_line_addch(line, ' ');
> > +             } else if (target * 2 == i) {
> > +                     if (!truncated)
> > +                             graph_line_write_column(line, &graph->new_columns[target], '|');
> > +             } else if (target == horizontal_edge_target &&
> > +                        i != horizontal_edge - 1) {
> >                               /*
> >                                * Set the mappings for all but the
> >                                * first segment to -1 so that they
> > @@ -1301,13 +1386,14 @@ static void graph_output_collapsing_line(struct git_graph *graph, struct graph_l
> >                                */
> >                               if (i != (target * 2)+3)
> >                                       graph->mapping[i] = -1;
> > -                             used_horizontal = 1;
> > -                     graph_line_write_column(line, &graph->new_columns[target], '_');
> > +                     used_horizontal = 1;
> > +                     if (!truncated)
> > +                             graph_line_write_column(line, &graph->new_columns[target], '_');
>
> Huh? The indentation of "used_horizontal..." changed. The reason is that
> this whole if-branch is indented too far by one tab. Perhaps an initial
> clean-up commit that only fixes this indentation?

I'll fix it, but I think it will be better to have it on the patch
with the truncation logic because
it is a tiny cleanup and doesn't really justify a whole commit for it.

> >               } else {
> >                       if (used_horizontal && i < horizontal_edge)
> >                               graph->mapping[i] = -1;
> > -                     graph_line_write_column(line, &graph->new_columns[target], '/');
> > -
> > +                     if (!truncated)
> > +                             graph_line_write_column(line, &graph->new_columns[target], '/');
> >               }
> >       }
> >
> > @@ -1356,7 +1442,6 @@ int graph_next_line(struct git_graph *graph, struct strbuf *sb)
> >               graph_output_collapsing_line(graph, &line);
> >               break;
> >       }
> > -
> >       graph_pad_horizontally(graph, &line);
> >       return shown_commit_line;
> >  }
>
> This removal of an empty line isn't warranted, I think.

No, I'll revert that.

>
> -- Hannes
>

[GSoC PATCH v4 3/3] graph: add documentation and tests about --graph-lane-limit

[Pablo Sabater]

Document --graph-lane-limit option in rev-list-options.adoc with
--graph option.

Add two tests in t4215 reusing existing graphs. The first test
limits to two lanes on a single tip and the second one limits to
three lanes on multiple tips. Both tests check that everything
is truncated correctly.

Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
 Documentation/rev-list-options.adoc |  5 +++
 t/t4215-log-skewed-merges.sh        | 53 +++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+)

diff --git a/Documentation/rev-list-options.adoc b/Documentation/rev-list-options.adoc
index 2d195a1474..2b5a1794cd 100644
--- a/Documentation/rev-list-options.adoc
+++ b/Documentation/rev-list-options.adoc
@@ -1259,6 +1259,11 @@ This implies the `--topo-order` option by default, but the
 	in between them in that case. If _<barrier>_ is specified, it
 	is the string that will be shown instead of the default one.
 
+`--graph-lane-limit=<n>`::
+	When `--graph` is used, limit the number of graph lanes to be shown.
+	Lanes over the limit are replaced with a truncation mark '.'. By default
+	there is no limit.
+
 ifdef::git-rev-list[]
 `--count`::
 	Print a number stating how many commits would have been
diff --git a/t/t4215-log-skewed-merges.sh b/t/t4215-log-skewed-merges.sh
index 28d0779a8c..650701df42 100755
--- a/t/t4215-log-skewed-merges.sh
+++ b/t/t4215-log-skewed-merges.sh
@@ -370,4 +370,57 @@ test_expect_success 'log --graph with multiple tips' '
 	EOF
 '
 
+test_expect_success 'log --graph --graph-lane-limit=2 limited to two lanes' '
+	check_graph --graph-lane-limit=2 M_7 <<-\EOF
+	*-.   7_M4
+	|\ \
+	| | * 7_G
+	| | * 7_F
+	| * . 7_E
+	| * . 7_D
+	* | . 7_C
+	| |/
+	|/|
+	* | 7_B
+	|/
+	* 7_A
+	EOF
+'
+
+test_expect_success 'log --graph --graph-lane-limit=3 limited to three lanes' '
+	check_graph --graph-lane-limit=3 M_1 M_3 M_5 M_7 <<-\EOF
+	*   7_M1
+	|\
+	| | *   7_M2
+	| | |\
+	| | | * 7_H
+	| | | . 7_M3
+	| | | . 7_J
+	| | | . 7_I
+	| | | . 7_M4
+	| |_|_.
+	|/| | .
+	| | |_.
+	| |/| .
+	| | | .
+	| | |/.
+	| | * . 7_G
+	| | | .
+	| | |/.
+	| | * . 7_F
+	| * | . 7_E
+	| | |/.
+	| |/| .
+	| * | . 7_D
+	| | |/
+	| |/|
+	* | | 7_C
+	| |/
+	|/|
+	* | 7_B
+	|/
+	* 7_A
+	EOF
+'
+
 test_done
-- 
2.43.0

Re: [GSoC PATCH v4 3/3] graph: add documentation and tests about --graph-lane-limit

[Johannes Sixt]

Am 23.03.26 um 22:59 schrieb Pablo Sabater:
> @@ -1259,6 +1259,11 @@ This implies the `--topo-order` option by default, but the
>  	in between them in that case. If _<barrier>_ is specified, it
>  	is the string that will be shown instead of the default one.
>  
> +`--graph-lane-limit=<n>`::
> +	When `--graph` is used, limit the number of graph lanes to be shown.
> +	Lanes over the limit are replaced with a truncation mark '.'. By default
> +	there is no limit.

This should probably mention that 0 means no limit.

> +
>  ifdef::git-rev-list[]
>  `--count`::
>  	Print a number stating how many commits would have been
> diff --git a/t/t4215-log-skewed-merges.sh b/t/t4215-log-skewed-merges.sh
> index 28d0779a8c..650701df42 100755
> --- a/t/t4215-log-skewed-merges.sh
> +++ b/t/t4215-log-skewed-merges.sh
> @@ -370,4 +370,57 @@ test_expect_success 'log --graph with multiple tips' '
>  	EOF
>  '
>  
> +test_expect_success 'log --graph --graph-lane-limit=2 limited to two lanes' '
> +	check_graph --graph-lane-limit=2 M_7 <<-\EOF
> +	*-.   7_M4
> +	|\ \
> +	| | * 7_G
> +	| | * 7_F
> +	| * . 7_E
> +	| * . 7_D
> +	* | . 7_C
> +	| |/
> +	|/|
> +	* | 7_B
> +	|/
> +	* 7_A

I'm confused. If the lane limit is 2, why do we have actually have 3 lanes?

> +test_expect_success 'log --graph --graph-lane-limit=3 limited to three lanes' '
> +	check_graph --graph-lane-limit=3 M_1 M_3 M_5 M_7 <<-\EOF
> +	*   7_M1
> +	|\
> +	| | *   7_M2
> +	| | |\
> +	| | | * 7_H
> +	| | | . 7_M3
> +	| | | . 7_J
> +	| | | . 7_I
> +	| | | . 7_M4
> +	| |_|_.
> +	|/| | .
> +	| | |_.
> +	| |/| .
> +	| | | .
> +	| | |/.
> +	| | * . 7_G
> +	| | | .
> +	| | |/.
> +	| | * . 7_F
> +	| * | . 7_E
> +	| | |/.
> +	| |/| .
> +	| * | . 7_D
> +	| | |/
> +	| |/|
> +	* | | 7_C
> +	| |/
> +	|/|
> +	* | 7_B
> +	|/
> +	* 7_A

Same here. Why is there a fourth lane?

Oh! "Truncation" here does not mean that the vertial lines are cut off
and are supposed to continue sometime later in the chart. It literally
means that the *line* is truncated and just some stuff *on that line* is
omitted.

Ouch! That was not what I was expecting. I thought that truncation means
that when the eye follows a line vertically, it finds the truncation
point of the line at some point, and then the continuation of that line
is again some time later down the chart. The only clue which lanes are
the same would be the color, which would have to be remedied somehow.

I don't know what to make of it. I have to reconsider.

-- Hannes

Re: [GSoC PATCH v4 3/3] graph: add documentation and tests about --graph-lane-limit

[Pablo]

Johannes Sixt (<j6t@kdbg.org>) writes:
>
> Am 23.03.26 um 22:59 schrieb Pablo Sabater:
> > @@ -1259,6 +1259,11 @@ This implies the `--topo-order` option by default, but the
> >       in between them in that case. If _<barrier>_ is specified, it
> >       is the string that will be shown instead of the default one.
> >
> > +`--graph-lane-limit=<n>`::
> > +     When `--graph` is used, limit the number of graph lanes to be shown.
> > +     Lanes over the limit are replaced with a truncation mark '.'. By default
> > +     there is no limit.
>
> This should probably mention that 0 means no limit.

I'll add that, it is for zero or any negative number.

>
> > +
> >  ifdef::git-rev-list[]
> >  `--count`::
> >       Print a number stating how many commits would have been
> > diff --git a/t/t4215-log-skewed-merges.sh b/t/t4215-log-skewed-merges.sh
> > index 28d0779a8c..650701df42 100755
> > --- a/t/t4215-log-skewed-merges.sh
> > +++ b/t/t4215-log-skewed-merges.sh
> > @@ -370,4 +370,57 @@ test_expect_success 'log --graph with multiple tips' '
> >       EOF
> >  '
> >
> > +test_expect_success 'log --graph --graph-lane-limit=2 limited to two lanes' '
> > +     check_graph --graph-lane-limit=2 M_7 <<-\EOF
> > +     *-.   7_M4
> > +     |\ \
> > +     | | * 7_G
> > +     | | * 7_F
> > +     | * . 7_E
> > +     | * . 7_D
> > +     * | . 7_C
> > +     | |/
> > +     |/|
> > +     * | 7_B
> > +     |/
> > +     * 7_A
>
> I'm confused. If the lane limit is 2, why do we have actually have 3 lanes?
>
> > +test_expect_success 'log --graph --graph-lane-limit=3 limited to three lanes' '
> > +     check_graph --graph-lane-limit=3 M_1 M_3 M_5 M_7 <<-\EOF
> > +     *   7_M1
> > +     |\
> > +     | | *   7_M2
> > +     | | |\
> > +     | | | * 7_H
> > +     | | | . 7_M3
> > +     | | | . 7_J
> > +     | | | . 7_I
> > +     | | | . 7_M4
> > +     | |_|_.
> > +     |/| | .
> > +     | | |_.
> > +     | |/| .
> > +     | | | .
> > +     | | |/.
> > +     | | * . 7_G
> > +     | | | .
> > +     | | |/.
> > +     | | * . 7_F
> > +     | * | . 7_E
> > +     | | |/.
> > +     | |/| .
> > +     | * | . 7_D
> > +     | | |/
> > +     | |/|
> > +     * | | 7_C
> > +     | |/
> > +     |/|
> > +     * | 7_B
> > +     |/
> > +     * 7_A
>
> Same here. Why is there a fourth lane?

The extra column is the truncation marker that shows in every lane that
had to be truncated horizontally, not an actual lane, even tho that by
keeping the right side edge might seem confusing.

>
> Oh! "Truncation" here does not mean that the vertical lines are cut off
> and are supposed to continue sometime later in the chart. It literally
> means that the *line* is truncated and just some stuff *on that line* is
> omitted.
>
> Ouch! That was not what I was expecting. I thought that truncation means
> that when the eye follows a line vertically, it finds the truncation
> point of the line at some point, and then the continuation of that line
> is again some time later down the chart. The only clue which lanes are
> the same would be the color, which would have to be remedied somehow.
>
> I don't know what to make of it. I have to reconsider.

Yes, the truncation is horizontal, each lane is cut at the lane limit. I went
with this because it is more accessible starting to work at the graph and
understanding it (it is a 17 years old code).

Vertical truncation needs rearranging which lanes are visible and which
ones come and go, similar to gitk (This is the TODO idea that is present
on the graph.c code) I think it will be hard to do it even if there is a way
of not having to rewrite the whole graph rendering code. prob if this is done
it won't replace the actual --graph but actually become something like
--rgraph as a different graph rendering option.

>
> -- Hannes
>

I hope I a made my intentions with horizontal truncation more clear.
I'll send a v5 with all the changes talked about.

Thanks for the feedback throughout all the patches!
Pablo

Re: [GSoC PATCH v4 0/3] graph: add --graph-lane-limit option

[Johannes Sixt]

Am 23.03.26 um 22:59 schrieb Pablo Sabater:
> Repositories that have many active branches at the same time produce
> wide graphs. A lane consists of two columns, the edge and the space
> padding, each branch takes a lane in the graph and there is no way
> to limit how many can be shown.

Generally, I like the goal of this patch series. However, the way in
which it is presented and justified can be improved substantially, IMO.

It begins with the statement of what this patch series wants to achieve.
It is "limit the width of the graph", isn't it? It is not "add
--graph-lane-limit"; that is just a tool to achieve the goal.

To help reviewers, you should present an example chart in the cover
letter that shows the before- and after-state (with and without the user
of the new option).

As far as the separation into patches is concerned, I see a few
problems. With the current separation is difficult to justify the
patches. For example, the first patch adds prerequisites for a later
patch, but it is unclear how these are used. The answer to the question
"Why do we need this?" is simply "because the next patch uses them", but
this is a very weak justification, because the next questions are "how
are they used and why didn't you squash this into the next patch?"

Let me suggest a different separation.

1. The first patch limits the graph width with a hard-coded limit, say
15 lanes. It limits the graph *always*. Choose a limit that is large
enough to pass all tests.

2. The next patch adds --graph-lane-limit and its documentation. Let it
do its thing. Revert to the default limit value 0, i.e., unlimited.

3. Next, add additional eye-candy. I am alluding to the line that marks
where a graph lane was truncated.

(4. If more detailed document is warranted, e.g., an example chart, do
this as a separate patch that can now show all bells and whistles that
the earlier commits have implemented. Whether this makes sense as a
separate step, or whether documentation grows with the earlier patches,
is a judgement call.)

As far as commit messages are concerned, always, always provide an
answer to "Why?" for every detail.

- Why do we want to limit the graph width?
- Why is the hard-coded limit 15? (because it lets tests pass and is
still a useful limit; we'll make it dynamic later.)
- Why do we always limit the graph width? (Because it makes this patch
simpler; we'll fix this later.)
- Why does 0 mean unlimited? (Consistency with --max-parents.)
- Why is the truncation marked with a fullstop "."? (...)

I'll also look over the patches, but I don't do C code, so I can provide
only superficial comments, if any.

-- Hannes

Re: [GSoC PATCH v4 0/3] graph: add --graph-lane-limit option

[Pablo]

Johannes Sixt (<j6t@kdbg.org>) writes:

> Generally, I like the goal of this patch series. However, the way in
> which it is presented and justified can be improved substantially, IMO.
>
> It begins with the statement of what this patch series wants to achieve.
> It is "limit the width of the graph", isn't it? It is not "add
> --graph-lane-limit"; that is just a tool to achieve the goal.
>
> To help reviewers, you should present an example chart in the cover
> letter that shows the before- and after-state (with and without the user
> of the new option).

True, I'll do that. Because it seems on your last review that I didn't explain
myself correctly, the idea was to:

Without --graph-lane-limit:

| | | | | | | * commit message
| | | | | |/

With --graph-lane-limit=3:

| | | . commit message
| | | .

It truncates the lanes horizontally at the lane limit, the "." replaces
everything over the limit (n+1).

> As far as the separation into patches is concerned, I see a few
> problems. With the current separation is difficult to justify the
> patches. For example, the first patch adds prerequisites for a later
> patch, but it is unclear how these are used. The answer to the question
> "Why do we need this?" is simply "because the next patch uses them", but
> this is a very weak justification, because the next questions are "how
> are they used and why didn't you squash this into the next patch?"
>
> Let me suggest a different separation.

I'll merge 1st and 2nd patch together into a single one, adding the option
together with the actual logic that does it. This fixes what SZEDER said about
the first patch alone breaking the build.

And the documentation + tests on a separate commit.

> 1. The first patch limits the graph width with a hard-coded limit, say
> 15 lanes. It limits the graph *always*. Choose a limit that is large
> enough to pass all tests.
>
> 2. The next patch adds --graph-lane-limit and its documentation. Let it
> do its thing. Revert to the default limit value 0, i.e., unlimited.
>
> 3. Next, add additional eye-candy. I am alluding to the line that marks
> where a graph lane was truncated.
>
> (4. If more detailed document is warranted, e.g., an example chart, do
> this as a separate patch that can now show all bells and whistles that
> the earlier commits have implemented. Whether this makes sense as a
> separate step, or whether documentation grows with the earlier patches,
> is a judgement call.)
>
> As far as commit messages are concerned, always, always provide an
> answer to "Why?" for every detail.
>
> - Why do we want to limit the graph width?
> - Why is the hard-coded limit 15? (because it lets tests pass and is
> still a useful limit; we'll make it dynamic later.)
> - Why do we always limit the graph width? (Because it makes this patch
> simpler; we'll fix this later.)
> - Why does 0 mean unlimited? (Consistency with --max-parents.)
> - Why is the truncation marked with a fullstop "."? (...)

Ok, I'll make sure to answer clearly all the WHY's on the commits.


> I'll also look over the patches, but I don't do C code, so I can provide
> only superficial comments, if any.
>
> -- Hannes
>

Re: [GSoC PATCH v4 0/3] graph: add --graph-lane-limit option

[Johannes Sixt]

Am 25.03.26 um 13:28 schrieb Pablo:
> Johannes Sixt (<j6t@kdbg.org>) writes:
>> Let me suggest a different separation.
> 
> I'll merge 1st and 2nd patch together into a single one, adding the option
> together with the actual logic that does it. This fixes what SZEDER said about
> the first patch alone breaking the build.
> 
> And the documentation + tests on a separate commit.

It is better to add documentation and tests in the same commit that add
the feature, because both serve as a specification what the code is
supposed to do. This way reviewers can decide whether the code does
indeed work as designed. On top of that, when the code has to be
inspected later, the commit that introduced the code shows immediately
whether a certain behavior was intentional or not.

So, you would end up with a single patch.

But to make reviewing easier, I proposed a different split:

>> 1. The first patch limits the graph width with a hard-coded limit, say
>> 15 lanes. It limits the graph *always*. Choose a limit that is large
>> enough to pass all tests.

This change will touch the graphing engine, but almost nothing else.

>> 2. The next patch adds --graph-lane-limit and its documentation. Let it
>> do its thing. Revert to the default limit value 0, i.e., unlimited.

This change now introduces all the plumbing that passes the user's
option through to the engine.

>> 3. Next, add additional eye-candy. I am alluding to the line that marks
>> where a graph lane was truncated.

If possible, this change provides final touches that can reasonably be
left out from the first patch without compromising its basic functionality.

>> (4. If more detailed document is warranted, e.g., an example chart, do
>> this as a separate patch that can now show all bells and whistles that
>> the earlier commits have implemented. Whether this makes sense as a
>> separate step, or whether documentation grows with the earlier patches,
>> is a judgement call.)

This could be a new paragraph in the manuals with example charts if
doing so makes sense.

-- Hannes

Re: [GSoC PATCH v4 0/3] graph: add --graph-lane-limit option

[Pablo]

Johannes Sixt (<j6t@kdbg.org>) writes:
>
> Am 25.03.26 um 13:28 schrieb Pablo:
> > Johannes Sixt (<j6t@kdbg.org>) writes:
> >> Let me suggest a different separation.
> >
> > I'll merge 1st and 2nd patch together into a single one, adding the option
> > together with the actual logic that does it. This fixes what SZEDER said about
> > the first patch alone breaking the build.
> >
> > And the documentation + tests on a separate commit.
>
> It is better to add documentation and tests in the same commit that add
> the feature, because both serve as a specification what the code is
> supposed to do. This way reviewers can decide whether the code does
> indeed work as designed. On top of that, when the code has to be
> inspected later, the commit that introduced the code shows immediately
> whether a certain behavior was intentional or not.
>
> So, you would end up with a single patch.
>
> But to make reviewing easier, I proposed a different split:
>
> >> 1. The first patch limits the graph width with a hard-coded limit, say
> >> 15 lanes. It limits the graph *always*. Choose a limit that is large
> >> enough to pass all tests.
>
> This change will touch the graphing engine, but almost nothing else.
>
> >> 2. The next patch adds --graph-lane-limit and its documentation. Let it
> >> do its thing. Revert to the default limit value 0, i.e., unlimited.
>
> This change now introduces all the plumbing that passes the user's
> option through to the engine.
>
> >> 3. Next, add additional eye-candy. I am alluding to the line that marks
> >> where a graph lane was truncated.
>
> If possible, this change provides final touches that can reasonably be
> left out from the first patch without compromising its basic functionality.
>
> >> (4. If more detailed document is warranted, e.g., an example chart, do
> >> this as a separate patch that can now show all bells and whistles that
> >> the earlier commits have implemented. Whether this makes sense as a
> >> separate step, or whether documentation grows with the earlier patches,
> >> is a judgement call.)
>
> This could be a new paragraph in the manuals with example charts if
> doing so makes sense.

Ok, I'll do that to make it better for reviewing, thanks. I've just
sent the v5 at the same
time with this morning's feedback so it has the 2 patch split I talked
about but I'll
do a v6 following this.

>
> -- Hannes
>

Thanks for explaining everything, the patience and the feedback.
Pablo

[GSoC PATCH v5 0/2] graph: add --graph-lane-limit option

[Pablo Sabater]

Repositories that have many active branches at the same time produce
wide graphs. A lane consists of two columns, the edge and the space
padding, each branch takes a lane in the graph and there is no way
to limit how many can be shown.

The limit is a horizontal truncation, each lane is cut at the lane limit:

  Without --graph-lane-limit:

  *   7_M1
  |\  
  | * 7_E
  * | 7_C
  | | *   7_M2
  | | |\  
  | | | * 7_H
  | | |/  
  | |/|   
  | * | 7_D
  | | * 7_G
  | | * 7_F
  | |/  
  |/|   
  * | 7_B
  |/  
  * 7_A

  With --graph-lane-limit=1:

  *   7_M1
  |\  
  | * 7_E
  * ~ 7_C
  | ~ 7_M2
  | ~ 7_H
  | ~ 
  | ~ 
  | * 7_D
  | ~ 7_G
  | ~ 7_F
  | ~ 
  |/~ 
  * ~ 7_B
  |/  
  * 7_A

The '~' is the truncation mark, not an actual lane. It was chosen because '.' is
already used in octopus merges and '~' is not used elsewhere in the graph. Yet
the edges between the last visible lane and the truncation mark are still
conserved.

The '*' commit mark is visible when it lives on a visible lane or the first
hidden lane, any deeper lane doesn't show the commit mark but keeps the commit
message visible. 

Merges where neither the commit nor its parents live on a visible lane are skipped
because they don't carry any visible information.

The original idea to limit columns was noted as a TODO in c12172d2ea
(Add history graph API, 2008-05-04).  This does not implement
gitk-style column rearrangement, it only truncates the visual output.

Possible future improvements:

- When all branches involved in collapsing or padding are over the limit, the
  truncated lane doesn't show any information, this lane could be removed to
  make the graph more compact. Currently these lanes still appear because
  graph_output_collapsing_line() mixes state handling with rendering, so it can't
  be skipped but callers always expect a non empty buffer. Fixing it would need
  to refactor the callers to handle empty buffers instead of expecting them to
  always have content.

- Collapsing and merges lanes that start on visible lanes but end on hidden ones
  are kept to maintain the most information possible on the visible lanes, but
  the information about where they go is lost. They can be kept, removed or think
  of a way to show that information without showing the lanes.

Changes since v4:

- Merged the option parsing and the truncation logic into a single
  patch, fixing the DEVELOPER=1 build break when the first patch was alone.
- Added before/after example to clarify horizontal truncation.
- Fixed error message to match existing format strings.
- Shortened code comment.
- Changed truncation_max -= 1 to truncation_max--.
- Fixed pre-existing indentation in graph_output_collapsing_line().
- Fixed unnecessary blank line changes.
- Added that zero and negative values mean no limit in the docs.
- Changed truncation mark from '.' to '~' because '.' is already
  used in octopus merges.
- Added more tests.

Pablo Sabater (2):
  graph: add --graph-lane-limit option
  graph: add documentation and tests about --graph-lane-limit

 Documentation/rev-list-options.adoc |   6 ++
 graph.c                             | 149 +++++++++++++++++++++++-----
 revision.c                          |   6 ++
 revision.h                          |   1 +
 t/t4215-log-skewed-merges.sh        | 144 +++++++++++++++++++++++++++
 5 files changed, 283 insertions(+), 23 deletions(-)


base-commit: ce74208c2fa13943fffa58f168ac27a76d0eb789
-- 
2.43.0

[GSoC PATCH v5 1/2] graph: add --graph-lane-limit option

[Pablo Sabater]

Repositories that have many active branches at the same time produce
wide graphs. A lane consists of two columns, the edge and the space
padding, each branch takes a lane in the graph and there is no way
to limit how many can be shown.

Add '--graph-lane-limit=<n>' revision option that caps the number
of visible lanes to n. This option requires '--graph', without it
a limit to the graph has no meaning, in this case error out.

Zero and negative values are valid inputs but silently ignored
treating them as "no limit", the same as not using the option.
This follows what '--max-parents' does with negative values.

When the limit is set, lanes over the limit are not drawn.
Teach each graph state to stop rendering at the lane limit
and print a "~" truncation mark, so users know that
there are hidden lanes. The "~" was chosen because it was not used
elsewhere in the graph and it is discrete.

On the commit line, if the commit lives on a visible lane, show
the normal commit mark and truncate after it, if the commit lives
on the first hidden lane show the "*" instead of the truncation mark
so it is known that this commit sits on the first hidden lane.
Commits on deeper lanes don't leave a mark.

For merges, the post-merge lane is only needed when the commit or
the first parent lives on a visible lane (to draw the connection
between them), when both are on hidden lanes, post-merge carries no
useful information, skip it and go to collapsing or padding state.

Also fix a pre-existing indentation issue.

Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
 graph.c    | 150 +++++++++++++++++++++++++++++++++++++++++++++--------
 revision.c |   6 +++
 revision.h |   1 +
 3 files changed, 134 insertions(+), 23 deletions(-)

diff --git a/graph.c b/graph.c
index 26f6fbf000..2218f00c40 100644
--- a/graph.c
+++ b/graph.c
@@ -317,6 +317,15 @@ struct git_graph {
 	struct strbuf prefix_buf;
 };
 
+static int graph_needs_truncation(struct git_graph *graph, int lane)
+{
+	int max = graph->revs->graph_max_lanes;
+	/*
+	 * Ignore values <= 0, meaning no limit.
+	 */
+	return max > 0 && lane >= max;
+}
+
 static const char *diff_output_prefix_callback(struct diff_options *opt, void *data)
 {
 	struct git_graph *graph = data;
@@ -696,6 +705,18 @@ static void graph_update_columns(struct git_graph *graph)
 		}
 	}
 
+	/*
+	 * If graph_max_lanes is set, cap the padding from the branches
+	 */
+	if (graph->revs->graph_max_lanes > 0) {
+		/*
+		 * width of "| " per lanes plus truncation mark "~ ".
+		 */
+		int max_columns_width = graph->revs->graph_max_lanes * 2 + 2;
+		if (graph->width > max_columns_width)
+			graph->width = max_columns_width;
+	}
+
 	/*
 	 * Shrink mapping_size to be the minimum necessary
 	 */
@@ -846,6 +867,10 @@ static void graph_output_padding_line(struct git_graph *graph,
 	 * Output a padding row, that leaves all branch lines unchanged
 	 */
 	for (i = 0; i < graph->num_new_columns; i++) {
+		if (graph_needs_truncation(graph, i)) {
+			graph_line_addstr(line, "~ ");
+			break;
+		}
 		graph_line_write_column(line, &graph->new_columns[i], '|');
 		graph_line_addch(line, ' ');
 	}
@@ -903,6 +928,9 @@ static void graph_output_pre_commit_line(struct git_graph *graph,
 			seen_this = 1;
 			graph_line_write_column(line, col, '|');
 			graph_line_addchars(line, ' ', graph->expansion_row);
+		} else if (seen_this && graph_needs_truncation(graph, i)) {
+			graph_line_addstr(line, "~ ");
+			break;
 		} else if (seen_this && (graph->expansion_row == 0)) {
 			/*
 			 * This is the first line of the pre-commit output.
@@ -994,6 +1022,12 @@ static void graph_draw_octopus_merge(struct git_graph *graph, struct graph_line
 		col = &graph->new_columns[j];
 
 		graph_line_write_column(line, col, '-');
+
+		if (graph_needs_truncation(graph, j / 2 + i)) {
+			graph_line_addstr(line, "~ ");
+			break;
+		}
+
 		graph_line_write_column(line, col, (i == dashed_parents - 1) ? '.' : '-');
 	}
 
@@ -1028,8 +1062,17 @@ static void graph_output_commit_line(struct git_graph *graph, struct graph_line
 			seen_this = 1;
 			graph_output_commit_char(graph, line);
 
+			if (graph_needs_truncation(graph, i)) {
+				graph_line_addch(line, ' ');
+				break;
+			}
+
 			if (graph->num_parents > 2)
 				graph_draw_octopus_merge(graph, line);
+		} else if (graph_needs_truncation(graph, i)) {
+			graph_line_addstr(line, "~ ");
+			seen_this = 1;
+			break;
 		} else if (seen_this && (graph->edges_added > 1)) {
 			graph_line_write_column(line, col, '\\');
 		} else if (seen_this && (graph->edges_added == 1)) {
@@ -1065,10 +1108,32 @@ static void graph_output_commit_line(struct git_graph *graph, struct graph_line
 
 	/*
 	 * Update graph->state
+	 *
+	 * If the commit is a merge and the first parent is in a visible lane,
+	 * then the GRAPH_POST_MERGE is needed to draw the merge lane.
+	 *
+	 * If the commit is over the truncation limit, but the first parent is on
+	 * a visible lane, then we still need the merge lane but truncated.
+	 *
+	 * If both commit and first parent are over the truncation limit, then
+	 * there's no need to draw the merge lane because it would work as a
+	 * padding lane.
 	 */
-	if (graph->num_parents > 1)
-		graph_update_state(graph, GRAPH_POST_MERGE);
-	else if (graph_is_mapping_correct(graph))
+	if (graph->num_parents > 1) {
+		if (!graph_needs_truncation(graph, graph->commit_index)) {
+			graph_update_state(graph, GRAPH_POST_MERGE);
+		} else {
+			struct commit_list *first_parent = first_interesting_parent(graph);
+			int first_parent_col = graph_find_new_column_by_commit(graph, first_parent->item);
+
+			if (!graph_needs_truncation(graph, first_parent_col))
+				graph_update_state(graph, GRAPH_POST_MERGE);
+			else if (graph_is_mapping_correct(graph))
+				graph_update_state(graph, GRAPH_PADDING);
+			else
+				graph_update_state(graph, GRAPH_COLLAPSING);
+		}
+	} else if (graph_is_mapping_correct(graph))
 		graph_update_state(graph, GRAPH_PADDING);
 	else
 		graph_update_state(graph, GRAPH_COLLAPSING);
@@ -1109,6 +1174,7 @@ static void graph_output_post_merge_line(struct git_graph *graph, struct graph_l
 			int par_column;
 			int idx = graph->merge_layout;
 			char c;
+			int truncated = 0;
 			seen_this = 1;
 
 			for (j = 0; j < graph->num_parents; j++) {
@@ -1117,23 +1183,46 @@ static void graph_output_post_merge_line(struct git_graph *graph, struct graph_l
 
 				c = merge_chars[idx];
 				graph_line_write_column(line, &graph->new_columns[par_column], c);
+				if (graph_needs_truncation(graph, j / 2 + i) &&
+				    j / 2 + i <= graph->num_columns) {
+					if ((j + i * 2) % 2 != 0)
+						graph_line_addch(line, ' ');
+					graph_line_addstr(line, "~ ");
+					truncated = 1;
+					break;
+				}
+
 				if (idx == 2) {
-					if (graph->edges_added > 0 || j < graph->num_parents - 1)
+					if (graph_needs_truncation(graph, (j + 1) / 2 + i) &&
+					    j < graph->num_parents - 1) {
+						graph_line_addstr(line, "~ ");
+						truncated = 1;
+						break;
+					} else if (graph->edges_added > 0 || j < graph->num_parents - 1)
 						graph_line_addch(line, ' ');
 				} else {
 					idx++;
 				}
 				parents = next_interesting_parent(graph, parents);
 			}
+			if (truncated)
+				break;
 			if (graph->edges_added == 0)
 				graph_line_addch(line, ' ');
-
+		} else if (graph_needs_truncation(graph, i)) {
+			graph_line_addstr(line, "~ ");
+			break;
 		} else if (seen_this) {
 			if (graph->edges_added > 0)
 				graph_line_write_column(line, col, '\\');
 			else
 				graph_line_write_column(line, col, '|');
-			graph_line_addch(line, ' ');
+			/*
+			 * If it's between two lanes and next would be truncated,
+			 * don't add space padding.
+			 */
+			if (!graph_needs_truncation(graph, i + 1))
+				graph_line_addch(line, ' ');
 		} else {
 			graph_line_write_column(line, col, '|');
 			if (graph->merge_layout != 0 || i != graph->commit_index - 1) {
@@ -1164,6 +1253,7 @@ static void graph_output_collapsing_line(struct git_graph *graph, struct graph_l
 	short used_horizontal = 0;
 	int horizontal_edge = -1;
 	int horizontal_edge_target = -1;
+	int truncated = 0;
 
 	/*
 	 * Swap the mapping and old_mapping arrays
@@ -1279,26 +1369,35 @@ static void graph_output_collapsing_line(struct git_graph *graph, struct graph_l
 	 */
 	for (i = 0; i < graph->mapping_size; i++) {
 		int target = graph->mapping[i];
-		if (target < 0)
-			graph_line_addch(line, ' ');
-		else if (target * 2 == i)
-			graph_line_write_column(line, &graph->new_columns[target], '|');
-		else if (target == horizontal_edge_target &&
-			 i != horizontal_edge - 1) {
-				/*
-				 * Set the mappings for all but the
-				 * first segment to -1 so that they
-				 * won't continue into the next line.
-				 */
-				if (i != (target * 2)+3)
-					graph->mapping[i] = -1;
-				used_horizontal = 1;
-			graph_line_write_column(line, &graph->new_columns[target], '_');
+
+		if (!truncated && graph_needs_truncation(graph, i / 2)) {
+			graph_line_addstr(line, "~ ");
+			truncated = 1;
+		}
+
+		if (target < 0) {
+			if (!truncated)
+				graph_line_addch(line, ' ');
+		} else if (target * 2 == i) {
+			if (!truncated)
+				graph_line_write_column(line, &graph->new_columns[target], '|');
+		} else if (target == horizontal_edge_target &&
+			   i != horizontal_edge - 1) {
+			/*
+			 * Set the mappings for all but the
+			 * first segment to -1 so that they
+			 * won't continue into the next line.
+			 */
+			if (i != (target * 2)+3)
+				graph->mapping[i] = -1;
+			used_horizontal = 1;
+			if (!truncated)
+				graph_line_write_column(line, &graph->new_columns[target], '_');
 		} else {
 			if (used_horizontal && i < horizontal_edge)
 				graph->mapping[i] = -1;
-			graph_line_write_column(line, &graph->new_columns[target], '/');
-
+			if (!truncated)
+				graph_line_write_column(line, &graph->new_columns[target], '/');
 		}
 	}
 
@@ -1372,6 +1471,11 @@ static void graph_padding_line(struct git_graph *graph, struct strbuf *sb)
 	for (i = 0; i < graph->num_columns; i++) {
 		struct column *col = &graph->columns[i];
 
+		if (graph_needs_truncation(graph, i)) {
+			graph_line_addch(&line, '~');
+			break;
+		}
+
 		graph_line_write_column(&line, col, '|');
 
 		if (col->commit == graph->commit && graph->num_parents > 2) {
diff --git a/revision.c b/revision.c
index 31808e3df0..81b67682a8 100644
--- a/revision.c
+++ b/revision.c
@@ -2605,6 +2605,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 	} else if (!strcmp(arg, "--no-graph")) {
 		graph_clear(revs->graph);
 		revs->graph = NULL;
+	} else if (skip_prefix(arg, "--graph-lane-limit=", &optarg)) {
+		revs->graph_max_lanes = parse_count(optarg);
 	} else if (!strcmp(arg, "--encode-email-headers")) {
 		revs->encode_email_headers = 1;
 	} else if (!strcmp(arg, "--no-encode-email-headers")) {
@@ -3172,6 +3174,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 
 	if (revs->no_walk && revs->graph)
 		die(_("options '%s' and '%s' cannot be used together"), "--no-walk", "--graph");
+
+	if (revs->graph_max_lanes > 0 && !revs->graph)
+		die(_("the option '%s' requires '%s'"), "--graph-lane-limit", "--graph");
+
 	if (!revs->reflog_info && revs->grep_filter.use_reflog_filter)
 		die(_("the option '%s' requires '%s'"), "--grep-reflog", "--walk-reflogs");
 
diff --git a/revision.h b/revision.h
index 69242ecb18..874ccce625 100644
--- a/revision.h
+++ b/revision.h
@@ -304,6 +304,7 @@ struct rev_info {
 
 	/* Display history graph */
 	struct git_graph *graph;
+	int graph_max_lanes;
 
 	/* special limits */
 	int skip_count;

base-commit: ce74208c2fa13943fffa58f168ac27a76d0eb789
-- 
2.43.0

Re: [GSoC PATCH v5 1/2] graph: add --graph-lane-limit option

[Junio C Hamano]

Pablo Sabater <pabloosabaterr@gmail.com> writes:

> +static int graph_needs_truncation(struct git_graph *graph, int lane)
> +{
> +	int max = graph->revs->graph_max_lanes;
> +	/*
> +	 * Ignore values <= 0, meaning no limit.
> +	 */
> +	return max > 0 && lane >= max;
> +}

Make a mental note that this helper function works on number of
lanes, not display columns (which is roughly twice the number of
lanes).

> @@ -696,6 +705,18 @@ static void graph_update_columns(struct git_graph *graph)
>  		}
>  	}
>  
> +	/*
> +	 * If graph_max_lanes is set, cap the padding from the branches
> +	 */
> +	if (graph->revs->graph_max_lanes > 0) {
> +		/*
> +		 * width of "| " per lanes plus truncation mark "~ ".
> +		 */
> +		int max_columns_width = graph->revs->graph_max_lanes * 2 + 2;
> +		if (graph->width > max_columns_width)
> +			graph->width = max_columns_width;
> +	}
> +
>  	/*
>  	 * Shrink mapping_size to be the minimum necessary
>  	 */
> @@ -846,6 +867,10 @@ static void graph_output_padding_line(struct git_graph *graph,
>  	 * Output a padding row, that leaves all branch lines unchanged
>  	 */
>  	for (i = 0; i < graph->num_new_columns; i++) {
> +		if (graph_needs_truncation(graph, i)) {
> +			graph_line_addstr(line, "~ ");
> +			break;
> +		}

And that mental note helps to convince us this loop makes sense, as
it increments 'i' one by one ;-)

> @@ -903,6 +928,9 @@ static void graph_output_pre_commit_line(struct git_graph *graph,
>  			seen_this = 1;
>  			graph_line_write_column(line, col, '|');
>  			graph_line_addchars(line, ' ', graph->expansion_row);
> +		} else if (seen_this && graph_needs_truncation(graph, i)) {
> +			graph_line_addstr(line, "~ ");
> +			break;
>  		} else if (seen_this && (graph->expansion_row == 0)) {
>  			/*
>  			 * This is the first line of the pre-commit output.
> @@ -994,6 +1022,12 @@ static void graph_draw_octopus_merge(struct git_graph *graph, struct graph_line
>  		col = &graph->new_columns[j];
>  
>  		graph_line_write_column(line, col, '-');

And here, 'j' comes from graph->mapping[] array.  Does that count in
display columns or lanes?

> +		if (graph_needs_truncation(graph, j / 2 + i)) {

This makes it look as if 'j' counts in columns and needs to be
divided by 2 to make it comparable to lanes.

> +			graph_line_addstr(line, "~ ");
> +			break;
> +		}
> +
>  		graph_line_write_column(line, col, (i == dashed_parents - 1) ? '.' : '-');
>  	}
>  

> +	if (graph->num_parents > 1) {
> +		if (!graph_needs_truncation(graph, graph->commit_index)) {
> +			graph_update_state(graph, GRAPH_POST_MERGE);
> +		} else {
> +			struct commit_list *first_parent = first_interesting_parent(graph);
> +			int first_parent_col = graph_find_new_column_by_commit(graph, first_parent->item);

Are we sure that first_interesting_parent() will always give us a
non-NULL pointer?

Can we use a bit shorter identifier names to deal with these overly
long lines?  The lifetime of these two variables is very short so they
do not have to be so descriptive.

			struct commit *p = first_interesting_parent(graph)->item;
			int lane = graph_find_new_column_by_commit(graph, p);

> +			if (!graph_needs_truncation(graph, first_parent_col))
> +				graph_update_state(graph, GRAPH_POST_MERGE);
> +			else if (graph_is_mapping_correct(graph))
> +				graph_update_state(graph, GRAPH_PADDING);
> +			else
> +				graph_update_state(graph, GRAPH_COLLAPSING);
> +		}
> +	} else if (graph_is_mapping_correct(graph))

Re: [GSoC PATCH v5 1/2] graph: add --graph-lane-limit option

[Pablo]

Junio C Hamano (<gitster@pobox.com>) writes:
>
> Pablo Sabater <pabloosabaterr@gmail.com> writes:
>
> > +static int graph_needs_truncation(struct git_graph *graph, int lane)
> > +{
> > +     int max = graph->revs->graph_max_lanes;
> > +     /*
> > +      * Ignore values <= 0, meaning no limit.
> > +      */
> > +     return max > 0 && lane >= max;
> > +}
>
> Make a mental note that this helper function works on number of
> lanes, not display columns (which is roughly twice the number of
> lanes).
>
> > @@ -696,6 +705,18 @@ static void graph_update_columns(struct git_graph *graph)
> >               }
> >       }
> >
> > +     /*
> > +      * If graph_max_lanes is set, cap the padding from the branches
> > +      */
> > +     if (graph->revs->graph_max_lanes > 0) {
> > +             /*
> > +              * width of "| " per lanes plus truncation mark "~ ".
> > +              */
> > +             int max_columns_width = graph->revs->graph_max_lanes * 2 + 2;
> > +             if (graph->width > max_columns_width)
> > +                     graph->width = max_columns_width;
> > +     }
> > +
> >       /*
> >        * Shrink mapping_size to be the minimum necessary
> >        */
> > @@ -846,6 +867,10 @@ static void graph_output_padding_line(struct git_graph *graph,
> >        * Output a padding row, that leaves all branch lines unchanged
> >        */
> >       for (i = 0; i < graph->num_new_columns; i++) {
> > +             if (graph_needs_truncation(graph, i)) {
> > +                     graph_line_addstr(line, "~ ");
> > +                     break;
> > +             }
>
> And that mental note helps to convince us this loop makes sense, as
> it increments 'i' one by one ;-)

Ok, I'll add the note to graph_needs_truncation() and any other places
that might need to be more clear about if it handles columns or lanes.

>
> > @@ -903,6 +928,9 @@ static void graph_output_pre_commit_line(struct git_graph *graph,
> >                       seen_this = 1;
> >                       graph_line_write_column(line, col, '|');
> >                       graph_line_addchars(line, ' ', graph->expansion_row);
> > +             } else if (seen_this && graph_needs_truncation(graph, i)) {
> > +                     graph_line_addstr(line, "~ ");
> > +                     break;
> >               } else if (seen_this && (graph->expansion_row == 0)) {
> >                       /*
> >                        * This is the first line of the pre-commit output.
> > @@ -994,6 +1022,12 @@ static void graph_draw_octopus_merge(struct git_graph *graph, struct graph_line
> >               col = &graph->new_columns[j];
> >
> >               graph_line_write_column(line, col, '-');
>
> And here, 'j' comes from graph->mapping[] array.  Does that count in
> display columns or lanes?
>
> > +             if (graph_needs_truncation(graph, j / 2 + i)) {
>
> This makes it look as if 'j' counts in columns and needs to be
> divided by 2 to make it comparable to lanes.

Actually, no, because there are other parts like
graph_output_post_merge_line handling i and j  like that and it is a
more mechanical thing that logical I didn't double checked it, it
should be something like commit_index + 1 + i similar to what j is,
imma check to be sure and add another test for this to be sure because
current ones pass this and that's why I thought it was ok in the first
place.

>
> > +                     graph_line_addstr(line, "~ ");
> > +                     break;
> > +             }
> > +
> >               graph_line_write_column(line, col, (i == dashed_parents - 1) ? '.' : '-');
> >       }
> >
>
> > +     if (graph->num_parents > 1) {
> > +             if (!graph_needs_truncation(graph, graph->commit_index)) {
> > +                     graph_update_state(graph, GRAPH_POST_MERGE);
> > +             } else {
> > +                     struct commit_list *first_parent = first_interesting_parent(graph);
> > +                     int first_parent_col = graph_find_new_column_by_commit(graph, first_parent->item);
>
> Are we sure that first_interesting_parent() will always give us a
> non-NULL pointer?

my bad, first_interestign_parent() can be a NULL, will add a check for that

>
> Can we use a bit shorter identifier names to deal with these overly
> long lines?  The lifetime of these two variables is very short so they
> do not have to be so descriptive.
>
>                         struct commit *p = first_interesting_parent(graph)->item;
>                         int lane = graph_find_new_column_by_commit(graph, p);
>
> > +                     if (!graph_needs_truncation(graph, first_parent_col))
> > +                             graph_update_state(graph, GRAPH_POST_MERGE);
> > +                     else if (graph_is_mapping_correct(graph))
> > +                             graph_update_state(graph, GRAPH_PADDING);
> > +                     else
> > +                             graph_update_state(graph, GRAPH_COLLAPSING);
> > +             }
> > +     } else if (graph_is_mapping_correct(graph))
>

sure

Thanks for the feedback I'll start with the v6,
Pablo.

Re: [GSoC PATCH v5 1/2] graph: add --graph-lane-limit option

[Pablo]

Pablo (<pabloosabaterr@gmail.com>) writes:

> > > +                     graph_line_addstr(line, "~ ");
> > > +                     break;
> > > +             }
> > > +
> > >               graph_line_write_column(line, col, (i == dashed_parents - 1) ? '.' : '-');
> > >       }
> > >
> >
> > > +     if (graph->num_parents > 1) {
> > > +             if (!graph_needs_truncation(graph, graph->commit_index)) {
> > > +                     graph_update_state(graph, GRAPH_POST_MERGE);
> > > +             } else {
> > > +                     struct commit_list *first_parent = first_interesting_parent(graph);
> > > +                     int first_parent_col = graph_find_new_column_by_commit(graph, first_parent->item);
> >
> > Are we sure that first_interesting_parent() will always give us a
> > non-NULL pointer?
>
> my bad, first_interestign_parent() can be a NULL, will add a check for that

Actually, I've been looking and first_interesting_parent() can't
return NULL here because: num_parents is counted using
first_interesting_parent()/next_interesting_parent() so if num_parents
> 1 it guarantees that first_interesting_parent() is non-NULL. I'll
add a BUG() to have it reflected on the code.

Pablo

Re: [GSoC PATCH v5 1/2] graph: add --graph-lane-limit option

[Junio C Hamano]

Pablo <pabloosabaterr@gmail.com> writes:

>> Make a mental note that this helper function works on number of
>> lanes, not display columns (which is roughly twice the number of
>> lanes).
>> ...
>> And that mental note helps to convince us this loop makes sense, as
>> it increments 'i' one by one ;-)
>
> Ok, I'll add the note to graph_needs_truncation() and any other places
> that might need to be more clear about if it handles columns or lanes.

Sorry, I should have taken into account that you are new around
here.  My "mental note" comment wasn't meant to suggest adding extra
comments in the code.  Rather, it is "readers would make a mental
note here after reading this piece of code---and then what they
later see this other piece of code, what it does is consistent with
what they remember from the earlier piece code did, which is good"
(if they are inconsistent, you'd see a similar "make a mental note
here" followed later by "but this contradicts what we saw
earlier. what is going on?!?!" instead).

[GSoC PATCH v5 2/2] graph: add documentation and tests about --graph-lane-limit

[Pablo Sabater]

Document --graph-lane-limit option in rev-list-options.adoc with
--graph option.

Add multiple tests in t4215 reusing existing last graph, test
for different scenarios.

Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
 Documentation/rev-list-options.adoc |   6 ++
 t/t4215-log-skewed-merges.sh        | 144 ++++++++++++++++++++++++++++
 2 files changed, 150 insertions(+)

diff --git a/Documentation/rev-list-options.adoc b/Documentation/rev-list-options.adoc
index 2d195a1474..56590f4e95 100644
--- a/Documentation/rev-list-options.adoc
+++ b/Documentation/rev-list-options.adoc
@@ -1259,6 +1259,12 @@ This implies the `--topo-order` option by default, but the
 	in between them in that case. If _<barrier>_ is specified, it
 	is the string that will be shown instead of the default one.
 
+`--graph-lane-limit=<n>`::
+	When `--graph` is used, limit the number of graph lanes to be shown.
+	Lanes over the limit are replaced with a truncation mark '~'. By default
+	it is set to 0 (no limit), zero and negative values are ignored and
+	treated as no limit.
+
 ifdef::git-rev-list[]
 `--count`::
 	Print a number stating how many commits would have been
diff --git a/t/t4215-log-skewed-merges.sh b/t/t4215-log-skewed-merges.sh
index 28d0779a8c..1612f05f1b 100755
--- a/t/t4215-log-skewed-merges.sh
+++ b/t/t4215-log-skewed-merges.sh
@@ -370,4 +370,148 @@ test_expect_success 'log --graph with multiple tips' '
 	EOF
 '
 
+test_expect_success 'log --graph --graph-lane-limit=2 limited to two lanes' '
+	check_graph --graph-lane-limit=2 M_7 <<-\EOF
+	*-.   7_M4
+	|\ \
+	| | * 7_G
+	| | * 7_F
+	| * ~ 7_E
+	| * ~ 7_D
+	* | ~ 7_C
+	| |/
+	|/|
+	* | 7_B
+	|/
+	* 7_A
+	EOF
+'
+
+test_expect_success 'log --graph --graph-lane-limit=1 truncate mid octopus merge' '
+	check_graph --graph-lane-limit=1 M_7 <<-\EOF
+	*-~  7_M4
+	|\~
+	| ~ 7_G
+	| ~ 7_F
+	| * 7_E
+	| * 7_D
+	* ~ 7_C
+	| ~
+	|/~
+	* ~ 7_B
+	|/
+	* 7_A
+	EOF
+'
+
+test_expect_success 'log --graph --graph-lane-limit=3 limited to three lanes' '
+	check_graph --graph-lane-limit=3 M_1 M_3 M_5 M_7 <<-\EOF
+	*   7_M1
+	|\
+	| | *   7_M2
+	| | |\
+	| | | * 7_H
+	| | | ~ 7_M3
+	| | | ~ 7_J
+	| | | ~ 7_I
+	| | | ~ 7_M4
+	| |_|_~
+	|/| | ~
+	| | |_~
+	| |/| ~
+	| | | ~
+	| | |/~
+	| | * ~ 7_G
+	| | | ~
+	| | |/~
+	| | * ~ 7_F
+	| * | ~ 7_E
+	| | |/~
+	| |/| ~
+	| * | ~ 7_D
+	| | |/
+	| |/|
+	* | | 7_C
+	| |/
+	|/|
+	* | 7_B
+	|/
+	* 7_A
+	EOF
+'
+
+test_expect_success 'log --graph --graph-lane-limit=6 check if it only shows first of 3 parent merge' '
+	check_graph --graph-lane-limit=6 M_1 M_3 M_5 M_7 <<-\EOF
+	*   7_M1
+	|\
+	| | *   7_M2
+	| | |\
+	| | | * 7_H
+	| | | | *   7_M3
+	| | | | |\
+	| | | | | * 7_J
+	| | | | * | 7_I
+	| | | | | | * 7_M4
+	| |_|_|_|_|/~
+	|/| | | | |/~
+	| | |_|_|/| ~
+	| |/| | | |/
+	| | | |_|/|
+	| | |/| | |
+	| | * | | | 7_G
+	| | | |_|/
+	| | |/| |
+	| | * | | 7_F
+	| * | | | 7_E
+	| | |/ /
+	| |/| |
+	| * | | 7_D
+	| | |/
+	| |/|
+	* | | 7_C
+	| |/
+	|/|
+	* | 7_B
+	|/
+	* 7_A
+	EOF
+'
+
+test_expect_success 'log --graph --graph-lane-limit=7 check if it shows all 3 parent merge' '
+	check_graph --graph-lane-limit=7 M_1 M_3 M_5 M_7 <<-\EOF
+	*   7_M1
+	|\
+	| | *   7_M2
+	| | |\
+	| | | * 7_H
+	| | | | *   7_M3
+	| | | | |\
+	| | | | | * 7_J
+	| | | | * | 7_I
+	| | | | | | *   7_M4
+	| |_|_|_|_|/|\
+	|/| | | | |/ /
+	| | |_|_|/| /
+	| |/| | | |/
+	| | | |_|/|
+	| | |/| | |
+	| | * | | | 7_G
+	| | | |_|/
+	| | |/| |
+	| | * | | 7_F
+	| * | | | 7_E
+	| | |/ /
+	| |/| |
+	| * | | 7_D
+	| | |/
+	| |/|
+	* | | 7_C
+	| |/
+	|/|
+	* | 7_B
+	|/
+	* 7_A
+	EOF
+'
+
 test_done
-- 
2.43.0

[GSoC PATCH v6 0/3] graph: add --graph-lane-limit option

[Pablo Sabater]

Repositories that have many active branches at the same time produce
wide graphs. A lane consists of two columns, the edge and the space
padding, each branch takes a lane in the graph and there is no way
to limit how many can be shown.

The limit is a horizontal truncation, each lane is cut at the lane limit:

  Without --graph-lane-limit:

  *   7_M1
  |\  
  | * 7_E
  * | 7_C
  | | *   7_M2
  | | |\  
  | | | * 7_H
  | | |/  
  | |/|   
  | * | 7_D
  | | * 7_G
  | | * 7_F
  | |/  
  |/|   
  * | 7_B
  |/  
  * 7_A

  With --graph-lane-limit=1:

  *   7_M1
  |\  
  | * 7_E
  * ~ 7_C
  | ~ 7_M2
  | ~ 7_H
  | ~ 
  | ~ 
  | * 7_D
  | ~ 7_G
  | ~ 7_F
  | ~ 
  |/~ 
  * ~ 7_B
  |/  
  * 7_A

The '~' is the truncation mark, not an actual lane. It was chosen because '.' is
already used in octopus merges and '~' is not used elsewhere in the graph. Yet
the edges between the last visible lane and the truncation mark are still
conserved.

The '*' commit mark is visible when it lives on a visible lane or the first
hidden lane, any deeper lane doesn't show the commit mark but keeps the commit
message visible. 

Merges where neither the commit nor its parents live on a visible lane are skipped
because they don't carry any visible information.

The original idea to limit columns was noted as a TODO in c12172d2ea
(Add history graph API, 2008-05-04).  This does not implement
gitk-style column rearrangement, it only truncates the visual output.

Possible future improvements:

- When all branches involved in collapsing or padding are over the limit, the
  truncated lane doesn't show any information, this lane could be removed to
  make the graph more compact. Currently these lanes still appear because
  graph_output_collapsing_line() mixes state handling with rendering, so it can't
  be skipped but callers always expect a non empty buffer. Fixing it would need
  to refactor the callers to handle empty buffers instead of expecting them to
  always have content.

- Collapsing and merges lanes that start on visible lanes but end on hidden ones
  are kept to maintain the most information possible on the visible lanes, but
  the information about where they go or come from is lost. They can be kept,
  removed or think of a way to show that information without showing the lanes.

Changes since v5:

- Changed patch structure
- Fixed octopus merge truncation check to commit_index + 1 + i.
- Added check for first_interesting_parent() NULL check.
- Shortened variable names.
- Added clarifications when converting between lanes and columns.

Pablo Sabater (3):
  graph: limit the graph width to a hard-coded max
  graph: add --graph-lane-limit option
  graph: add truncation mark to capped lanes

 Documentation/rev-list-options.adoc |   6 +
 graph.c                             | 178 ++++++++++++++++++++++++----
 revision.c                          |   6 +
 revision.h                          |   1 +
 t/t4215-log-skewed-merges.sh        | 144 ++++++++++++++++++++++
 5 files changed, 311 insertions(+), 24 deletions(-)


base-commit: 5361983c075154725be47b65cca9a2421789e410
-- 
2.43.0

[GSoC PATCH v6 1/3] graph: limit the graph width to a hard-coded max

[Pablo Sabater]

Repositories that have many active branches at the same time
produce wide graphs. A lane consists of two columns, the edge and the
padding (or another edge), each branch takes a lane in the graph and
there is no way to limit how many can be shown.

Limit the graph engine to draw at most 15 lanes. Lanes over the limit are
not rendered.

On the commit line, if the commit lives on a visible lane, show the
normal commit mark and stop rendering. If the commit lives on the
first hidden lane, show the "*" commit mark so it is known that
this commit lives in the first hidden lane. Commits on deeper lanes
aren't rendered, but the commit subject will always remain.

For merges, the post-merge lane is only needed when the commit or
the first parent lives on a visible lane (to draw the connection
between them), when both are on hidden lanes, post-merge carries
no useful information, skip it and go to collapsing or padding state.

Also fix a pre-existing indentation issue.

The hard-coded limit will be replaced by a user-facing option
on a subsequent commit.

Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
 graph.c | 161 +++++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 136 insertions(+), 25 deletions(-)

diff --git a/graph.c b/graph.c
index 26f6fbf000..70458cf323 100644
--- a/graph.c
+++ b/graph.c
@@ -82,6 +82,8 @@ static void graph_show_line_prefix(const struct diff_options *diffopt)
 static const char **column_colors;
 static unsigned short column_colors_max;
 
+static unsigned int max_lanes = 15;
+
 static void parse_graph_colors_config(struct strvec *colors, const char *string)
 {
 	const char *end, *start;
@@ -317,6 +319,11 @@ struct git_graph {
 	struct strbuf prefix_buf;
 };
 
+static inline int graph_needs_truncation(int lane)
+{
+	return lane >= max_lanes;
+}
+
 static const char *diff_output_prefix_callback(struct diff_options *opt, void *data)
 {
 	struct git_graph *graph = data;
@@ -607,7 +614,7 @@ static void graph_update_columns(struct git_graph *graph)
 {
 	struct commit_list *parent;
 	int max_new_columns;
-	int i, seen_this, is_commit_in_columns;
+	int i, seen_this, is_commit_in_columns, max;
 
 	/*
 	 * Swap graph->columns with graph->new_columns
@@ -696,6 +703,14 @@ static void graph_update_columns(struct git_graph *graph)
 		}
 	}
 
+	/*
+	 * Cap to the hard-coded limit.
+	 * Allow commits from merges to align to the merged lane.
+	 */
+	max = max_lanes * 2 + 2;
+	if (graph->width > max)
+		graph->width = max;
+
 	/*
 	 * Shrink mapping_size to be the minimum necessary
 	 */
@@ -846,6 +861,8 @@ static void graph_output_padding_line(struct git_graph *graph,
 	 * Output a padding row, that leaves all branch lines unchanged
 	 */
 	for (i = 0; i < graph->num_new_columns; i++) {
+		if (graph_needs_truncation(i))
+			break;
 		graph_line_write_column(line, &graph->new_columns[i], '|');
 		graph_line_addch(line, ' ');
 	}
@@ -903,6 +920,8 @@ static void graph_output_pre_commit_line(struct git_graph *graph,
 			seen_this = 1;
 			graph_line_write_column(line, col, '|');
 			graph_line_addchars(line, ' ', graph->expansion_row);
+		} else if (seen_this && graph_needs_truncation(i)) {
+			break;
 		} else if (seen_this && (graph->expansion_row == 0)) {
 			/*
 			 * This is the first line of the pre-commit output.
@@ -994,6 +1013,14 @@ static void graph_draw_octopus_merge(struct git_graph *graph, struct graph_line
 		col = &graph->new_columns[j];
 
 		graph_line_write_column(line, col, '-');
+
+		/*
+		 * Commit is at commit_index, each iteration move one lane to
+		 * the right from the commit.
+		 */
+		if (graph_needs_truncation(graph->commit_index + 1 + i))
+			break;
+
 		graph_line_write_column(line, col, (i == dashed_parents - 1) ? '.' : '-');
 	}
 
@@ -1028,8 +1055,16 @@ static void graph_output_commit_line(struct git_graph *graph, struct graph_line
 			seen_this = 1;
 			graph_output_commit_char(graph, line);
 
+			if (graph_needs_truncation(i)) {
+				graph_line_addch(line, ' ');
+				break;
+			}
+
 			if (graph->num_parents > 2)
 				graph_draw_octopus_merge(graph, line);
+		} else if (graph_needs_truncation(i)) {
+			seen_this = 1;
+			break;
 		} else if (seen_this && (graph->edges_added > 1)) {
 			graph_line_write_column(line, col, '\\');
 		} else if (seen_this && (graph->edges_added == 1)) {
@@ -1065,13 +1100,46 @@ static void graph_output_commit_line(struct git_graph *graph, struct graph_line
 
 	/*
 	 * Update graph->state
+	 *
+	 * If the commit is a merge and the first parent is in a visible lane,
+	 * then the GRAPH_POST_MERGE is needed to draw the merge lane.
+	 *
+	 * If the commit is over the truncation limit, but the first parent is on
+	 * a visible lane, then we still need the merge lane but truncated.
+	 *
+	 * If both commit and first parent are over the truncation limit, then
+	 * there's no need to draw the merge lane because it would work as a
+	 * padding lane.
 	 */
-	if (graph->num_parents > 1)
-		graph_update_state(graph, GRAPH_POST_MERGE);
-	else if (graph_is_mapping_correct(graph))
+	if (graph->num_parents > 1) {
+		if (!graph_needs_truncation(graph->commit_index)) {
+			graph_update_state(graph, GRAPH_POST_MERGE);
+		} else {
+			struct commit_list *p = first_interesting_parent(graph);
+			int lane;
+
+			/*
+			 * graph->num_parents are found using first_interesting_parent
+			 * and next_interesting_parent so it can't be a scenario
+			 * where num_parents > 1 and there are no interesting parents
+			 */
+			if (!p)
+				BUG("num_parents > 1 but no interesting parent");
+
+			lane = graph_find_new_column_by_commit(graph, p->item);
+
+			if (!graph_needs_truncation(lane))
+				graph_update_state(graph, GRAPH_POST_MERGE);
+			else if (graph_is_mapping_correct(graph))
+				graph_update_state(graph, GRAPH_PADDING);
+			else
+				graph_update_state(graph, GRAPH_COLLAPSING);
+		}
+	} else if (graph_is_mapping_correct(graph)) {
 		graph_update_state(graph, GRAPH_PADDING);
-	else
+	} else {
 		graph_update_state(graph, GRAPH_COLLAPSING);
+	}
 }
 
 static const char merge_chars[] = {'/', '|', '\\'};
@@ -1109,6 +1177,7 @@ static void graph_output_post_merge_line(struct git_graph *graph, struct graph_l
 			int par_column;
 			int idx = graph->merge_layout;
 			char c;
+			int truncated = 0;
 			seen_this = 1;
 
 			for (j = 0; j < graph->num_parents; j++) {
@@ -1117,23 +1186,53 @@ static void graph_output_post_merge_line(struct git_graph *graph, struct graph_l
 
 				c = merge_chars[idx];
 				graph_line_write_column(line, &graph->new_columns[par_column], c);
+
+				/*
+				 * j counts parents, it needs to be halved to be
+				 * comparable with i. Don't truncate if there are
+				 * no more lanes to print (end of the lane)
+				 */
+				if (graph_needs_truncation(j / 2 + i) &&
+				    j / 2 + i <= graph->num_columns) {
+					if ((j + i * 2) % 2 != 0)
+						graph_line_addch(line, ' ');
+					truncated = 1;
+					break;
+				}
+
 				if (idx == 2) {
-					if (graph->edges_added > 0 || j < graph->num_parents - 1)
+					/*
+					 * Check if the next lane needs truncation
+					 * to avoid having the padding doubled
+					 */
+					if (graph_needs_truncation((j + 1) / 2 + i) &&
+					    j < graph->num_parents - 1) {
+						truncated = 1;
+						break;
+					} else if (graph->edges_added > 0 || j < graph->num_parents - 1)
 						graph_line_addch(line, ' ');
 				} else {
 					idx++;
 				}
 				parents = next_interesting_parent(graph, parents);
 			}
+			if (truncated)
+				break;
 			if (graph->edges_added == 0)
 				graph_line_addch(line, ' ');
-
+		} else if (graph_needs_truncation(i)) {
+			break;
 		} else if (seen_this) {
 			if (graph->edges_added > 0)
 				graph_line_write_column(line, col, '\\');
 			else
 				graph_line_write_column(line, col, '|');
-			graph_line_addch(line, ' ');
+			/*
+			 * If it's between two lanes and next would be truncated,
+			 * don't add space padding.
+			 */
+			if (!graph_needs_truncation(i + 1))
+				graph_line_addch(line, ' ');
 		} else {
 			graph_line_write_column(line, col, '|');
 			if (graph->merge_layout != 0 || i != graph->commit_index - 1) {
@@ -1164,6 +1263,7 @@ static void graph_output_collapsing_line(struct git_graph *graph, struct graph_l
 	short used_horizontal = 0;
 	int horizontal_edge = -1;
 	int horizontal_edge_target = -1;
+	int truncated = 0;
 
 	/*
 	 * Swap the mapping and old_mapping arrays
@@ -1279,26 +1379,34 @@ static void graph_output_collapsing_line(struct git_graph *graph, struct graph_l
 	 */
 	for (i = 0; i < graph->mapping_size; i++) {
 		int target = graph->mapping[i];
-		if (target < 0)
-			graph_line_addch(line, ' ');
-		else if (target * 2 == i)
-			graph_line_write_column(line, &graph->new_columns[target], '|');
-		else if (target == horizontal_edge_target &&
-			 i != horizontal_edge - 1) {
-				/*
-				 * Set the mappings for all but the
-				 * first segment to -1 so that they
-				 * won't continue into the next line.
-				 */
-				if (i != (target * 2)+3)
-					graph->mapping[i] = -1;
-				used_horizontal = 1;
-			graph_line_write_column(line, &graph->new_columns[target], '_');
+
+		if (!truncated && graph_needs_truncation(i / 2)) {
+			truncated = 1;
+		}
+
+		if (target < 0) {
+			if (!truncated)
+				graph_line_addch(line, ' ');
+		} else if (target * 2 == i) {
+			if (!truncated)
+				graph_line_write_column(line, &graph->new_columns[target], '|');
+		} else if (target == horizontal_edge_target &&
+			   i != horizontal_edge - 1) {
+			/*
+			 * Set the mappings for all but the
+			 * first segment to -1 so that they
+			 * won't continue into the next line.
+			 */
+			if (i != (target * 2)+3)
+				graph->mapping[i] = -1;
+			used_horizontal = 1;
+			if (!truncated)
+				graph_line_write_column(line, &graph->new_columns[target], '_');
 		} else {
 			if (used_horizontal && i < horizontal_edge)
 				graph->mapping[i] = -1;
-			graph_line_write_column(line, &graph->new_columns[target], '/');
-
+			if (!truncated)
+				graph_line_write_column(line, &graph->new_columns[target], '/');
 		}
 	}
 
@@ -1372,6 +1480,9 @@ static void graph_padding_line(struct git_graph *graph, struct strbuf *sb)
 	for (i = 0; i < graph->num_columns; i++) {
 		struct column *col = &graph->columns[i];
 
+		if (graph_needs_truncation(i))
+			break;
+
 		graph_line_write_column(&line, col, '|');
 
 		if (col->commit == graph->commit && graph->num_parents > 2) {
-- 
2.43.0

[GSoC PATCH v6 2/3] graph: add --graph-lane-limit option

[Pablo Sabater]

Replace the hard-coded lane limit with a user-facing
option '--graph-lane-limit=<n>'. It caps the number of
visible lanes to n. This option requires '--graph', without
it, limiting the graph has no meaning, in this case error out.

Zero and negative values are valid inputs but silently
ignored treating them as "no limit", the same as not using
the option. This follows what '--max-parents' does with
negative values.

The default is 0, same as not being used.

Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
 Documentation/rev-list-options.adoc |   5 +
 graph.c                             |  53 +++++-----
 revision.c                          |   6 ++
 revision.h                          |   1 +
 t/t4215-log-skewed-merges.sh        | 144 ++++++++++++++++++++++++++++
 5 files changed, 186 insertions(+), 23 deletions(-)

diff --git a/Documentation/rev-list-options.adoc b/Documentation/rev-list-options.adoc
index 2d195a1474..1b6ea89a63 100644
--- a/Documentation/rev-list-options.adoc
+++ b/Documentation/rev-list-options.adoc
@@ -1259,6 +1259,11 @@ This implies the `--topo-order` option by default, but the
 	in between them in that case. If _<barrier>_ is specified, it
 	is the string that will be shown instead of the default one.
 
+`--graph-lane-limit=<n>`::
+	When `--graph` is used, limit the number of graph lanes to be shown.
+	Lanes over the limit are not shown. By default it is set to 0 
+	(no limit), zero and negative values are ignored and treated as no limit.
+
 ifdef::git-rev-list[]
 `--count`::
 	Print a number stating how many commits would have been
diff --git a/graph.c b/graph.c
index 70458cf323..ee1f9e2d2d 100644
--- a/graph.c
+++ b/graph.c
@@ -82,8 +82,6 @@ static void graph_show_line_prefix(const struct diff_options *diffopt)
 static const char **column_colors;
 static unsigned short column_colors_max;
 
-static unsigned int max_lanes = 15;
-
 static void parse_graph_colors_config(struct strvec *colors, const char *string)
 {
 	const char *end, *start;
@@ -319,9 +317,13 @@ struct git_graph {
 	struct strbuf prefix_buf;
 };
 
-static inline int graph_needs_truncation(int lane)
+static inline int graph_needs_truncation(struct git_graph *graph, int lane)
 {
-	return lane >= max_lanes;
+	int max = graph->revs->graph_max_lanes;
+	/*
+	 * Ignore values <= 0, meaning no limit.
+	 */
+	return max > 0 && lane >= max;
 }
 
 static const char *diff_output_prefix_callback(struct diff_options *opt, void *data)
@@ -614,7 +616,7 @@ static void graph_update_columns(struct git_graph *graph)
 {
 	struct commit_list *parent;
 	int max_new_columns;
-	int i, seen_this, is_commit_in_columns, max;
+	int i, seen_this, is_commit_in_columns;
 
 	/*
 	 * Swap graph->columns with graph->new_columns
@@ -704,12 +706,17 @@ static void graph_update_columns(struct git_graph *graph)
 	}
 
 	/*
-	 * Cap to the hard-coded limit.
-	 * Allow commits from merges to align to the merged lane.
+	 *  If graph_max_lanes is set, cap the width
 	 */
-	max = max_lanes * 2 + 2;
-	if (graph->width > max)
-		graph->width = max;
+	if (graph->revs->graph_max_lanes > 0) {
+		/*
+		 * Width is column index while a lane is half that.
+		 * Allow commits from merges to align to the merged lane.
+		 */
+		int max_width = graph->revs->graph_max_lanes * 2 + 2;
+		if (graph->width > max_width)
+			graph->width = max_width;
+	}
 
 	/*
 	 * Shrink mapping_size to be the minimum necessary
@@ -861,7 +868,7 @@ static void graph_output_padding_line(struct git_graph *graph,
 	 * Output a padding row, that leaves all branch lines unchanged
 	 */
 	for (i = 0; i < graph->num_new_columns; i++) {
-		if (graph_needs_truncation(i))
+		if (graph_needs_truncation(graph, i))
 			break;
 		graph_line_write_column(line, &graph->new_columns[i], '|');
 		graph_line_addch(line, ' ');
@@ -920,7 +927,7 @@ static void graph_output_pre_commit_line(struct git_graph *graph,
 			seen_this = 1;
 			graph_line_write_column(line, col, '|');
 			graph_line_addchars(line, ' ', graph->expansion_row);
-		} else if (seen_this && graph_needs_truncation(i)) {
+		} else if (seen_this && graph_needs_truncation(graph, i)) {
 			break;
 		} else if (seen_this && (graph->expansion_row == 0)) {
 			/*
@@ -1018,7 +1025,7 @@ static void graph_draw_octopus_merge(struct git_graph *graph, struct graph_line
 		 * Commit is at commit_index, each iteration move one lane to
 		 * the right from the commit.
 		 */
-		if (graph_needs_truncation(graph->commit_index + 1 + i))
+		if (graph_needs_truncation(graph, graph->commit_index + 1 + i))
 			break;
 
 		graph_line_write_column(line, col, (i == dashed_parents - 1) ? '.' : '-');
@@ -1055,14 +1062,14 @@ static void graph_output_commit_line(struct git_graph *graph, struct graph_line
 			seen_this = 1;
 			graph_output_commit_char(graph, line);
 
-			if (graph_needs_truncation(i)) {
+			if (graph_needs_truncation(graph, i)) {
 				graph_line_addch(line, ' ');
 				break;
 			}
 
 			if (graph->num_parents > 2)
 				graph_draw_octopus_merge(graph, line);
-		} else if (graph_needs_truncation(i)) {
+		} else if (graph_needs_truncation(graph, i)) {
 			seen_this = 1;
 			break;
 		} else if (seen_this && (graph->edges_added > 1)) {
@@ -1112,7 +1119,7 @@ static void graph_output_commit_line(struct git_graph *graph, struct graph_line
 	 * padding lane.
 	 */
 	if (graph->num_parents > 1) {
-		if (!graph_needs_truncation(graph->commit_index)) {
+		if (!graph_needs_truncation(graph, graph->commit_index)) {
 			graph_update_state(graph, GRAPH_POST_MERGE);
 		} else {
 			struct commit_list *p = first_interesting_parent(graph);
@@ -1128,7 +1135,7 @@ static void graph_output_commit_line(struct git_graph *graph, struct graph_line
 
 			lane = graph_find_new_column_by_commit(graph, p->item);
 
-			if (!graph_needs_truncation(lane))
+			if (!graph_needs_truncation(graph, lane))
 				graph_update_state(graph, GRAPH_POST_MERGE);
 			else if (graph_is_mapping_correct(graph))
 				graph_update_state(graph, GRAPH_PADDING);
@@ -1192,7 +1199,7 @@ static void graph_output_post_merge_line(struct git_graph *graph, struct graph_l
 				 * comparable with i. Don't truncate if there are
 				 * no more lanes to print (end of the lane)
 				 */
-				if (graph_needs_truncation(j / 2 + i) &&
+				if (graph_needs_truncation(graph, j / 2 + i) &&
 				    j / 2 + i <= graph->num_columns) {
 					if ((j + i * 2) % 2 != 0)
 						graph_line_addch(line, ' ');
@@ -1205,7 +1212,7 @@ static void graph_output_post_merge_line(struct git_graph *graph, struct graph_l
 					 * Check if the next lane needs truncation
 					 * to avoid having the padding doubled
 					 */
-					if (graph_needs_truncation((j + 1) / 2 + i) &&
+					if (graph_needs_truncation(graph, (j + 1) / 2 + i) &&
 					    j < graph->num_parents - 1) {
 						truncated = 1;
 						break;
@@ -1220,7 +1227,7 @@ static void graph_output_post_merge_line(struct git_graph *graph, struct graph_l
 				break;
 			if (graph->edges_added == 0)
 				graph_line_addch(line, ' ');
-		} else if (graph_needs_truncation(i)) {
+		} else if (graph_needs_truncation(graph, i)) {
 			break;
 		} else if (seen_this) {
 			if (graph->edges_added > 0)
@@ -1231,7 +1238,7 @@ static void graph_output_post_merge_line(struct git_graph *graph, struct graph_l
 			 * If it's between two lanes and next would be truncated,
 			 * don't add space padding.
 			 */
-			if (!graph_needs_truncation(i + 1))
+			if (!graph_needs_truncation(graph, i + 1))
 				graph_line_addch(line, ' ');
 		} else {
 			graph_line_write_column(line, col, '|');
@@ -1380,7 +1387,7 @@ static void graph_output_collapsing_line(struct git_graph *graph, struct graph_l
 	for (i = 0; i < graph->mapping_size; i++) {
 		int target = graph->mapping[i];
 
-		if (!truncated && graph_needs_truncation(i / 2)) {
+		if (!truncated && graph_needs_truncation(graph, i / 2)) {
 			truncated = 1;
 		}
 
@@ -1480,7 +1487,7 @@ static void graph_padding_line(struct git_graph *graph, struct strbuf *sb)
 	for (i = 0; i < graph->num_columns; i++) {
 		struct column *col = &graph->columns[i];
 
-		if (graph_needs_truncation(i))
+		if (graph_needs_truncation(graph, i))
 			break;
 
 		graph_line_write_column(&line, col, '|');
diff --git a/revision.c b/revision.c
index 31808e3df0..81b67682a8 100644
--- a/revision.c
+++ b/revision.c
@@ -2605,6 +2605,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 	} else if (!strcmp(arg, "--no-graph")) {
 		graph_clear(revs->graph);
 		revs->graph = NULL;
+	} else if (skip_prefix(arg, "--graph-lane-limit=", &optarg)) {
+		revs->graph_max_lanes = parse_count(optarg);
 	} else if (!strcmp(arg, "--encode-email-headers")) {
 		revs->encode_email_headers = 1;
 	} else if (!strcmp(arg, "--no-encode-email-headers")) {
@@ -3172,6 +3174,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 
 	if (revs->no_walk && revs->graph)
 		die(_("options '%s' and '%s' cannot be used together"), "--no-walk", "--graph");
+
+	if (revs->graph_max_lanes > 0 && !revs->graph)
+		die(_("the option '%s' requires '%s'"), "--graph-lane-limit", "--graph");
+
 	if (!revs->reflog_info && revs->grep_filter.use_reflog_filter)
 		die(_("the option '%s' requires '%s'"), "--grep-reflog", "--walk-reflogs");
 
diff --git a/revision.h b/revision.h
index 69242ecb18..874ccce625 100644
--- a/revision.h
+++ b/revision.h
@@ -304,6 +304,7 @@ struct rev_info {
 
 	/* Display history graph */
 	struct git_graph *graph;
+	int graph_max_lanes;
 
 	/* special limits */
 	int skip_count;
diff --git a/t/t4215-log-skewed-merges.sh b/t/t4215-log-skewed-merges.sh
index 28d0779a8c..d7524e9366 100755
--- a/t/t4215-log-skewed-merges.sh
+++ b/t/t4215-log-skewed-merges.sh
@@ -370,4 +370,148 @@ test_expect_success 'log --graph with multiple tips' '
 	EOF
 '
 
+test_expect_success 'log --graph --graph-lane-limit=2 limited to two lanes' '
+	check_graph --graph-lane-limit=2 M_7 <<-\EOF
+	*-.   7_M4
+	|\ \
+	| | * 7_G
+	| | * 7_F
+	| *   7_E
+	| *   7_D
+	* |   7_C
+	| |/
+	|/|
+	* | 7_B
+	|/
+	* 7_A
+	EOF
+'
+
+test_expect_success 'log --graph --graph-lane-limit=1 truncate mid octopus merge' '
+	check_graph --graph-lane-limit=1 M_7 <<-\EOF
+	*-  7_M4
+	|\
+	|   7_G
+	|   7_F
+	| * 7_E
+	| * 7_D
+	*   7_C
+	|
+	|/
+	*   7_B
+	|/
+	* 7_A
+	EOF
+'
+
+test_expect_success 'log --graph --graph-lane-limit=3 limited to three lanes' '
+	check_graph --graph-lane-limit=3 M_1 M_3 M_5 M_7 <<-\EOF
+	*   7_M1
+	|\
+	| | *   7_M2
+	| | |\
+	| | | * 7_H
+	| | |   7_M3
+	| | |   7_J
+	| | |   7_I
+	| | |   7_M4
+	| |_|_
+	|/| |
+	| | |_
+	| |/|
+	| | |
+	| | |/
+	| | *   7_G
+	| | |
+	| | |/
+	| | *   7_F
+	| * |   7_E
+	| | |/
+	| |/|
+	| * |   7_D
+	| | |/
+	| |/|
+	* | | 7_C
+	| |/
+	|/|
+	* | 7_B
+	|/
+	* 7_A
+	EOF
+'
+
+test_expect_success 'log --graph --graph-lane-limit=6 check if it only shows first of 3 parent merge' '
+	check_graph --graph-lane-limit=6 M_1 M_3 M_5 M_7 <<-\EOF
+	*   7_M1
+	|\
+	| | *   7_M2
+	| | |\
+	| | | * 7_H
+	| | | | *   7_M3
+	| | | | |\
+	| | | | | * 7_J
+	| | | | * | 7_I
+	| | | | | | * 7_M4
+	| |_|_|_|_|/
+	|/| | | | |/
+	| | |_|_|/|
+	| |/| | | |/
+	| | | |_|/|
+	| | |/| | |
+	| | * | | | 7_G
+	| | | |_|/
+	| | |/| |
+	| | * | | 7_F
+	| * | | | 7_E
+	| | |/ /
+	| |/| |
+	| * | | 7_D
+	| | |/
+	| |/|
+	* | | 7_C
+	| |/
+	|/|
+	* | 7_B
+	|/
+	* 7_A
+	EOF
+'
+
+test_expect_success 'log --graph --graph-lane-limit=7 check if it shows all 3 parent merge' '
+	check_graph --graph-lane-limit=7 M_1 M_3 M_5 M_7 <<-\EOF
+	*   7_M1
+	|\
+	| | *   7_M2
+	| | |\
+	| | | * 7_H
+	| | | | *   7_M3
+	| | | | |\
+	| | | | | * 7_J
+	| | | | * | 7_I
+	| | | | | | *   7_M4
+	| |_|_|_|_|/|\
+	|/| | | | |/ /
+	| | |_|_|/| /
+	| |/| | | |/
+	| | | |_|/|
+	| | |/| | |
+	| | * | | | 7_G
+	| | | |_|/
+	| | |/| |
+	| | * | | 7_F
+	| * | | | 7_E
+	| | |/ /
+	| |/| |
+	| * | | 7_D
+	| | |/
+	| |/|
+	* | | 7_C
+	| |/
+	|/|
+	* | 7_B
+	|/
+	* 7_A
+	EOF
+'
+
 test_done
-- 
2.43.0

[GSoC PATCH v6 3/3] graph: add truncation mark to capped lanes

[Pablo Sabater]

When lanes are hidden by --graph-lane-limit, show a "~"
truncation mark, so users know that there are lanes
being truncated. The "~" is chosen because it is not
used elsewhere in the graph and it is discrete.

Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
 Documentation/rev-list-options.adoc |  5 ++-
 graph.c                             | 22 +++++++---
 t/t4215-log-skewed-merges.sh        | 64 ++++++++++++++---------------
 3 files changed, 52 insertions(+), 39 deletions(-)

diff --git a/Documentation/rev-list-options.adoc b/Documentation/rev-list-options.adoc
index 1b6ea89a63..937ffc6195 100644
--- a/Documentation/rev-list-options.adoc
+++ b/Documentation/rev-list-options.adoc
@@ -1261,8 +1261,9 @@ This implies the `--topo-order` option by default, but the
 
 `--graph-lane-limit=<n>`::
 	When `--graph` is used, limit the number of graph lanes to be shown.
-	Lanes over the limit are not shown. By default it is set to 0 
-	(no limit), zero and negative values are ignored and treated as no limit.
+	Lanes over the limit are replaced with a truncation mark '~'. 
+	By default it is set to 0 (no limit), zero and negative values
+	are ignored and treated as no limit.
 
 ifdef::git-rev-list[]
 `--count`::
diff --git a/graph.c b/graph.c
index ee1f9e2d2d..842282685f 100644
--- a/graph.c
+++ b/graph.c
@@ -706,11 +706,11 @@ static void graph_update_columns(struct git_graph *graph)
 	}
 
 	/*
-	 *  If graph_max_lanes is set, cap the width
+	 * If graph_max_lanes is set, cap the width
 	 */
 	if (graph->revs->graph_max_lanes > 0) {
 		/*
-		 * Width is column index while a lane is half that.
+		 * width of "| " per lanes plus truncation mark "~ ".
 		 * Allow commits from merges to align to the merged lane.
 		 */
 		int max_width = graph->revs->graph_max_lanes * 2 + 2;
@@ -868,8 +868,10 @@ static void graph_output_padding_line(struct git_graph *graph,
 	 * Output a padding row, that leaves all branch lines unchanged
 	 */
 	for (i = 0; i < graph->num_new_columns; i++) {
-		if (graph_needs_truncation(graph, i))
+		if (graph_needs_truncation(graph, i)) {
+			graph_line_addstr(line, "~ ");
 			break;
+		}
 		graph_line_write_column(line, &graph->new_columns[i], '|');
 		graph_line_addch(line, ' ');
 	}
@@ -928,6 +930,7 @@ static void graph_output_pre_commit_line(struct git_graph *graph,
 			graph_line_write_column(line, col, '|');
 			graph_line_addchars(line, ' ', graph->expansion_row);
 		} else if (seen_this && graph_needs_truncation(graph, i)) {
+			graph_line_addstr(line, "~ ");
 			break;
 		} else if (seen_this && (graph->expansion_row == 0)) {
 			/*
@@ -1025,8 +1028,10 @@ static void graph_draw_octopus_merge(struct git_graph *graph, struct graph_line
 		 * Commit is at commit_index, each iteration move one lane to
 		 * the right from the commit.
 		 */
-		if (graph_needs_truncation(graph, graph->commit_index + 1 + i))
+		if (graph_needs_truncation(graph, graph->commit_index + 1 + i)) {
+			graph_line_addstr(line, "~ ");
 			break;
+		}
 
 		graph_line_write_column(line, col, (i == dashed_parents - 1) ? '.' : '-');
 	}
@@ -1070,6 +1075,7 @@ static void graph_output_commit_line(struct git_graph *graph, struct graph_line
 			if (graph->num_parents > 2)
 				graph_draw_octopus_merge(graph, line);
 		} else if (graph_needs_truncation(graph, i)) {
+			graph_line_addstr(line, "~ ");
 			seen_this = 1;
 			break;
 		} else if (seen_this && (graph->edges_added > 1)) {
@@ -1203,6 +1209,7 @@ static void graph_output_post_merge_line(struct git_graph *graph, struct graph_l
 				    j / 2 + i <= graph->num_columns) {
 					if ((j + i * 2) % 2 != 0)
 						graph_line_addch(line, ' ');
+					graph_line_addstr(line, "~ ");
 					truncated = 1;
 					break;
 				}
@@ -1214,6 +1221,7 @@ static void graph_output_post_merge_line(struct git_graph *graph, struct graph_l
 					 */
 					if (graph_needs_truncation(graph, (j + 1) / 2 + i) &&
 					    j < graph->num_parents - 1) {
+						graph_line_addstr(line, "~ ");
 						truncated = 1;
 						break;
 					} else if (graph->edges_added > 0 || j < graph->num_parents - 1)
@@ -1228,6 +1236,7 @@ static void graph_output_post_merge_line(struct git_graph *graph, struct graph_l
 			if (graph->edges_added == 0)
 				graph_line_addch(line, ' ');
 		} else if (graph_needs_truncation(graph, i)) {
+			graph_line_addstr(line, "~ ");
 			break;
 		} else if (seen_this) {
 			if (graph->edges_added > 0)
@@ -1388,6 +1397,7 @@ static void graph_output_collapsing_line(struct git_graph *graph, struct graph_l
 		int target = graph->mapping[i];
 
 		if (!truncated && graph_needs_truncation(graph, i / 2)) {
+			graph_line_addstr(line, "~ ");
 			truncated = 1;
 		}
 
@@ -1487,8 +1497,10 @@ static void graph_padding_line(struct git_graph *graph, struct strbuf *sb)
 	for (i = 0; i < graph->num_columns; i++) {
 		struct column *col = &graph->columns[i];
 
-		if (graph_needs_truncation(graph, i))
+		if (graph_needs_truncation(graph, i)) {
+			graph_line_addstr(&line, "~ ");
 			break;
+		}
 
 		graph_line_write_column(&line, col, '|');
 
diff --git a/t/t4215-log-skewed-merges.sh b/t/t4215-log-skewed-merges.sh
index d7524e9366..1612f05f1b 100755
--- a/t/t4215-log-skewed-merges.sh
+++ b/t/t4215-log-skewed-merges.sh
@@ -376,9 +376,9 @@ test_expect_success 'log --graph --graph-lane-limit=2 limited to two lanes' '
 	|\ \
 	| | * 7_G
 	| | * 7_F
-	| *   7_E
-	| *   7_D
-	* |   7_C
+	| * ~ 7_E
+	| * ~ 7_D
+	* | ~ 7_C
 	| |/
 	|/|
 	* | 7_B
@@ -389,16 +389,16 @@ test_expect_success 'log --graph --graph-lane-limit=2 limited to two lanes' '
 
 test_expect_success 'log --graph --graph-lane-limit=1 truncate mid octopus merge' '
 	check_graph --graph-lane-limit=1 M_7 <<-\EOF
-	*-  7_M4
-	|\
-	|   7_G
-	|   7_F
+	*-~  7_M4
+	|\~
+	| ~ 7_G
+	| ~ 7_F
 	| * 7_E
 	| * 7_D
-	*   7_C
-	|
-	|/
-	*   7_B
+	* ~ 7_C
+	| ~
+	|/~
+	* ~ 7_B
 	|/
 	* 7_A
 	EOF
@@ -411,24 +411,24 @@ test_expect_success 'log --graph --graph-lane-limit=3 limited to three lanes' '
 	| | *   7_M2
 	| | |\
 	| | | * 7_H
-	| | |   7_M3
-	| | |   7_J
-	| | |   7_I
-	| | |   7_M4
-	| |_|_
-	|/| |
-	| | |_
-	| |/|
-	| | |
-	| | |/
-	| | *   7_G
-	| | |
-	| | |/
-	| | *   7_F
-	| * |   7_E
-	| | |/
-	| |/|
-	| * |   7_D
+	| | | ~ 7_M3
+	| | | ~ 7_J
+	| | | ~ 7_I
+	| | | ~ 7_M4
+	| |_|_~
+	|/| | ~
+	| | |_~
+	| |/| ~
+	| | | ~
+	| | |/~
+	| | * ~ 7_G
+	| | | ~
+	| | |/~
+	| | * ~ 7_F
+	| * | ~ 7_E
+	| | |/~
+	| |/| ~
+	| * | ~ 7_D
 	| | |/
 	| |/|
 	* | | 7_C
@@ -452,9 +452,9 @@ test_expect_success 'log --graph --graph-lane-limit=6 check if it only shows fir
 	| | | | | * 7_J
 	| | | | * | 7_I
 	| | | | | | * 7_M4
-	| |_|_|_|_|/
-	|/| | | | |/
-	| | |_|_|/|
+	| |_|_|_|_|/~
+	|/| | | | |/~
+	| | |_|_|/| ~
 	| |/| | | |/
 	| | | |_|/|
 	| | |/| | |
-- 
2.43.0