-
Notifications
You must be signed in to change notification settings - Fork 10
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
Default date is not configurable #39
Comments
Hey @haydenflinner, pull requests are always welcome. I had a look at |
Unfortunately not, and I don't think this lib will be made to fit my use-case as easily as I thought it would during prototyping. I really need to know what was found, whether it was a time without a date, or a date without a time, or a real timestamp. Once I know that, falling back to a default date or time is easy, and I can use the context of what time or date was parsed to decide what my fallback value is. With the API as-is, I need to know my default ahead of time, and at return-time I can't tell if what I got back was my default or just a value which happened to match it. For example, suppose I'm parsing a list of timestamps, and they suddenly change from 23:59:00 to 00:01. Probably, it is a new day. Detecting that with default_date set requires doing some math like 'did my timestamps suddenly jump back >20 hours). For now, I copy-pasted the whole thing into my Btw, I added support for fractional seconds in |
Thanks for sharing this information! I'm planning for a new version of this rust crate, and it might involve a complete rewrite to improve parsing speed and provide more flexible configuration options for the parser. I will take your use case into consideration. When I first wrote this parser, it was part of a cli tool, and I made some opinionated decisions on the library's api design. As time went by, I've noticed a growing demand for increased flexibility. Addressing these requests would require some degree of redesign. And I've always wanted to change the way how the date time string is parsed, transitioning away from regex towards a character by character parsing approach. |
That's cool news, thanks! On the regex note, it would be neat if Rust had compile-time regexes rather than always requiring the runtime, but I suspect the reason it doesn't is that regexing is rarely the bottleneck. Make sure you've profiled ( From a performance perspective, there are two main things that I noticed. One is that most users who care about speed are likely parsing large sequences of the same data, so if we were to guess the right format the first time we'd be far faster than always iterating through the whole list (if their format is towards the bottom). To that end just remembering which format was last parsed in this parser would be 95+% of the way there, if you really wanted to overkill you could keep some sort of data structure to sort them by observed frequency. The other thing that might help is joining all of the regexes into one big one (using |
Agreed on what you suggested. Thanks, Hayden 🙏 I will first try statically declare regex, and replace the use of For overall speed, I just ran |
Awesome, gonna be hard to beat that! Have you given any thought to how the API redesign will look? Maybe returning a struct containing Option Option? This is not yet at the top of my priorities either, I can always just make my users type in the exact date/time they're looking for 😄 but it is a little weird in my log-viewer when someone types "2023/04/10" and the viewer seeks to "current time" on that date haha. |
Would a patch be welcomed that's similar to the below, but for setting the default date to be used? Right now there are a lot of places that default to utc::now(), which doesn't work for my use-case. I need to set some context of what day I expect dateless timestamps to be.
826b8c4
The text was updated successfully, but these errors were encountered: