ronantakizawa/codereview-bench huggingface dataset:
Repository: typst/typst
File: crates/typst/src/lib.rs (Rust)

Code before:
    let is_utf8_error = matches!(file_error, FileError::InvalidUtf8);
    let mut diagnostic =
        SourceDiagnostic::error(Span::detached(), EcoString::from(file_error));

    // Attempt to provide helpful hints for UTF-8 errors.
    // Perhaps the user mistyped the filename.
    // For example, they could have written "file.pdf" instead of
    // "file.typ".
    if is_utf8_error {
        let path = input.vpath().as_rootless_path();
        let extension = path.extension();

        if let Some(extension) = extension {
            if extension == "typ" {
                // No hints if the file is already a .typ file.
                // The file is indeed just invalid.
                return eco_vec![diagnostic];
            }

            diagnostic.hint(eco_format!(
                "a file with the `.{}` extension is not usually a Typst file",
                extension.to_string_lossy()
            ));
        };

        if path.with_extension("typ").exists() {
            diagnostic.hint("check if you meant to use the `.typ` extension instead");
        }
    }

    eco_vec![diagnostic]
}


Review comment (suggestion):
This is a problem. The core `typst` crate should never do FS itself. 

Code after:
    world: Tracked<dyn World + '_>,
    file_error: FileError,
    input: FileId,
) -> EcoVec<SourceDiagnostic> {
    let is_utf8_error = matches!(file_error, FileError::InvalidUtf8);
    let mut diagnostic =
        SourceDiagnostic::error(Span::detached(), EcoString::from(file_error));

    // Attempt to provide helpful hints for UTF-8 errors. Perhaps the user
    // mistyped the filename. For example, they could have written "file.pdf"
    // instead of "file.typ".
    if is_utf8_error {
        let path = input.vpath();
        let extension = path.as_rootless_path().extension();
        if extension.is_some_and(|extension| extension == "typ") {
            // No hints if the file is already a .typ file.
            // The file is indeed just invalid.
            return eco_vec![diagnostic];
        }

        match extension {
            Some(extension) => {
                diagnostic.hint(eco_format!(
                    "a file with the `.{}` extension is not usually a Typst file",
                    extension.to_string_lossy()
                ));
            }

            None => {
                diagnostic
                    .hint("a file without an extension is not usually a Typst file");
            }
        };

        if world.source(input.with_extension("typ")).is_ok() {
            diagnostic.hint("check if you meant to use the `.typ` extension instead");
        }
    }

    eco_vec![diagnostic]
}
Repository: typst/typst
File: crates/typst/src/visualize/polygon.rs (Rust)

Code before:
/// A closed polygon.
///
/// The polygon is defined by its corner points and is closed automatically.
///
/// # Example
/// ```example
/// #polygon(
///   fill: blue.lighten(80%),
///   stroke: blue,
///   (20%, 0pt),
///   (60%, 0pt),
///   (80%, 2cm),
///   (0%,  2cm),
/// )
/// ```
#[elem(scope, Show)]
pub struct PolygonElem {
    /// How to fill the polygon.
    ///
    /// When setting a fill, the default stroke disappears. To create a
    /// rectangle with both fill and stroke, you have to configure both.
    pub fill: Option<Paint>,

    /// The rule used to fill the polygon.
    /// Defaults to `non-zero`.
    pub fill_rule: Smart<FillRule>,

    /// How to [stroke] the polygon. This can be:
    ///
    /// Can be set to  `{none}` to disable the stroke or to `{auto}` for a
    /// stroke of `{1pt}` black if and if only if no fill is given.
    #[resolve]
    #[fold]
    pub stroke: Smart<Option<Stroke>>,

    /// The vertices of the polygon. Each point is specified as an array of two
    /// [relative lengths]($relative).
    #[variadic]
    pub vertices: Vec<Axes<Rel<Length>>>,
}

