Repository: microsoft/edit
File: src/icu.rs (Rust)

Code before:
use crate::unicode::Utf8Chars;
use crate::{apperr, arena_format, sys};

static mut ENCODINGS: Vec<&'static str> = Vec::new();

/// Returns a list of encodings ICU supports.
pub fn get_available_encodings() -> &'static [&'static str] {
    // OnceCell for people that want to put it into a static.
    #[allow(static_mut_refs)]
    unsafe {
        if ENCODINGS.is_empty() {
            if let Ok(f) = init_if_needed() {
                let mut n = 0;
                loop {
                    let name = (f.ucnv_getAvailableName)(n);
                    if name.is_null() {
                        break;
                    }

                    let name = CStr::from_ptr(name).to_str().unwrap_unchecked();
                    ENCODINGS.push(name);

                    // Push the encoding for UTF-8 BOM separately since ICU does not distinguish it from the regular UTF-8 encoding.
                    if name == "UTF-8" {
                        ENCODINGS.push("UTF-8 BOM");
                    }

                    n += 1;
                }
            }

            if ENCODINGS.is_empty() {
                ENCODINGS.push("UTF-8");
            }
        }
        &ENCODINGS
    }
}

/// Formats the given ICU error code into a human-readable string.
pub fn apperr_format(f: &mut std::fmt::Formatter<'_>, code: u32) -> std::fmt::Result {
    fn format(code: u32) -> &'static str {
        let Ok(f) = init_if_needed() else {
            return "";
        };

        let status = icu_ffi::UErrorCode::new(code);
        let ptr = unsafe { (f.u_errorName)(status) };
        if ptr.is_null() {
            return "";
        }

Review comment (suggestion):
I think we should flip this code. At the bottom of this function you can see:
```rs
if ENCODINGS.is_empty() {
    ENCODINGS.push("UTF-8");
}
```
but it should now be:
```rs
if ENCODINGS.is_empty() {
    ENCODINGS.push("UTF-8");
    ENCODINGS.push("UTF-8 BOM");
}
```
This seems quite redundant then with your new logic, so why not do this?
```rs
// Unconditionally push these two into the beginning of the list.
ENCODINGS.push("UTF-8");
ENCODINGS.push("UTF-8 BOM");

if let Ok(f) = init_if_needed() {
    loop {
        // ...
        if name != "UTF-8" { // <-- now it filters
            ENCODINGS.push(name);
        }
        // ...
    }
}
```

Code after:
use crate::unicode::Utf8Chars;
use crate::{apperr, arena_format, sys};

static mut ENCODINGS: Vec<&'static str> = Vec::new();

/// Returns a list of encodings ICU supports.
pub fn get_available_encodings() -> &'static [&'static str] {
    // OnceCell for people that want to put it into a static.
    #[allow(static_mut_refs)]
    unsafe {
        if ENCODINGS.is_empty() {
            ENCODINGS.push("UTF-8");
            ENCODINGS.push("UTF-8 BOM");

            if let Ok(f) = init_if_needed() {
                let mut n = 0;
                loop {
                    let name = (f.ucnv_getAvailableName)(n);
                    if name.is_null() {
                        break;
                    }

                    let name = CStr::from_ptr(name).to_str().unwrap_unchecked();
                    // We have already pushed UTF-8 above.
                    // There is no need to filter UTF-8 BOM here, since ICU does not distinguish it from UTF-8.
                    if name != "UTF-8" {
                        ENCODINGS.push(name);
                    }

                    n += 1;
                }
            }
        }
        &ENCODINGS
    }
}

/// Formats the given ICU error code into a human-readable string.
pub fn apperr_format(f: &mut std::fmt::Formatter<'_>, code: u32) -> std::fmt::Result {
    fn format(code: u32) -> &'static str {
        let Ok(f) = init_if_needed() else {
            return "";
        };

        let status = icu_ffi::UErrorCode::new(code);
        let ptr = unsafe { (f.u_errorName)(status) };
        if ptr.is_null() {
            return "";
        }

        let str = unsafe { CStr::from_ptr(ptr) };