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

From: Johannes Sixt <[email-masked]>
Date: Wed, 25 Mar 2026 11:04:48 +0100

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