Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add datetime value parser #1723

Merged
merged 3 commits into from
Feb 16, 2023
Merged

Add datetime value parser #1723

merged 3 commits into from
Feb 16, 2023

Conversation

bim9262
Copy link
Collaborator

@bim9262 bim9262 commented Feb 11, 2023

Resolves #1708 (well will)

I added a new parsers for the current time.

Issues to resolve
* icon doesn't work
* Instead of parsing the format each iteration it seems that a StrftimeItems could be used, but the lifetimes eluded me so far.

@MaxVerevkin
Copy link
Collaborator

Instead of parsing the format each iteration it seems that a StrftimeItems could be used, but the lifetimes eluded me so far.

Ah, self referential structs... AFAIK there are a few approaches: use some library that will do some magic, leak the string to get static lifetime, write a wrapper type (like StrftimeItemsOwned) with a bit of unsafe which will "leak" (Box::into_raw) string on creation and will free to on drop (Box::from_raw). I'm okay with either the first or third option.

@MaxVerevkin
Copy link
Collaborator

I think this is correct, as long as it is in its own module: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=ffad275ea8783f05068a850ac351195c

@bim9262
Copy link
Collaborator Author

bim9262 commented Feb 12, 2023

@MaxVerevkin, why does it matter if it's in its own module?

@MaxVerevkin
Copy link
Collaborator

MaxVerevkin commented Feb 12, 2023

why does it matter if it's in its own module?

This way it can be proven that no code accidentally leaks static reference, because public API would only provide access to borrowed items.

Actually, never mind. Any chrono::format::Item can be made 'static:

fn make_static_item(item: Item<'_>) -> Item<'static> {
    match item {
        Item::Literal(str) => Item::OwnedLiteral(str.into()),
        // And so on
    }
}

fn parse(str: &str) -> Vec<Item<'static>> {
    StrftimeItems::new(str).map(make_static_item).collect()
}

src/blocks/time.rs Outdated Show resolved Hide resolved
@MaxVerevkin
Copy link
Collaborator

Actually, never mind. Any chrono::format::Item can be made 'static:

I even think this could be upstreamed to chrono

src/formatting/parse.rs Outdated Show resolved Hide resolved
@bim9262
Copy link
Collaborator Author

bim9262 commented Feb 12, 2023

Thanks @MaxVerevkin for your help getting the static item stuff worked out!! I added docs the the formatting page as well.

@bim9262
Copy link
Collaborator Author

bim9262 commented Feb 12, 2023

This should be tagged as breaking (removal of the locale option and addition of the new datetime formatter)

@bim9262
Copy link
Collaborator Author

bim9262 commented Feb 13, 2023

Updates to the parser still to come...
Done :)

src/formatting.rs Outdated Show resolved Hide resolved
src/formatting/formatter.rs Outdated Show resolved Hide resolved
@bim9262
Copy link
Collaborator Author

bim9262 commented Feb 16, 2023

Hopefully that does it 🤞!

@ammgws ammgws merged commit 8b28c70 into greshake:master Feb 16, 2023
@bim9262 bim9262 deleted the time_format branch February 16, 2023 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaking changes enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

time block - drop parsing format in the inner loop, maybe add datetime Value with formatter
3 participants