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.

Sign up for GitHub

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: output does not respect comma as millisecond separator #48037

Closed
746B61 opened this issue Aug 28, 2021 · 7 comments
Closed

time: output does not respect comma as millisecond separator #48037

746B61 opened this issue Aug 28, 2021 · 7 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Go1.18

Comments

@746B61
Copy link

746B61 commented Aug 28, 2021

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

$ go version
go version go1.17 linux/amd64

Does this issue reproduce with the latest release?

Yes

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/xxx/.cache/go-build"
GOENV="/home/xxx/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/xxx/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/xxx/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.17"
GCCGO="gccgo"
AR="ar"
CC="x86_64-pc-linux-gnu-gcc"
CXX="x86_64-pc-linux-gnu-g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build1250957969=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I used time.Format to format a time including milliseconds with layout string "15:04:05,000". Before go 1.17, the comma was not recognized as separator for milliseconds and the string ,000 was added to the format output verbatim. The result was for example 23:00:12,000. Since go 1.17, the comma is recognized as separator for milliseconds, but changed to a period in the output. The result now is for example 23:00:12.345. I expect the result respect the comma in the layout string and getting 23:00:12,345.

Having the zeros replaced with the actual milliseconds changed the behavior to previous versions but is an improvement from my perspective. But, the changed behavior regarding the comma as separator breaks existing code that relies on a comma being used, e.g., because it is parsed by some other system later on.

Example code: https://play.golang.org/p/Bq6p4dxqhgG

What did you expect to see?

23:00:12,345
(or 23:00:12,000 as in go <1.17)

What did you see instead?

23:00:12.345
@WangLeonard
Copy link
Contributor

Its behavior is changed by this commit f02a26b

@gopherbot
Copy link

Change https://golang.org/cl/345438 mentions this issue: time/format: support comma as separator for fractional seconds

@seankhliao seankhliao changed the title time/format: output does not respect comma as millisecond separator time: output does not respect comma as millisecond separator Aug 29, 2021
@cherrymui cherrymui added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 30, 2021
@cherrymui cherrymui added this to the Go1.18 milestone Aug 30, 2021
@cherrymui
Copy link
Member

cc @rsc @robpike

@AlekSi
Copy link
Contributor

AlekSi commented Sep 3, 2021

Please consider backporting it into 1.17.

@cherrymui
Copy link
Member

@gopherbot please consider backporting it into 1.17. Thanks.

@gopherbot
Copy link

Backport issue(s) opened: #48177 (for 1.17).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

@gopherbot gopherbot mentioned this issue Sep 3, 2021
Closed
@gopherbot
Copy link

Change https://golang.org/cl/350149 mentions this issue: [release-branch.go1.17] time: propagate "," separator for fractional seconds into Format

gopherbot pushed a commit that referenced this issue Sep 15, 2021
@korzhao @ianlancetaylor
[release-branch.go1.17] time: propagate "," separator for fractional …
e79c297 
…seconds into Format

In CL 300996 that fixed issue #6189, we made Parse recognize
"," as a separator for fractional seconds.
However, we didn't modify Format to propagate the separator
verbatim from Parse. Without this change, we break prior
functionality that relied on a comma being used in Format.

For #48037
Fixes #48177

Change-Id: I6565a25e8657ca3747a58b25acba58f27cdcddc0
Reviewed-on: https://go-review.googlesource.com/c/go/+/345438
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Trust: Cherry Mui <cherryyz@google.com>
(cherry picked from commit e1c3f21)
Reviewed-on: https://go-review.googlesource.com/c/go/+/350149
Trust: Ian Lance Taylor <iant@golang.org>
@seankhliao seankhliao mentioned this issue Oct 2, 2021
Closed
@golang golang locked and limited conversation to collaborators Sep 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Go1.18
Development

No branches or pull requests
5 participants
@AlekSi @gopherbot @cherrymui @WangLeonard @746B61