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

RFC 2822 should have only a single space between weekday and day #1269

Closed
pitdicker opened this issue Sep 7, 2023 · 5 comments
Closed

RFC 2822 should have only a single space between weekday and day #1269

pitdicker opened this issue Sep 7, 2023 · 5 comments

Comments

@pitdicker
Copy link
Collaborator

From #249:

Test : EDT.ymd(2015, 2, 3).and_hms_milli(23, 16, 9, 150) generates Tue,<space><space>3 Feb 2015 23:16:09 +0500 i.e. 2 spaces before day's value 3.

It should be Tue,<space>3 Feb 2015 23:16:09 +0500

The rfc recommends to fold the white spaces (replace multiple white spaces to single white space).
From rfc :

Though folding   white space is permitted throughout the date-time specification, it
is RECOMMENDED that a single space be used in each place that FWS
appears (whether it is required or optional); some older
implementations may not interpret other occurrences of folding white
space correctly.

day             =       ([FWS] 1*2DIGIT) / obs-day
@pitdicker
Copy link
Collaborator Author

And the opinion of @quodlibetor at the time:

Okay, I think you've convinced me. My inclination is that this is a breaking change and so it should wait for Chrono 0.5. It should get in there, so I've added it to that milestone. There's no real timeline for that release right now, sadly.

I would happily immediately take a PR to the chrono docs explaining the fact that the date is space-padded.

I would like to also say that I would also take a PR changing the alignment from space-padded to zero-padded, but the problem is that often, as I'm sure you've encountered, time-parsing code is sensitive to whitespace so any code that relies on this existing behavior might get busted if we eliminate double spaces. On the other hand, all code that can handle chrono-formatted RFC-2833 timestamps must already be capable of handling arbitrary whitespace, so maybe it's not that big of a deal, and I should just accept this PR after you've addressed my one comment. On the other other hand, accepting this PR will ruin alignment of any terminal apps that are depending on it for their display.

So I still think this needs to at least wait for 0.5, but I'm obviously a bit torn and am open to argument.

@pitdicker
Copy link
Collaborator Author

I am not sure yet. Just opening this issue so that we don't loose the issue accidentally.

@pitdicker pitdicker changed the title RFC 2822 should have only a single day between RFC 2822 should have only a single space between weekday and day Sep 7, 2023
@djc
Copy link
Member

djc commented Sep 7, 2023

Are we talking about formatting or parsing? It's not very clear from title or description.

@pitdicker
Copy link
Collaborator Author

About formatting. The current formatting has a space as padding when the day has a single digit, leading to two spaces between the weekday and day.

If we would follow RFC 2822 religiously, a single space is better, or a 0 as padding.

@djc
Copy link
Member

djc commented Sep 7, 2023

I think formatting tweaks probably shouldn't be semver-breaking usually, except insofar as they break roundtripping with our RFC 2822 parsing code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants