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

Add clock cli flag #9138

Closed
wants to merge 10 commits into from
Closed

Add clock cli flag #9138

wants to merge 10 commits into from

Conversation

satotake
Copy link
Contributor

@satotake satotake commented Nov 6, 2021

issue #8787
If I understand @regisphilibert 's comment properly, with the flag, users want to check

  • future/expired content's manners
  • now in template
  • hugo list output

If so, we can implement this flag by just replacing a few of time.Now() without changing internal time of go.

This PRs add buildTime flag in order to fake hugo's build datetime like so

hugo --buildTime 2021-11-06
hugo --buildTime 2021-11-06 server
hugo --buildTime 2021-11-06 list future
hugo --buildTime 2021-11-06 list expired

Close #8787

@flimzy
Copy link

flimzy commented Feb 22, 2022

This is a feature that would make my life a lot easier. What are the chances of getting this reviewed and rebased any time soon?

@davidsneighbour
Copy link
Contributor

This is a feature that would make my life a lot easier. What are the chances of getting this reviewed and rebased any time soon?

Thumbs up and useful contributions tend to do that ;)

The "build time" moniker is slightly misleading but I like the concept. If it has enough testing to make sure it works as expected and when the "conflicts that must be resolved" are resolved (probably just a fetch of the current HEAD and re-check) our little conversation here will put it in the TODO pile again and someone who knows Golang will surely have a look.

@flimzy
Copy link

flimzy commented Feb 23, 2022

@satotake Are you still interested in this issue/PR? If not, I'd be happy to take it over and do a rebase in a new PR.

@satotake satotake force-pushed the add-date-flag branch 2 times, most recently from 92f2a2e to dc25c77 Compare February 23, 2022 15:55
@satotake
Copy link
Contributor Author

satotake commented Feb 23, 2022

Honestly, this PR has been a dim memory for me but rebasing and resolving were not so tough.
Thanks for your offer, @flimzy .

If you do not mind, could you review this PR or have any idea, @bep?
Especially, my concern is flag name as @davidsneighbour said.
Of course, it is ok for me to change the name.

commands/list_test.go Outdated Show resolved Hide resolved
tpl/time/time.go Outdated Show resolved Hide resolved
commands/list_test.go Outdated Show resolved Hide resolved
hugolib/hugo_sites_build_test.go Outdated Show resolved Hide resolved
hugolib/page_test.go Outdated Show resolved Hide resolved
resources/resource/dates.go Outdated Show resolved Hide resolved
satotake added 3 commits May 4, 2022 17:48
* Fix `resource.IsFuture` and `resource.IsExpired` args
* Move `TestBuildTimeFlag` in `commands/list_test.go` to `hugo_sites_build_test.go`
* Isolate `TestShouldBuildWithNowOverride` in `hugolib/page_test.go`
Due to capturing stdout for list command test
@bep
Copy link
Member

bep commented May 6, 2022

OK, I have thought some more about this, and I think we need to do an adjustment.

The thing is, when we're changing the build time, I also suspect people would also expect the clock to change in other situations, e.g.:

{{ ge .PublishDate now }}

Should always print true.

With that in mind, I think we need a ... clock.

What I suggest is that we

  • rename the buildTime flag to clock (?)
  • rework the implementation in this PR to also consider the system clock. I have toyed around with this package tonight, which I thinks should be simple enough to reason about: https://github.com/bep/clock
  • Replace all usage of time.Now with htime.Now

Thoughts?

@satotake
Copy link
Contributor Author

satotake commented May 7, 2022

rename the buildTime flag to clock (?)

Looks good. I thought just time or date might be ambiguous for this.

rework the implementation in this PR to also consider the system clock. I have toyed around with this package tonight, which I thinks should be simple enough to reason about: https://github.com/bep/clock

I have presumed fixed time for this. yes but it is ok to make it running. I will check it your package.

Replace all usage of time.Now with htime.Now

Not sure but I suspect that they are not all but most. For example, values for logging and so on? Anyway, I will check it, too.

@satotake satotake changed the title Add buildTime cli flag Add clock cli flag May 7, 2022
satotake added 2 commits May 7, 2022 17:09
- rename `buildTime` `clcok`
- install `github.com/bep/clock` and rework
@bep
Copy link
Member

bep commented May 7, 2022

Thanks, I have rebased this into a new PR and will finish this in #9864

@bep bep closed this May 7, 2022
@satotake
Copy link
Contributor Author

satotake commented May 7, 2022

ok

I replaced time.Now with htime.Now as much as possible (including logging) for now.

except for

@github-actions
Copy link

github-actions bot commented May 8, 2023

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add --date CLI flag
4 participants