#[scope]
impl PolygonElem {
    /// A regular polygon, defined by its size and number of vertices.
    ///
    /// ```example
    /// #polygon.regular(
    ///   fill: blue.lighten(80%),
    ///   stroke: blue,
    ///   size: 30pt,
    ///   vertices: 3,

Review comment (suggestion):
I don't think this needs to be `Smart`. It can instead just provide a default via `#[default(FillRule::NonZero)]`.

Same for `PathElem`

Code after:
/// A closed polygon.
///
/// The polygon is defined by its corner points and is closed automatically.
///
/// # Example
/// ```example
/// #polygon(
///   fill: blue.lighten(80%),
///   stroke: blue,
///   (20%, 0pt),
///   (60%, 0pt),
///   (80%, 2cm),
///   (0%,  2cm),
/// )
/// ```
#[elem(scope, Show)]
pub struct PolygonElem {
    /// How to fill the polygon.
    ///
    /// When setting a fill, the default stroke disappears. To create a
    /// rectangle with both fill and stroke, you have to configure both.
    pub fill: Option<Paint>,

    /// The rule used to fill the polygon.
    #[default]
    pub fill_rule: FillRule,

    /// How to [stroke] the polygon. This can be:
    ///
    /// Can be set to  `{none}` to disable the stroke or to `{auto}` for a
    /// stroke of `{1pt}` black if and if only if no fill is given.
    #[resolve]
    #[fold]
    pub stroke: Smart<Option<Stroke>>,

