Refactor mutt_str_pretty_size() to use struct Buffer

[flatcap]

Overview

mutt_str_pretty_size() turns a large number into a "pretty" string, e.g, "1.3M"

Upgrading this function is part of a larger plan to use struct Buffer for most string handling.
struct Buffer is widely used throughout NeoMutt, so there're lots of examples to copy'n'paste.

See:

• struct Buffer

Notes:
The work should be done on top of branch devel/expando.
This branch affects several of the callers of mutt_str_pretty_size() and it will be merged soon.

Tasks

1. Change mutt_str_pretty_size() to take struct Buffer
   Inside the function, replace the string functions snprintf() etc with buf_add_printf(), etc

2. Some of the callers may need updating to use struct Buffer also.

3. Split mutt_str_pretty_size() into two functions.

   • A wrapper that does the config lookups
   • A simple function that takes flags and does the actual work

4. Move the simple work function into libmutt
   mutt/ contains utility functions that don't have any dependencies.

5. Write some simple tests for new libmutt function

---

All these tasks are fairly simply, but each requires a bit more NeoMutt knowledge than the last.
Have a look around, have a think, ask lots of questions (here, or on IRC).

Thanks!

[Pierre-Colin]

Hi. I noticed that mutt_str_pretty_size() doesn’t communicate errors. I think this refactoring is an appropriate time to address this. I propose this change for task 1. It returns the number of characters written, 0 meaning a (non-fatal) error occurred. It also includes updated Doxygen comments, though I haven’t checked whether they compile.

Patch

diff --git a/muttlib.c b/muttlib.c
index fefeb93..db54514 100644
--- a/muttlib.c
+++ b/muttlib.c
@@ -1094,47 +1094,63 @@ void buf_sanitize_filename(struct Buffer *buf, const char *path, short slash)
 
 /**
  * mutt_str_pretty_size - Display an abbreviated size, like 3.4K
- * @param buf    Buffer for the result
- * @param buflen Length of the buffer
- * @param num    Number to abbreviate
+ * @param buf  Buffer to write into
+ * @param num  Number to abbreviate
+ * @retval num Number of characters written
+ * @retval 0   Error
+ *
+ * Formats a number to abbreviate it with the metric prefixes K and M if
+ * needed. If necessary, the buffer is expanded. The following configuration
+ * flags affect the formatting:
+ *
+ * - `size_show_bytes`: no prefix is used if `num < 1024`.
+ *
+ * - `size_units_on_left`: the prefix precedes the number (e.g. K2 instead of
+ * 2K).
+ *
+ * - `size_show_fractions`: at most one decimal digit is written (e.g. 1.2K).
+ *
+ * - `size_show_mb`: enable the prefix M.
  */
-void mutt_str_pretty_size(char *buf, size_t buflen, size_t num)
+size_t mutt_str_pretty_size(struct Buffer *buf, size_t num)
 {
-  if (!buf || (buflen == 0))
-    return;
+  if (!buf)
+    return 0;
 
   const bool c_size_show_bytes = cs_subset_bool(NeoMutt->sub, "size_show_bytes");
   const bool c_size_show_fractions = cs_subset_bool(NeoMutt->sub, "size_show_fractions");
   const bool c_size_show_mb = cs_subset_bool(NeoMutt->sub, "size_show_mb");
   const bool c_size_units_on_left = cs_subset_bool(NeoMutt->sub, "size_units_on_left");
 
+  int r;
   if (c_size_show_bytes && (num < 1024))
   {
-    snprintf(buf, buflen, "%d", (int) num);
+    r = buf_add_printf(buf, "%zu", num);
   }
   else if (num == 0)
   {
-    mutt_str_copy(buf, c_size_units_on_left ? "K0" : "0K", buflen);
+    return buf_addstr(buf, c_size_units_on_left? "K0" : "0K");
   }
   else if (c_size_show_fractions && (num < 10189)) /* 0.1K - 9.9K */
   {
-    snprintf(buf, buflen, c_size_units_on_left ? "K%3.1f" : "%3.1fK",
-             (num < 103) ? 0.1 : (num / 1024.0));
+    r = buf_add_printf(buf, c_size_units_on_left ? "K%3.1f" : "%3.1fK",
+                       (num < 103) ? 0.1 : (num / 1024.0));
   }
   else if (!c_size_show_mb || (num < 1023949)) /* 10K - 999K */
   {
     /* 51 is magic which causes 10189/10240 to be rounded up to 10 */
-    snprintf(buf, buflen, c_size_units_on_left ? ("K%zu") : ("%zuK"), (num + 51) / 1024);
+    r = buf_add_printf(buf, c_size_units_on_left ? "K%zu" : "%zuK", (num + 51) / 1024);
   }
   else if (c_size_show_fractions && (num < 10433332)) /* 1.0M - 9.9M */
   {
-    snprintf(buf, buflen, c_size_units_on_left ? "M%3.1f" : "%3.1fM", num / 1048576.0);
+    r = buf_add_printf(buf, c_size_units_on_left ? "M%3.1f" : "%3.1fM", num / 1048576.0);
   }
   else /* 10M+ */
   {
     /* (10433332 + 52428) / 1048576 = 10 */
-    snprintf(buf, buflen, c_size_units_on_left ? ("M%zu") : ("%zuM"), (num + 52428) / 1048576);
+    r = buf_add_printf(buf, units_on_left ? "M%zu" : "%zuM", (num + 52428) / 1048576);
   }
+  return r * (r > 0);
 }
 
 /**

[Pierre-Colin]

My bad. It wasn’t my intent to solve the entire issue (hence why I didn’t go any further). I mostly wanted to point out that interface change that I believe should have been among the tasks. Maybe providing a diff was too much. I’ll let @reido2012 do the rest as I was going to all along.

[reido2012]

Thanks for the hints with your diff @Pierre-Colin. I'll take it from here 🙏🏾

@flatcap I appreciate the easy first task documentation and support! Given that the shelf life is short I will try to get the changes done as soon as I can.