Repository: nushell/nushell
File: crates/nu-protocol/src/config/table.rs (Rust)

Code before:
                        if let Ok(count) = count.try_into() {
                            self.abbreviated_row_count = Some(count);
                        } else {
                            errors.invalid_value(path, "a non-negative integer", val);
                        }
                    }
                    _ => errors.type_mismatch(path, Type::custom("int or nothing"), val),
                },
                "footer_inheritance" => self.footer_inheritance.update(val, path, errors),
                "missing_value_symbol" => match val.as_str() {
                    Ok(val) => self.missing_value_symbol = val.to_string(),
                    Err(_) => errors.type_mismatch(path, Type::String, val),
                },
                "batch_duration" => match val.as_duration() {
                    Err(_) => errors.type_mismatch(path, Type::Duration, val),
                    Ok(..0) => errors.invalid_value(path, "a non-negative duration", val),
                    Ok(val) => self.batch_duration = Duration::from_nanos(val as u64),
                },
                "stream_page_size" => {
                    match val.as_int().map(|int| int.try_into().map(NonZeroU16::new)) {
                        Err(_) => errors.type_mismatch(path, Type::Int, val),
                        Ok(Err(_)) => errors.invalid_value(path, "a non-negative value", val),
                        Ok(Ok(None)) => errors.invalid_value(path, "a non-zero value", val),
                        Ok(Ok(Some(val))) => self.stream_page_size = val,
                    }
                }
                _ => errors.unknown_option(path, val),
            }
        }
    }
}


Review comment (suggestion):
I really don't like the nesting here, and separate messages for negative and zero seem redundant when we can "expect a positive value":
```suggestion
                "stream_page_size" => {
                    let Ok(n) = val.as_int() else {
                        errors.type_mismatch(path, Type::Int, val);
                        continue;
                    };
                    let Some(n) = u16::try_from(n).ok().and_then(NonZeroU16::new) else {
                        errors.invalid_value(path, "a positive value", val);
                        continue;
                    };
                    self.stream_page_size = n;
                }
```

Code after:
                            self.abbreviated_row_count = Some(count);
                        } else {
                            errors.invalid_value(path, "a non-negative integer", val);
                        }
                    }
                    _ => errors.type_mismatch(path, Type::custom("int or nothing"), val),
                },
                "footer_inheritance" => self.footer_inheritance.update(val, path, errors),
                "missing_value_symbol" => match val.as_str() {
                    Ok(val) => self.missing_value_symbol = val.to_string(),
                    Err(_) => errors.type_mismatch(path, Type::String, val),
                },
                "batch_duration" => {
                    match Duration::from_value(val.clone()).map_err(ConfigError::from) {
                        Ok(val) => self.batch_duration = val,
                        Err(err) => errors.error(err),
                    }
                }
                "stream_page_size" => {
                    let Ok(n) = val.as_int() else {
                        errors.type_mismatch(path, Type::Int, val);
                        continue;
                    };
                    let Some(n) = u16::try_from(n).ok().and_then(NonZeroU16::new) else {
                        errors.invalid_value(path, "a positive value", val);
                        continue;
                    };
                    self.stream_page_size = n;
                }
                _ => errors.unknown_option(path, val),
            }
        }
    }
}