    /// The vertices of the polygon. Each point is specified as an array of two
    /// [relative lengths]($relative).
    #[variadic]
    pub vertices: Vec<Axes<Rel<Length>>>,
}

#[scope]
impl PolygonElem {
    /// A regular polygon, defined by its size and number of vertices.
    ///
    /// ```example
    /// #polygon.regular(
    ///   fill: blue.lighten(80%),
    ///   stroke: blue,
    ///   size: 30pt,
    ///   vertices: 3,
Repository: typst/typst
File: crates/typst/src/foundations/repr.rs (Rust)

Code before:
//! Debug representation of values.

use ecow::{eco_format, EcoString};
use typst::utils::format::round_with_precision;

use crate::foundations::{func, Str, Value};

/// The Unicode minus sign.
pub const MINUS_SIGN: &str = "\u{2212}";

/// Returns the string representation of a value.
///
/// When inserted into content, most values are displayed as this representation
/// in monospace with syntax-highlighting. The exceptions are `{none}`,
/// integers, floats, strings, content, and functions.
///
/// **Note:** This function is for debugging purposes. Its output should not be
/// considered stable and may change at any time!
///
/// # Example
/// ```example
/// #none vs #repr(none) \
/// #"hello" vs #repr("hello") \
/// #(1, 2) vs #repr((1, 2)) \
/// #[*Hi*] vs #repr([*Hi*])
/// ```
#[func(title = "Representation")]
pub fn repr(
    /// The value whose string representation to produce.

Review comment (refactor):
This would probably be `crate::utils` instead (which would move this line under `use crate::foundations` below), though maybe we should just not import it at all and use the full path, since there's only one usage. Not sure what Laurenz thinks about this though.

Code after:
//! Debug representation of values.

use ecow::{eco_format, EcoString};

use crate::foundations::{func, Str, Value};
use crate::utils::format::round_with_precision;

/// The Unicode minus sign.
pub const MINUS_SIGN: &str = "\u{2212}";

/// Returns the string representation of a value.
///
/// When inserted into content, most values are displayed as this representation
/// in monospace with syntax-highlighting. The exceptions are `{none}`,
/// integers, floats, strings, content, and functions.
///
/// **Note:** This function is for debugging purposes. Its output should not be
/// considered stable and may change at any time!
///
/// # Example
/// ```example
/// #none vs #repr(none) \
/// #"hello" vs #repr("hello") \
/// #(1, 2) vs #repr((1, 2)) \
/// #[*Hi*] vs #repr([*Hi*])
/// ```
#[func(title = "Representation")]
pub fn repr(
    /// The value whose string representation to produce.
Repository: typst/typst
File: crates/typst-utils/src/format.rs (Rust)

Code before:
//! Different formatting functions.

use std::time::Duration;

/// Returns value with `n` digits after floating point where `n` is `precision`.
/// Standard rounding rule applies (if `n+1`th digit >= 5 then round up).
/// If `value` is +/- infinity or NaN returns `value`.
pub fn round_with_precision(value: f64, precision: u8) -> f64 {
    if value.is_infinite()
        || value.is_nan()
        // Integers that big can't have fractional part (mantissa is already full).
        || value.abs() >= (1_i64 << 53) as f64
        // Binary64 format can only precisely represent up to log_10(2^53) digits.
        || precision >= 17
    {
        return value;
    }
    let offset = 10_f64.powi(precision.into());
    if !(value * offset).is_finite() {
        return value;
    }
    (value * offset).round() / offset
}

/// Returns number of `(days, hours, minutes, seconds, milliseconds, microseconds)`.
fn get_duration_parts(duration: &Duration) -> (u16, u8, u8, u8, u16, u16) {
    // In practice we probably don't need nanoseconds.
    let micros = duration.as_micros();
    let (millis, micros) = (micros / 1000, (micros % 1000) as u16);
    let (sec, millis) = (millis / 1000, (millis % 1000) as u16);
    let (mins, sec) = (sec / 60, (sec % 60) as u8);
    let (hours, mins) = (mins / 60, (mins % 60) as u8);

Review comment (performance):
For consistency with the comment style elsewhere (also improved wording a bit).

```suggestion
/// Returns value with `n` digits after floating point where `n` is `precision`.
/// Standard rounding rules apply (if `n+1`th digit >= 5, round up).
///
/// If rounding `value` would have no possible effect (e.g. it's infinite or
/// NaN), returns `value` unchanged.
```

Code after:
//! Different formatting functions.

use std::time::Duration;

/// Returns value with `n` digits after floating point where `n` is `precision`.
/// Standard rounding rules apply (if `n+1`th digit >= 5, round up).
///
/// If rounding the `value` will have no effect (e.g., it's infinite or NaN),
/// returns `value` unchanged.
///
/// # Examples
///
/// ```
/// # use typst_utils::format::round_with_precision;
/// let rounded = round_with_precision(-0.56553, 2);
/// assert_eq!(-0.57, rounded);
/// ```
pub fn round_with_precision(value: f64, precision: u8) -> f64 {
    // Don't attempt to round the float if that wouldn't have any effect.
    // This includes infinite or NaN values, as well as integer values
    // with a filled mantissa (which can't have a fractional part).
    // Rounding with a precision larger than the amount of digits that can be
    // effectively represented would also be a no-op. Given that, the check
    // below ensures we won't proceed if `|value| >= 2^53` or if
    // `precision >= 15`, which also ensures the multiplication by `offset`
    // won't return `inf`, since `2^53 * 10^15` (larger than any possible
    // `value * offset` multiplication) does not.
    if value.is_infinite()
        || value.is_nan()
        || value.abs() >= (1_i64 << f64::MANTISSA_DIGITS) as f64
        || precision as u32 >= f64::DIGITS
    {
Repository: typst/typst
File: crates/typst-utils/src/format.rs (Rust)

Code before:
//! Different formatting functions.

use std::time::Duration;

/// Returns value with `n` digits after floating point where `n` is `precision`.
/// Standard rounding rules apply (if `n+1`th digit >= 5, round up).
///
/// If rounding the `value` will have no effect (e.g., it's infinite or
/// NaN), returns `value` unchanged.
///
/// # Examples
///
/// ```
/// # use typst_utils::format::round_with_precision;
/// let rounded = round_with_precision(-0.56553, 2);
/// assert_eq!(-0.57, rounded);
/// ```
pub fn round_with_precision(value: f64, precision: u8) -> f64 {
    // Don't attempt to round the float if that wouldn't have any effect. This
    // includes infinite or NaN values, as well as integer values
    // with a filled mantissa (which can't have a fractional part). Rounding
    // with a precision larger than the amount of digits that can be
    // effectively represented would also be a no-op.
    if value.is_infinite()
        || value.is_nan()
        || value.abs() >= (1_i64 << f64::MANTISSA_DIGITS) as f64
        || precision as u32 >= f64::DIGITS
    {
        return value;
    }
    let offset = 10_f64.powi(precision.into());
    assert!((value * offset).is_finite(), "{value} * {offset} is not finite!");
    (value * offset).round() / offset
}

Review comment (suggestion):
Should reflow here to fill the 80 columns (it's possible this may be necessary elsewhere too)

```suggestion
/// If rounding the `value` will have no effect (e.g., it's infinite or NaN),
/// returns `value` unchanged.
```

Code after:
//! Different formatting functions.

use std::time::Duration;

/// Returns value with `n` digits after floating point where `n` is `precision`.
/// Standard rounding rules apply (if `n+1`th digit >= 5, round up).
///
/// If rounding the `value` will have no effect (e.g., it's infinite or NaN),
/// returns `value` unchanged.
///
/// # Examples
///
/// ```
/// # use typst_utils::format::round_with_precision;
/// let rounded = round_with_precision(-0.56553, 2);
/// assert_eq!(-0.57, rounded);
/// ```
pub fn round_with_precision(value: f64, precision: u8) -> f64 {
    // Don't attempt to round the float if that wouldn't have any effect.
    // This includes infinite or NaN values, as well as integer values
    // with a filled mantissa (which can't have a fractional part).
    // Rounding with a precision larger than the amount of digits that can be
    // effectively represented would also be a no-op. Given that, the check
    // below ensures we won't proceed if `|value| >= 2^53` or if
    // `precision >= 15`, which also ensures the multiplication by `offset`
    // won't return `inf`, since `2^53 * 10^15` (larger than any possible
    // `value * offset` multiplication) does not.
    if value.is_infinite()
        || value.is_nan()
        || value.abs() >= (1_i64 << f64::MANTISSA_DIGITS) as f64
        || precision as u32 >= f64::DIGITS
    {
        return value;
    }
Repository: typst/typst
File: crates/typst/src/layout/repeat.rs (Rust)

Code before:
/// ```example
/// Sign on the dotted line:
/// #box(width: 1fr, repeat[.])
///
/// #set text(10pt)
/// #v(8pt, weak: true)
/// #align(right)[
///   Berlin, the 22nd of December, 2022
/// ]
/// ```
#[elem(Show)]
pub struct RepeatElem {
    /// The content to repeat.
    #[required]
    pub body: Content,

    /// The gap between each instance of the body.
    #[default]
    pub gap: Length,

    /// Whether to increase the gap between instances to completely fill the
    /// available space.
    ///
    /// When `{auto}`, inherits the paragraph's [`justify`]($par.justify).
    #[default(Smart::Custom(true))]
    pub justify: Smart<bool>,
}

impl Show for Packed<RepeatElem> {
    fn show(&self, _: &mut Engine, _: StyleChain) -> SourceResult<Content> {
        Ok(BlockElem::multi_layouter(self.clone(), layout_repeat)
            .pack()
            .spanned(self.span()))
    }
}

/// Layout the repeated content.
#[typst_macros::time(span = elem.span())]
fn layout_repeat(
    elem: &Packed<RepeatElem>,
    engine: &mut Engine,
    locator: Locator,
    styles: StyleChain,
    regions: Regions,
) -> SourceResult<Fragment> {
    let pod = Regions::one(regions.size, Axes::new(false, false));
    let piece = elem.body().layout(engine, locator, styles, pod)?.into_frame();
    let size = Size::new(regions.size.x, piece.height());

    if !size.is_finite() {
        bail!(elem.span(), "repeat with no size restrictions");

Review comment (suggestion):
I'm not sure whether inheriting from `par.justify` is really good. I think `repeat` is typically used to produce a very specific layout and it changing up randomly based on whether the user uses justification could produce unintended results.

Code after:
/// Sign on the dotted line:
/// #box(width: 1fr, repeat[.])
///
/// #set text(10pt)
/// #v(8pt, weak: true)
/// #align(right)[
///   Berlin, the 22nd of December, 2022
/// ]
/// ```
#[elem(Show)]
pub struct RepeatElem {
    /// The content to repeat.
    #[required]
    pub body: Content,

    /// The gap between each instance of the body.
    #[default]
    pub gap: Length,

    /// Whether to increase the gap between instances to completely fill the
    /// available space.
    #[default(true)]
    pub justify: bool,
}

impl Show for Packed<RepeatElem> {
    fn show(&self, _: &mut Engine, _: StyleChain) -> SourceResult<Content> {
        Ok(BlockElem::single_layouter(self.clone(), layout_repeat)
            .pack()
            .spanned(self.span()))
    }
}

/// Layout the repeated content.
#[typst_macros::time(span = elem.span())]
fn layout_repeat(
    elem: &Packed<RepeatElem>,
    engine: &mut Engine,
    locator: Locator,
    styles: StyleChain,
    region: Region,
) -> SourceResult<Frame> {
    let pod = Regions::one(region.size, Axes::new(false, false));
    let piece = elem.body().layout(engine, locator, styles, pod)?.into_frame();
    let size = Size::new(region.size.x, piece.height());

    if !size.is_finite() {
        bail!(elem.span(), "repeat with no size restrictions");
    }

    let mut frame = Frame::soft(size);
Repository: typst/typst
File: crates/typst/src/text/mod.rs (Rust)

Code before:
    }
}

/// A stylistic set in a font.
#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)]
pub struct StylisticSet(u8);

impl StylisticSet {
    /// Create a new set, clamping to 1-20.
    pub fn new(index: u8) -> Self {
        Self(index.clamp(1, 20))
    }

    /// Get the value, guaranteed to be 1-20.
    pub fn get(self) -> u8 {
        self.0
    }
}

cast! {
    StylisticSet,
    self => self.0.into_value(),
    v: i64 => match v {
        1 ..= 20 => Self::new(v as u8),
        _ => bail!("stylistic set must be between 1 and 20"),
    },
}

#[derive(Debug, Copy, Clone, Default, Eq, PartialEq, Hash)]
pub struct StylisticSets(u32);

impl StylisticSets {
    pub fn into_array(self) -> Array {
        self.sets().map(IntoValue::into_value).collect()
    }

    pub fn has(self, ss: u8) -> bool {
        self.0 & (1 << (ss as u32)) != 0
    }

    pub fn sets(self) -> impl Iterator<Item = u8> {
        (1..=20).filter(move |i| self.has(*i))
    }
}

cast! {
    StylisticSets,
    self => self.into_array().into_value(),
    v: i64 => match v {
        1 ..= 20 => Self(1 << (v as u32)),
        _ => bail!("stylistic set must be between 1 and 20"),

Review comment (suggestion):
I think we should retain the `none` option to not unnecessarily make this a breaking change. You can just add a cast from `NoneValue` to the empty set.

Code after:
    }
}

/// A set of stylistic sets to enable.
#[derive(Debug, Copy, Clone, Default, Eq, PartialEq, Hash)]
pub struct StylisticSets(u32);

impl StylisticSets {
    /// Converts this set into a Typst array of values.
    pub fn into_array(self) -> Array {
        self.sets().map(IntoValue::into_value).collect()
    }

    /// Returns whether this set contains a particular stylistic set.
    pub fn has(self, ss: u8) -> bool {
        self.0 & (1 << (ss as u32)) != 0
    }

    /// Returns an iterator over all stylistic sets to enable.
    pub fn sets(self) -> impl Iterator<Item = u8> {
        (1..=20).filter(move |i| self.has(*i))
    }
}

cast! {
    StylisticSets,
    self => self.into_array().into_value(),
    _: NoneValue => Self(0),
    v: i64 => match v {
        1 ..= 20 => Self(1 << (v as u32)),
        _ => bail!("stylistic set must be between 1 and 20"),
    },
    v: Vec<i64> => {
        let mut flags = 0;
        for i in v {
            match i {
                1 ..= 20 => flags |= 1 << (i as u32),
                _ => bail!("stylistic set must be between 1 and 20"),
            }
        }
        Self(flags)
    },
}

/// Which kind of numbers / figures to select.
#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash, Cast)]
pub enum NumberType {
    /// Numbers that fit well with capital text (the OpenType `lnum`
    /// font feature).
    Lining,
    /// Numbers that fit well into a flow of upper- and lowercase text (the
Repository: typst/typst
File: crates/typst/src/foundations/float.rs (Rust)

Code before:
    pub fn construct(
        /// The value that should be converted to a float.
        value: ToFloat,
    ) -> f64 {
        value.0
    }

    /// Checks if a float is not a number.
    ///
    /// In IEEE 754, more than one bit pattern represents a NaN. This function
    /// returns `true` if the float is any of those bit patterns.
    ///
    /// ```example
    /// #float.is-nan(0) \
    /// #float.is-nan(1) \
    /// #float.is-nan(calc.nan)
    /// ```
    #[func]
    pub fn is_nan(self) -> bool {
        f64::is_nan(self)
    }

    /// Checks if a float is infinite.
    ///
    /// Floats can represent positive and negative infinity. This function
    /// returns `{true}` if the float is either an infinity.
    ///
    /// ```example
    /// #float.is-infinite(0) \
    /// #float.is-infinite(1) \
    /// #float.is-infinite(calc.inf)
    /// ```
    #[func]
    pub fn is_infinite(self) -> bool {
        f64::is_infinite(self)
    }

    /// Calculates the sign of a floating point number.
    ///
    /// - If the number is positive (including `{+0.0}`), returns `{1.0}`.
    /// - If the number is negative (including `{-0.0}`), returns `{-1.0}`.
    /// - If the number is `{calc.nan}`, returns `{calc.nan}`.
    ///
    /// ```example
    /// #(5.0).signum() \
    /// #(-5.0).signum() \
    /// #(0.0).signum() \
    /// #calc.nan.signum()
    /// ```
    #[func]
    pub fn signum(self) -> f64 {

Review comment (bug):
the `an` change doesn't make sense and is also incorrect grammar. leaving it as it was is fine

Code after:
    #[func(constructor)]
    pub fn construct(
        /// The value that should be converted to a float.
        value: ToFloat,
    ) -> f64 {
        value.0
    }

    /// Checks if a float is not a number.
    ///
    /// In IEEE 754, more than one bit pattern represents a NaN. This function
    /// returns `true` if the float is any of those bit patterns.
    ///
    /// ```example
    /// #float.is-nan(0) \
    /// #float.is-nan(1) \
    /// #float.is-nan(calc.nan)
    /// ```
    #[func]
    pub fn is_nan(self) -> bool {
        f64::is_nan(self)
    }

    /// Checks if a float is infinite.
    ///
    /// Floats can represent positive infinity and negative infinity. This
    /// function returns `{true}` if the float is an infinity.
    ///
    /// ```example
    /// #float.is-infinite(0) \
    /// #float.is-infinite(1) \
    /// #float.is-infinite(calc.inf)
    /// ```
    #[func]
    pub fn is_infinite(self) -> bool {
        f64::is_infinite(self)
    }

    /// Calculates the sign of a floating point number.
    ///
    /// - If the number is positive (including `{+0.0}`), returns `{1.0}`.
    /// - If the number is negative (including `{-0.0}`), returns `{-1.0}`.
    /// - If the number is `{calc.nan}`, returns `{calc.nan}`.
    ///
    /// ```example
    /// #(5.0).signum() \
    /// #(-5.0).signum() \
    /// #(0.0).signum() \
    /// #calc.nan.signum()
    /// ```
    #[func]