Re: [PATCH v2 2/3] Adjust stat width calculations to take --graph output into account

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



The recent change to compute the width of diff --stat did not take into
consideration the output from --graph. The consequence is that when both
options are used, e.g. in 'log --stat --graph', the lines are too long.

Adjust stat width calculations to take --graph output into account.

Prevent graph width of diff-stat from falling below minimum.

Signed-off-by: Lucian Poston <lucian.poston@xxxxxxxxx>
---
 diff.c |   72 ++++++++++++++++++++++++++++++++++++++++++++++-----------------
 1 files changed, 52 insertions(+), 20 deletions(-)

diff --git a/diff.c b/diff.c
index 377ec1e..31ba10c 100644
--- a/diff.c
+++ b/diff.c
@@ -1383,6 +1383,8 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 	int width, name_width, graph_width, number_width = 4, count;
 	const char *reset, *add_c, *del_c;
 	const char *line_prefix = "";
+	int line_prefix_length = 0;
+	int reserved_character_count;
 	int extra_shown = 0;
 	struct strbuf *msg = NULL;
 
@@ -1392,6 +1394,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 	if (options->output_prefix) {
 		msg = options->output_prefix(options, options->output_prefix_data);
 		line_prefix = msg->buf;
+		line_prefix_length = options->output_prefix_length;
 	}
 
 	count = options->stat_count ? options->stat_count : data->nr;
@@ -1427,37 +1430,46 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 	 * We have width = stat_width or term_columns() columns total.
 	 * We want a maximum of min(max_len, stat_name_width) for the name part.
 	 * We want a maximum of min(max_change, stat_graph_width) for the +- part.
-	 * We also need 1 for " " and 4 + decimal_width(max_change)
-	 * for " | NNNN " and one the empty column at the end, altogether
+	 * Each line needs space for the following characters:
+	 *   - 1 for the initial " "
+	 *   - 4 + decimal_width(max_change) for " | NNNN "
+	 *   - 1 for the empty column at the end,
+	 * Altogether, the reserved_character_count totals
 	 * 6 + decimal_width(max_change).
 	 *
-	 * If there's not enough space, we will use the smaller of
-	 * stat_name_width (if set) and 5/8*width for the filename,
-	 * and the rest for constant elements + graph part, but no more
-	 * than stat_graph_width for the graph part.
-	 * (5/8 gives 50 for filename and 30 for the constant parts + graph
-	 * for the standard terminal size).
+	 * Additionally, there may be a line_prefix, which reduces the available
+	 * width by line_prefix_length.
+	 *
+	 * If there's not enough space, we will use the smaller of stat_name_width
+	 * (if set) and 5/8*width for the filename, and the rest for the graph
+	 * part, but no more than stat_graph_width for the graph part.
+	 * Assuming the line prefix is empty, on a standard 80 column terminal
+	 * this ratio results in 50 characters for the filename and 20 characters
+	 * for the graph (plus the 10 reserved characters).
 	 *
 	 * In other words: stat_width limits the maximum width, and
 	 * stat_name_width fixes the maximum width of the filename,
 	 * and is also used to divide available columns if there
 	 * aren't enough.
 	 */
+	reserved_character_count = 6 + number_width;
 
 	if (options->stat_width == -1)
 		width = term_columns();
 	else
 		width = options->stat_width ? options->stat_width : 80;
 
+	width -= line_prefix_length;
+
 	if (options->stat_graph_width == -1)
 		options->stat_graph_width = diff_stat_graph_width;
 
 	/*
-	 * Guarantee 3/8*16==6 for the graph part
-	 * and 5/8*16==10 for the filename part
+	 * Guarantees at least 6 characters for the graph part [16 * 3/8]
+	 * and at least 10 for the filename part [16 * 5/8]
 	 */
-	if (width < 16 + 6 + number_width)
-		width = 16 + 6 + number_width;
+	if (width < 16 + reserved_character_count)
+		width = 16 + reserved_character_count;
 
 	/*
 	 * First assign sizes that are wanted, ignoring available width.
@@ -1472,16 +1484,36 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 	/*
 	 * Adjust adjustable widths not to exceed maximum width
 	 */
-	if (name_width + number_width + 6 + graph_width > width) {
-		if (graph_width > width * 3/8 - number_width - 6)
-			graph_width = width * 3/8 - number_width - 6;
+	if (reserved_character_count + name_width + graph_width > width) {
+		/*
+		 * Reduce graph_width to be at most 3/8 of the unreserved space and no
+		 * less than 6, which leaves at least 5/8 for the filename.
+		 */
+		if (graph_width > width * 3/8 - reserved_character_count) {
+			graph_width = width * 3/8 - reserved_character_count;
+			if (graph_width < 6) {
+				graph_width = 6;
+			}
+		}
+
+		/*
+		 * If the remaining unreserved space will not accomodate the
+		 * filenames, adjust name_width to use all available remaining space.
+		 * Otherwise, assign any extra space to graph_width.
+		 */
+		if (name_width > width - reserved_character_count - graph_width) {
+			name_width = width - reserved_character_count - graph_width;
+		} else {
+			graph_width = width - reserved_character_count - name_width;
+		}
+
+		/*
+		 * If stat-graph-width was specified, limit graph_width to its value.
+		 */
 		if (options->stat_graph_width &&
-		    graph_width > options->stat_graph_width)
+				graph_width > options->stat_graph_width) {
 			graph_width = options->stat_graph_width;
-		if (name_width > width - number_width - 6 - graph_width)
-			name_width = width - number_width - 6 - graph_width;
-		else
-			graph_width = width - number_width - 6 - name_width;
+		}
 	}
 
 	/*
-- 
1.7.3.4

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]