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: Accept PT0S as valid Duration in time.ParseDuration #44681

Closed
andig opened this issue Feb 28, 2021 · 7 comments
Closed

time: Accept PT0S as valid Duration in time.ParseDuration #44681

andig opened this issue Feb 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
Backlog

Comments

@andig
Copy link
Contributor

andig commented Feb 28, 2021 •
edited

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

$ go version
go version go1.16 darwin/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="/Users/andig/Library/Caches/go-build"
GOENV="/Users/andig/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/andig/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/andig/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.16/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.16/libexec/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.16"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/andig/htdocs/evcc/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/73/89ycv7qn51j4kbm04jsz9b840000gn/T/go-build1971260061=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Parse zero duration:

_, err := time.ParseDuration("PT0S")

https://play.golang.org/p/m5cKJNEHD-n

What did you expect to see?

According to https://en.wikipedia.org/wiki/ISO_8601:

Date and time elements including their designator may be omitted if their value is zero, and lower-order elements may also be omitted for reduced precision. For example, "P23DT23H" and "P4Y" are both acceptable duration representations. However, at least one element must be present, thus "P" is not a valid representation for a duration of 0 seconds. "PT0S" or "P0D", however, are both valid and represent the same duration.

What did you see instead?

time: invalid duration "PT0S"
@andig andig changed the title time: Accept PT0S as valid Duration in time: Accept PT0S as valid Duration in time.ParseDuration Feb 28, 2021
@andig
Copy link
Contributor Author

andig commented Feb 28, 2021

Update: also decoding PT15H23M fails which looks right?

@seankhliao
Copy link
Member

seankhliao commented Feb 28, 2021 •
edited

The docs for time.ParseDuration specifies the format it supports (not ISO 8601)

Do you want to turn it into a proposal for supporting ISO 8601 durations?

@andig
Copy link
Contributor Author

andig commented Feb 28, 2021

The docs for time.ParseDuration specifies the format it supports (not ISO 8601). Do you want to turn it into a proposal for supporting ISO 8601 durations?

Good question. Quick check seems to indicate that there's currently no library that supports both decoding time.Duration in iso8601 and unmarshalling from JSON.

Changing time.Durations format seems out of question, even for Go2. I assume there is not much that can be done, maybe with the exception of adding iso8601 encoding/decoding functions?

@seankhliao
Copy link
Member

iso8601 uses uppercase while Go uses lowercase, so in theory no separate function is needed. However, Go has rejected supporting time intervals greater than an hour so I would consider it unlikely to accept full iso8601 durations

@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 5, 2021
@dmitshur dmitshur added this to the Backlog milestone Mar 5, 2021
@dmitshur
Copy link
Contributor

dmitshur commented Mar 5, 2021

CC @rsc via owners.

@andig
Copy link
Contributor Author

andig commented Mar 6, 2021

If go and ISO duration use different case, one could decode both transparently using the same decoding function. This would be great for unmarshaling JSON. For encoding, as additional ISO() string function similar to time ISOWeek() could be added.

While this is not a proposal, I could update it if there‘s likelihood of interest. Reasoning is that there‘s currently no library that seems to support encoding and decoding of ISO durations, they are part of at least one emobility REST apis though.

@andig
Copy link
Contributor Author

andig commented Aug 5, 2021

Note there is https://github.com/dylanmei/iso8601 or https://github.com/ChannelMeter/iso8601duration so it may not be necessary to have this in the language.

@andig andig closed this as completed Aug 5, 2021
@golang golang locked and limited conversation to collaborators Aug 5, 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
Backlog
Development

No branches or pull requests
4 participants
@andig @dmitshur @gopherbot @seankhliao