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

lib/time: consider any Time other than zero instant to be truthy #441

Merged

Conversation

SamWheating
Copy link
Contributor

I know that the truthiness of timestamps isn't well defined, but the current behaviour is the opposite of what I would expect:

Welcome to Starlark (go.starlark.net)
>>> bool(time.now())
False
>>> bool(time.parse_time("0001-01-01T00:00:00Z"))
True

This change inverts the existing logic so that any time other than 0001-01-01T00:00:00Z will evaluate to True.

Alternatively, I think that considering all times to be True would also be acceptable. Thoughts?

@@ -348,7 +348,7 @@ func (t Time) Hash() (uint32, error) {

// Truth returns the truth value of an object required by starlark.Value
// interface.
func (t Time) Truth() starlark.Bool { return starlark.Bool(time.Time(t).IsZero()) }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was clearly just a mistake that no-one noticed until now. Thanks for the fix!

Copy link
Contributor Author

@SamWheating SamWheating Dec 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem, thanks for the quick review!

I'm pretty interested in this project - do you mind if I pick up a few of the TODOs? Got any new features or library modules you've been thinking about?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty interested in this project - do you mind if I pick up a few of the TODOs? Got any new features or library modules you've been thinking about?

Please do! I don't have any specific new features in mind. Much of the effort of this project has been spent in trying to harmonize the two (now three) implementations around a common spec, and to be very judicious about new features. Feel free to have a crack at any TODO comment in the code, and don't hesitate to ask for clarification before spending significant effort. These issues too might be suitable tasks, depending on your tastes:

#356
#249
#270
#271
#292

Many thanks for the kind offer.

@adonovan adonovan merged commit 0eacda4 into google:master Dec 5, 2022
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.

2 participants