mooman219 / fontdue

The fastest font renderer in the world, written in pure rust.
Apache License 2.0
1.45k stars 72 forks source link

Code Formatting #46

Closed kirawi closed 3 years ago

kirawi commented 4 years ago

I hope this doesn't come off as snobbish or anything like that, but I find the code can be hard to read at times. For instance,

pub fn read_utf8(string: &str, byte_offset: &mut usize) -> char {
    let bytes = string.as_bytes();
    let x = bytes[*byte_offset];
    *byte_offset += 1;
    if x < 128 {
        return unsafe { core::char::from_u32_unchecked(x as u32) };
    }
    let init = (x & (0x7F >> 2)) as u32;
    let y = bytes[*byte_offset];
    *byte_offset += 1;
    let mut ch = utf8_acc_cont_byte(init, y);
    if x >= 0xE0 {
        let z = bytes[*byte_offset];
        *byte_offset += 1;
        let y_z = utf8_acc_cont_byte((y & CONT_MASK) as u32, z);
        ch = init << 12 | y_z;
        if x >= 0xF0 {
            let w = bytes[*byte_offset];
            *byte_offset += 1;
            ch = (init & 7) << 18 | utf8_acc_cont_byte(y_z, w);
        }
    }
    unsafe { core::char::from_u32_unchecked(ch) }
}

It's a bit hard to read at first glance because of a lack of newlines. I myself would've preferred,

pub fn read_utf8(string: &str, byte_offset: &mut usize) -> char {
    let bytes = string.as_bytes();
    let x = bytes[*byte_offset];
    *byte_offset += 1;

    if x < 128 {
        return unsafe { core::char::from_u32_unchecked(x as u32) };
    }

    let init = (x & (0x7F >> 2)) as u32;
    let y = bytes[*byte_offset];
    *byte_offset += 1;

    let mut ch = utf8_acc_cont_byte(init, y);

    if x >= 0xE0 {
        let z = bytes[*byte_offset];
        *byte_offset += 1;
        let y_z = utf8_acc_cont_byte((y & CONT_MASK) as u32, z);

        ch = init << 12 | y_z;

        if x >= 0xF0 {
            let w = bytes[*byte_offset];
            *byte_offset += 1;

            ch = (init & 7) << 18 | utf8_acc_cont_byte(y_z, w);
        }
    }
    unsafe { core::char::from_u32_unchecked(ch) }
}

Of course, this is an entirely subjective and pedantic issue and I apologise if I've offended you in some way, but I really think some degree of separation should be there. Thoughts?

mooman219 commented 3 years ago

I kinda just drew the line at whatever the auto-formatter likes will be enough. If you want to PR a formatting change to add some whitespace padding to these lines I'll be happy to merge it!