Re: [PATCH 2/2] Rename lineno_width to decimal_width and export it

Zbigniew Jędrzejewski-Szmek [off-list ref] writes:

This function will be used in calculating diff --stat graph width.
The name is changed because the function works for any number.
The function is moved from builtins/blame.c to pager.c because it
will be used not only in builtins/blame.c.

I'd prefer to see description of preliminary changes phrased without
depending too heavily on things that hasn't happened when possible.
Making a generic helper function to count digits necessary to print a
cardinal number available to future callers is a good thing by itself,
even if the "dynamic --stat width computation" turned out to be a bad
idea for whatever reason (I am not saying it is a bad idea here).

Perhaps like this.

	Subject: make lineno_width() from blame reusable for others

	builtin/blame.c has a helper function to compute how many columns
	we need to show a line-number, whose implementation is reusable as
	a more generic helper function to count the number of columns
	necessary to show any cardinal number.

	Rename it to decimal_width(), move it to pager.c and export it for
        use by future callers.

And you can say something like "I'll be using this in 'diff --stat' in
later patches" after the three-dash line.

---

Sign-off before the three-dash line?

+/*
+ * How many columns do we need to show numbers in decimal?

s/numbers/this number/;

+ */
+int decimal_width(int number)

Don't we want to make the argument "unsigned number" instead?

+{
+	int i, width;
+
+	for (width = 1, i = 10; i <= number; width++)
+		i *= 10;
+	return width;
+}

builtin/blame.c has a helper function to compute how many columns we
need to show a line-number, whose implementation is reusable as a more
generic helper function to count the number of columns necessary to
show any cardinal number.

Rename it to decimal_width(), move it to pager.c and export it for use
by future callers. The argument type is changed to unsigned to underline
the fact that the function supports only non-negative numbers.

Signed-off-by: Zbigniew Jędrzejewski-Szmek <redacted>
---
I'll be using this in the 'diff --stat: use the full terminal width'
patch.

v2: - change arg type to unsigned
    - corrected commit message

 builtin/blame.c | 18 +++---------------
 cache.h         |  1 +
 pager.c         | 12 ++++++++++++
 3 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 5a67c20..f028e8a 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c

@@ -1829,18 +1829,6 @@ static int read_ancestry(const char *graft_file)
 }
 
 /*
- * How many columns do we need to show line numbers in decimal?
- */
-static int lineno_width(int lines)
-{
-	int i, width;
-
-	for (width = 1, i = 10; i <= lines; width++)
-		i *= 10;
-	return width;
-}
-
-/*
  * How many columns do we need to show line numbers, authors,
  * and filenames?
  */

@@ -1880,9 +1868,9 @@ static void find_alignment(struct scoreboard *sb, int *option)
 		if (largest_score < ent_score(sb, e))
 			largest_score = ent_score(sb, e);
 	}
-	max_orig_digits = lineno_width(longest_src_lines);
-	max_digits = lineno_width(longest_dst_lines);
-	max_score_digits = lineno_width(largest_score);
+	max_orig_digits = decimal_width(longest_src_lines);
+	max_digits = decimal_width(longest_dst_lines);
+	max_score_digits = decimal_width(largest_score);
 }
 
 /*

diff --git a/cache.h b/cache.h
index 2f30b3a..3504bcc 100644
--- a/cache.h
+++ b/cache.h

@@ -1176,6 +1176,7 @@ extern const char *pager_program;
 extern int pager_in_use(void);
 extern int pager_use_color;
 extern int term_columns(void);
+extern int decimal_width(unsigned number);
 
 extern const char *editor_program;
 extern const char *askpass_program;

diff --git a/pager.c b/pager.c
index e06cfa0..2e16a9c 100644
--- a/pager.c
+++ b/pager.c

@@ -153,3 +153,15 @@ int term_columns(void)
 #endif
 	return term_columns_at_startup;
 }
+
+/*
+ * How many columns do we need to show this number in decimal?
+ */
+int decimal_width(unsigned number)
+{
+	int i, width;
+
+	for (width = 1, i = 10; i <= number; width++)
+		i *= 10;
+	return width;
+}

-- 
1.7.9.3.g2429d.dirty