Conversation
|
Cool! This conflicts with #5181, but we can rebase that once this is merged. |
|
Okay I believe the error is unrelated to this PR but do let me know. In terms of coverage, I've had to introduce two new (likely rare) error paths--caused if atime/mtime cannot be converted to DateTime due to overflow. Do let me know if I should attempt to add tests for these, honestly I'm not to sure how it would be triggered, perhaps an a/mtime of INTMAX or INTMIN. |
|
So I do have a bit of a hacky fix for the new test Which works in my terminal, but the test still fails with: Not sure why the test suite changes the ticks to quotes? Please advise -- though I probably won't get back to this till next weekend 😞 In any-case, I made the test a bit easier 🤷♂️ |
src/uu/touch/src/touch.rs
Outdated
| let hook = std::panic::take_hook(); | ||
| std::panic::set_hook(Box::new(|_| {})); | ||
| let result = std::panic::catch_unwind(|| parse_datetime::parse_datetime_at_date(ref_time, s)); | ||
| std::panic::set_hook(hook); |
There was a problem hiding this comment.
Ehh, I don't think that's a great idea 😄 This won't work with panic = "abort" for example. I think we need to fix parse_datetime instead. We don't need to match GNU's errors exactly. What's different?
There was a problem hiding this comment.
Yeah I figured, the panic is in Chrono itself. The commits are pretty contained so we can just drop b6c6cca
In terms of the error, touch seems to use directional quotes instead of single quotes.
There was a problem hiding this comment.
we should contribute in chrono to remove the panic and returns error in general
|
Oh the quotes are based on locale! We currently entirely ignore locale. If you run |
|
please ignore this test for now: |
|
Thanks :) |
Fix #5274
Fix #5307