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

v2 bug: Value does not work with TimestampFlag #1144

Closed
5 of 7 tasks
vschettino opened this issue Jun 5, 2020 · 5 comments · Fixed by #1160
Closed
5 of 7 tasks

v2 bug: Value does not work with TimestampFlag #1144

vschettino opened this issue Jun 5, 2020 · 5 comments · Fixed by #1160
Assignees
Labels
area/v2 relates to / is being considered for v2 kind/bug describes or fixes a bug status/claimed someone has claimed this issue

Comments

@vschettino
Copy link
Contributor

my urfave/cli version is

2.2.0

Checklist

  • Are you running the latest v2 release? The list of releases is here.
  • Did you check the manual for your release? The v2 manual is here
  • Did you perform a search about this problem? Here's the Github guide about searching.

Dependency Management

  • My project is using go modules.
  • My project is using vendoring.
  • My project is automatically downloading the latest version.
  • I am unsure of what my dependency management setup is.

Describe the bug

Passing a Value to TimestampFlag does nothing.

To reproduce

package main

import (
	"fmt"
	"github.com/urfave/cli/v2"
	"log"
	"os"
	"time"
)

func main() {
	var today = cli.NewTimestamp(time.Now())
	app := &cli.App{
		Commands: []*cli.Command{{
			Name: "sync",
			Flags: []cli.Flag{
				&cli.TimestampFlag{
					Name:   "from",
					Layout: "2006-01-02",
					Value:  today,
				},
			},
			Action: func(c *cli.Context) error {
				fmt.Println(c.Timestamp("from"))
				return nil
			},
		}},
	}
	err := app.Run(os.Args)
	if err != nil {
		log.Fatal(err)
	}
}
go install && go mymodule sync

Describe the steps or code required to reproduce the behavior

Observed behavior

"from" flag is <nil>

Expected behavior

The Value (today) would be printed, as it works with other types of flag, such as IntFlag.

Additional context

Docker build and run

Want to fix this yourself?

I'd love to help. Can you guys confirm this and give me some directions about how to proceed?

Run go version and paste its output here

go version go1.14.2 linux/amd64

Run go env and paste its output here

GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/root/.cache/go-build"
GOENV="/root/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="0"
GOMOD="/exfin/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 -m64 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build096471754=/tmp/go-build -gno-record-gcc-switches"

@vschettino vschettino added area/v2 relates to / is being considered for v2 kind/bug describes or fixes a bug status/triage maintainers still need to look into this labels Jun 5, 2020
@aloababa
Copy link

aloababa commented Jul 10, 2020

I guess it's because of this: https://github.com/urfave/cli/blob/master/flag_timestamp.go#L121
Default value always gets override.
So i think this can be fixed by adding an if like:

if f.Value == nil {
	f.Value = &Timestamp{}
}

@vschettino
Copy link
Contributor Author

@aloababa yes, seems like this is the place. Thanks for pointing out! I will open a PR to fix this on the weekend.

@coilysiren coilysiren added status/claimed someone has claimed this issue and removed status/triage maintainers still need to look into this labels Jul 10, 2020
@coilysiren
Copy link
Member

Sounds good, cc me when you're ready for a review ^^

@vschettino
Copy link
Contributor Author

@lynncyrin do you guys need any help on other issues? I saw a few that might be stale but I don't wanna to intrude ^^

@coilysiren
Copy link
Member

Yes we could totally use help with stale or help wanted issues 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/v2 relates to / is being considered for v2 kind/bug describes or fixes a bug status/claimed someone has claimed this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants