From f20c0552dd0f63f894272ad031a9908caec985e7 Mon Sep 17 00:00:00 2001
From: Guillaume Gielly <guillaume@dialup.fr>
Date: Mon, 23 Dec 2024 21:56:45 +0100
Subject: [PATCH] Enhance monetary formatting implementation and improve
 comments for clarity

---
 src/header/monetary/mod.rs     | 29 ++++++++++++++++-------------
 src/header/monetary/strfmon.rs |  9 ++++++---
 2 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/src/header/monetary/mod.rs b/src/header/monetary/mod.rs
index f2b4b4af23..2916ad256d 100644
--- a/src/header/monetary/mod.rs
+++ b/src/header/monetary/mod.rs
@@ -1,8 +1,7 @@
-
 /// monetary.h implementation for Redox, following the POSIX standard.
 /// Following https://pubs.opengroup.org/onlinepubs/9799919799/basedefs/monetary.h.html
 ///
-/// We should provide a strfmon() implementation that formats a monetary value.
+/// We should provide a strfmon() implementation that formats a monetary value,
 /// according to the current locale (TODO).
 use alloc::string::{String, ToString};
 use core::{ffi::CStr, ptr, slice, str};
@@ -62,7 +61,7 @@ struct FormatFlags {
 }
 
 /// Use our own floating point implementation
-/// TODO : use num-traits crate
+/// TODO : maybe we can use num-traits crate
 #[inline]
 fn my_fabs(x: f64) -> f64 {
     if x < 0.0 {
@@ -105,32 +104,36 @@ fn my_pow10(n: usize) -> f64 {
     result
 }
 
-/// TODO : improve grouping implementation
+/// Formats a monetary value according to the current locale.
 fn apply_grouping(int_str: &str, monetary: &LocaleMonetaryInfo) -> String {
     let mut grouped = String::with_capacity(int_str.len() * 2);
     let mut count = 0;
     let mut group_idx = 0;
+    let current_grouping = &monetary.mon_grouping;
+    let separator = monetary.mon_thousands_sep;
 
-    // The grouping array can have up to 4 elements, but the last element is always 0
-    for c in int_str.chars().rev() {
-        if count > 0 && count % monetary.mon_grouping[group_idx] == 0 {
-            grouped.push_str(monetary.mon_thousands_sep);
-            // Move to next grouping size if available
-            if group_idx + 1 < monetary.mon_grouping.len() {
-                group_idx += 1;
+    for c in int_str.chars() {
+        if count > 0 {
+            let current_group = current_grouping[group_idx.min(current_grouping.len() - 1)];
+            if current_group > 0 && count % current_group == 0 {
+                grouped.push_str(separator);
+                if group_idx + 1 < current_grouping.len() {
+                    group_idx += 1;
+                }
             }
         }
         grouped.push(c);
         count += 1;
     }
-    // Reverse the string to get the correct order
-    grouped.chars().rev().collect()
+
+    grouped
 }
 
 /// Safe handling of large monetary values. Returns None if the value is too large to format
 fn format_value_parts(value: f64, frac_digits: usize) -> Option<(String, i64)> {
     let abs_value = my_fabs(value);
     if abs_value > (i64::MAX as f64) {
+        // Check if the value is too large to format
         return None;
     }
 
diff --git a/src/header/monetary/strfmon.rs b/src/header/monetary/strfmon.rs
index 0d660e7733..3feda48182 100644
--- a/src/header/monetary/strfmon.rs
+++ b/src/header/monetary/strfmon.rs
@@ -12,9 +12,10 @@ use core::{ffi::CStr, ptr, slice, str};
    an optional left precision, an optional right precision, and a conversion specifier.
    The conversion specifier can be either 'i' or 'n'. The 'i' specifier formats the monetary value according to the
    locale-specific rules, while the 'n' specifier formats the monetary value according to the international rules.
-   
+
    The function returns the number of characters written to the buffer, excluding the null terminator, or -1 if an error occurred.
 
+   TODO : better handling error : always return -1 if an error occurred
 */
 pub unsafe extern "C" fn strfmon(
     s: *mut i8,        // char *
@@ -208,7 +209,7 @@ fn format_monetary(
     // Reverse the string to get the correct order
     let mut result = String::with_capacity(grouped.len() + 20);
 
-    // Add the sign
+    // Add the sign position
     let sign_posn = if is_negative {
         monetary.n_sign_posn
     } else {
@@ -226,7 +227,7 @@ fn format_monetary(
         ""
     };
 
-    // Add the sign
+    // Add the sign at the beginning
     if sign_posn == 0 {
         result.push('(');
     } else if sign_posn == 1 {
@@ -273,6 +274,7 @@ fn format_monetary(
     }
 
     if !cs_precedes {
+        // Add the currency symbol after the value
         if sep_by_space {
             result.push(' ');
         }
@@ -284,6 +286,7 @@ fn format_monetary(
     }
 
     if let Some(width) = flags.field_width {
+        // Check if the field width is specified
         if result.len() < width {
             let padding = " ".repeat(width - result.len());
             if flags.left_justify {