Navigation Menu

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

Jump to bottom

time: Time.Format does not provide space padding for numbers other than days #22802

Open
hallazzang opened this issue Nov 18, 2017 · 11 comments
Open

time: Time.Format does not provide space padding for numbers other than days #22802

hallazzang opened this issue Nov 18, 2017 · 11 comments
Labels
FeatureRequest help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unplanned

Comments

@hallazzang
Copy link
Contributor

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version go1.9.2 darwin/amd64

Does this issue reproduce with the latest release?

I'm not sure I'm running the latest release of Go, but I just ran brew upgrade go on my macOS laptop and it says go 1.9.2 already installed.

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOOS="darwin"

What did you do?

I was trying to make a library that gives programmers who wanted to use Python-like strftime method on time.Time a nice Strftime function. While writing test code to test if paddings are working, I found that there is no space(literal ) padding for 'month' and 'hour' but only 'day' haves it.

To explain what happens, here's Go playground's link: https://play.golang.org/p/cAgWgVD5Tu

As you can see underscore(_) layout string doesn't work for month, hour, minute and second.

What did you expect to see?

In the above playground, I expected 9 4 for month and day.

What did you see instead?

But I've got _9 4 instead. Same for month, hour, minute and second.

I would like to patch the time package and I'd like to know whether this issue has already issued or not. Thanks.
@bradfitz bradfitz changed the title time.Time.Format does not provide consistency for space padding time: Time.Format does not provide consistency for space padding Nov 18, 2017
@bradfitz bradfitz changed the title time: Time.Format does not provide consistency for space padding time: Time.Format does not provide space padding for numbers other than days Nov 18, 2017
@bradfitz bradfitz added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Nov 18, 2017
@bradfitz bradfitz added this to the Go1.11 milestone Nov 18, 2017
@bradfitz
Copy link
Contributor

Well, this is documented at least:

Within the format string, an underscore _ represents a space that may be replaced by a digit if the following number (a day) has two digits; for compatibility with fixed-width Unix time formats.

Up to @robpike to decide whether to extend it to support other things.

@hallazzang
Copy link
Contributor Author

Yes, and I've found another thing that 24-hour based hour cannot represented without padding since 15(already has two digits) is proper placeholder for it. Would it be solved, too?

@robpike
Copy link
Contributor

robpike commented Nov 19, 2017

It does seem inconsistent but is it a common enough case to worry about? No one has noticed the issue before and it's easy to fix by post-processing.

What do others think?

@hallazzang
Copy link
Contributor Author

hallazzang commented Nov 19, 2017 •
edited

Well I was surprised that no one has noticed this issue but me, I think the reason is that the zero padding fits well for almost every usecases. But in my opinion a consistency for standard packages matters a lot, would it be good to patch it by myself and send a pull request to this repository? I've never contributed to this large repository before, though.

@robpike
Copy link
Contributor

robpike commented Nov 19, 2017

This repository does not accept pull requests directly. Please see https://golang.org/doc/contribute.html

@hallazzang
Copy link
Contributor Author

Thank you. I'll read it and get on my work soon!

@hallazzang
Copy link
Contributor Author

hallazzang commented Nov 20, 2017 •
edited

By the way, how do you think about the 24-based hour's no-padding(7 instead of ^7 for minute, ^ represents blank space) representation? Currently it seems impossible to correctly support consistent padding rule for 24-based hour since it already has 2 digits(15). How could one workaround this?

@gopherbot
Copy link

Change https://golang.org/cl/78735 mentions this issue: time: add space padding layout strings(using underscore) for not only day but others

@rsc rsc added the NeedsFix The path to resolution is known, but the work has not been done. label Nov 27, 2017
@gopherbot gopherbot removed the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Nov 27, 2017
@rsc
Copy link
Contributor

rsc commented Nov 27, 2017

Rob will review.

@hallazzang
Copy link
Contributor Author

@rsc Anything should I do for now?

gopherbot pushed a commit that referenced this issue Dec 13, 2017
@hallazzang @robpike
time: add space padding layout strings(using underscore) for not only…
8776be1 
… day but others

As mentioned in #22802, only day component of layout string has space
padding(represented by one underscore before its placeholder). This
commit expands the rule for month, hour, minute and second.

Updates #22802 (maybe fixes it)

Change-Id: I886998380489862ab9a324a6774f2e4cf7124122
Reviewed-on: https://go-review.googlesource.com/78735
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
@hallazzang hallazzang mentioned this issue Dec 28, 2017
Closed
@gopherbot
Copy link

Change https://golang.org/cl/85998 mentions this issue: time: revert CL 78735 (was: space padding using underscore)

@ianlancetaylor ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 3, 2018
gopherbot pushed a commit that referenced this issue Jan 3, 2018
@ianlancetaylor
time: revert CL 78735 (was: space padding using underscore)
86cca11 
CL 78735 description:

    time: add space padding layout strings(using underscore) for not only day but others

    As mentioned in #22802, only day component of layout string has space
    padding(represented by one underscore before its placeholder). This
    commit expands the rule for month, hour, minute and second.

    Updates #22802 (maybe fixes it)

Revert this CL because it breaks currently working formats that happen
to use underscores.

Fixes #23259

Change-Id: I64acaaca9b5b74785ee0f0be7910574e87daa649
Reviewed-on: https://go-review.googlesource.com/85998
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
@gopherbot gopherbot removed the NeedsFix The path to resolution is known, but the work has not been done. label Jan 3, 2018
@ianlancetaylor ianlancetaylor modified the milestones: Go1.11, Unplanned Jun 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
FeatureRequest help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
6 participants
@bradfitz @rsc @ianlancetaylor @robpike @hallazzang @gopherbot