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

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

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

Comments

Projects
None yet
6 participants
@hallazzang
Copy link
Contributor

hallazzang commented Nov 18, 2017

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 this to the Go1.11 milestone Nov 18, 2017

@bradfitz

This comment has been minimized.

Copy link
Member

bradfitz commented Nov 18, 2017

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

This comment has been minimized.

Copy link
Contributor

hallazzang commented Nov 19, 2017

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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Contributor

hallazzang commented Nov 19, 2017

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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Contributor

hallazzang commented Nov 20, 2017

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

@hallazzang

This comment has been minimized.

Copy link
Contributor

hallazzang commented Nov 20, 2017

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

This comment has been minimized.

Copy link

gopherbot commented Nov 20, 2017

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 label Nov 27, 2017

@rsc

This comment has been minimized.

Copy link
Contributor

rsc commented Nov 27, 2017

Rob will review.

@hallazzang

This comment has been minimized.

Copy link
Contributor

hallazzang commented Nov 28, 2017

@rsc Anything should I do for now?

gopherbot pushed a commit that referenced this issue Dec 13, 2017

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)

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>
@gopherbot

This comment has been minimized.

Copy link

gopherbot commented Jan 3, 2018

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

gopherbot pushed a commit that referenced this issue Jan 3, 2018

time: revert CL 78735 (was: space padding using underscore)
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 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