Skip to content

Add padding size to Pad::Zero or Pad::Space #1663

Open

Description

@BigPapa314

The uu_date command in coreutils uses chrono to parse the format string. In #7334 there is a report that it is needed to add the amount of padding to the format string.

current behavior

let format_items = StrftimeItems::new("+%03d"); will yield Item::Error.

expected behavior

let format_items = StrftimeItems::new("+%03d"); should yield Item::Numeric(Numeric::Day, Pad::Zero(3)).
And in format_numeric() replace the hard coded 1 char padding to the value in Pad::Space or Pad::Zero.

Activity

  1. djc commented on Feb 24, 2025

    @djc
    Member

    Pad is public API and I don't want to take a semver-incompatible change for this -- would be open to accomodate this use case within the contraints.

  2. BigPapa314 commented on Feb 24, 2025

    @BigPapa314
    Author

    Ahh I missed that.

    Can you imagine a way to attach the padding width to the Item::Numeric enum without breaking the public API?

  3. djc commented on Feb 24, 2025

    @djc
    Member

    Well, Numeric is declared as #[non_exhaustive] at least, so it's probably possible to smuggle something in there?

  4. BigPapa314 commented on Feb 24, 2025

    @BigPapa314
    Author

    Hmmm. We could add something like

    enum Numeric {
        //...
        Padded { numeric: Numeric, pad_count: usize }
    }

    But that's a hack.

    Should I make a proposal or should we wait for 1.0 to add it to the Pad enum.

  5. djc commented on Feb 25, 2025

    @djc
    Member

    I guess we could try whether we can make Pad #[non_exhaustive] (and add a new variant) in a semver-compatible release. It's technically not compatible, but breakage might be pretty rare in practice.

  6. BigPapa314 commented on Feb 25, 2025

    @BigPapa314
    Author

    Hmm, so what are the implications for dependent code:

    1. The Numeric / Pad enums are retrieved from chrono and forwarded to chrono again. Like in uu_date.
      • There is no impact at all no mater if we add a new variant or if we modify the existing one.
    2. The enums are manually created and forwarded to chrono.
      • There would be no impact if we use the #[non_exhaustive] option.
      • There would be compile errors if we modify the current Pad and developers might need to init the width information.
        I guess there is no default initializer that we could use to init the width automatically to one.
    3. The enums are retrieved from chrono and handled in the dependent code.
      • The match statements will break without default branch when we add the #[non_exhaustive].
        The dependent code has to add the new variants with the width field.
      • The match statements might not break when we add the width field to the current Variant.
        This might keep the current behavior without taking respect to the width information.

    So it seams if we go the #[non_exhaustive] route there is only one case that is not breaking.

    What is your experience / opinion here? Did i miss something?

  7. djc commented on Feb 26, 2025

    @djc
    Member

    I don't understand the way you wrote this up.

  8. BigPapa314 commented on Feb 26, 2025

    @BigPapa314
    Author

    To sum it up. I'm not sure if the additional complexity of more Pad Variants is justified by the slightly lower breakage potential. What do you think?

  9. djc commented on Feb 28, 2025

    @djc
    Member

    Okay, I did a code search and I don't think we can make Pad non_exhaustive or add a variant to it without breaking a bunch of code. So I think we have to stick with adding a new Numeric variant for this.

    I don't know when chrono 0.5 will be released, but I know it won't happen in the near future, so I don't think waiting for a semver-incompatible release is a good option.

  10. BigPapa314 commented on Mar 3, 2025

    @BigPapa314
    Author

    Ok. I'll make a proposal.

  11. BigPapa314 commented on Mar 9, 2025

    @BigPapa314
    Author

    What do you think about the proposal?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions