Skip to content

Conversation

F0RRZZ
Copy link
Contributor

@F0RRZZ F0RRZZ commented Sep 16, 2025

@F0RRZZ F0RRZZ force-pushed the issue-137165 branch 5 times, most recently from 7f3f4d8 to 063d0e1 Compare September 16, 2025 22:39
@F0RRZZ F0RRZZ changed the title gh-137165: Add non-zero-padded Windows support for datetime.strftime gh-137165: Add non-zero-padded Windows support for datetime.strftime Sep 16, 2025
@F0RRZZ
Copy link
Contributor Author

F0RRZZ commented Sep 17, 2025

This feature was also broken on Android (x86_64)

Screenshot 2025-09-17 at 08 37 07

Now working and covered by tests

@F0RRZZ F0RRZZ marked this pull request as draft September 17, 2025 22:51
@pganssle
Copy link
Member

@F0RRZZ Given that you closed this PR, does this mean you do not want us to go foward with it? It looks like a great start. If you don't want to continue with it, do you mind if someone takes your work and runs with it?

@vstinner @F0RRZZ Any thoughts on whether this shim might work better in time.strftime instead of datetime.strftime? I think that normally the datetime-specific extensions to time.stftime have been to cover things like time zones that don't make sense in the time module, which argues in favor of it going there, but on the other hand, there are no other similar hacks in time, so that argues in favor of not complicating that function.

@vstinner
Copy link
Member

If strftime() is enhanced, it would be nice to do it in the time module. I have no opinion on this specific change.

@mhsmith
Copy link
Member

mhsmith commented Sep 18, 2025

@F0RRZZ has now created a new PR: #139088. You don't need to do this every time you make a change: please just push a new commit to the existing PR so the discussion is all in one place. And please don't force-push if you've already received a review, as it makes it more difficult to track the changes.

@F0RRZZ
Copy link
Contributor Author

F0RRZZ commented Sep 18, 2025

@pganssle I’ve opened a new pull request where I continued this work: #139088
I put this in _datetimemodule.c, since similar special cases are handled in datetime.strftime, that felt logical

@mhsmith Got it, thanks! I opened a new PR because with the large number of test commits the history felt a bit too messy, so I thought it would be cleaner to start fresh, sorry for the noise

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

Successfully merging this pull request may close these issues.

4 participants