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

Doc generation default value set by environment variable #1902

Open
3 tasks done
sj14 opened this issue May 1, 2024 · 4 comments
Open
3 tasks done

Doc generation default value set by environment variable #1902

sj14 opened this issue May 1, 2024 · 4 comments
Assignees
Labels
area/v2 relates to / is being considered for v2 kind/bug describes or fixes a bug

Comments

@sj14
Copy link

sj14 commented May 1, 2024

My urfave/cli version is v2.27.2

Checklist

Dependency Management

  • My project is using go modules.

Describe the bug

When you have a flag which can also be set by an env var, and the flag is empty, the value from the env will be used in the output docs markdown.

To reproduce

package main

import (
	"log"
	"os"

	"github.com/urfave/cli/v2"
)

func main() {
	app := &cli.App{
		Flags: []cli.Flag{
			&cli.StringFlag{
				Name:    "url",
				Value:   "http://127.0.0.1",
				EnvVars: []string{"URL"},
			},
			&cli.StringFlag{
				Name:    "token",
				Value:   "",
				EnvVars: []string{"TOKEN"},
			},
		},
		Commands: []*cli.Command{
			{
				Name:   "docs",
				Hidden: true,
				Action: func(ctx *cli.Context) error {
					s, err := ctx.App.ToMan()
					if err != nil {
						return err
					}
					return os.WriteFile("DOCS.man", []byte(s), os.ModePerm)
				},
			},
		},
	}

	if err := app.Run(os.Args); err != nil {
		log.Fatal(err)
	}
}

Observed behavior

When running the example like this

TOKEN=my-token URL=my-url go run test/main.go docs

a file called DOCS.man is created with the follwing content:

.nh
.TH main 8

.SH NAME
.PP
main - A new cli application


.SH SYNOPSIS
.PP
main

.EX
[--help|-h]
[--token]=[value]
[--url]=[value]
.EE

.PP
\fBUsage\fP:

.EX
main [GLOBAL OPTIONS] command [COMMAND OPTIONS] [ARGUMENTS...]
.EE


.SH GLOBAL OPTIONS
.PP
\fB--help, -h\fP: show help

.PP
\fB--token\fP="":  (default: my-token)

.PP
\fB--url\fP="":  (default: "http://127.0.0.1")


.SH COMMANDS
.SH help, h
.PP
Shows a list of commands or help for one command

Expected behavior

The token flag should not show the environment variable as the default value.

Additional context

This happens for the Markdown generation too.

Run go version and paste its output here

go version go1.22.2 darwin/amd64

Run go env and paste its output here

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/Users/simon/Library/Caches/go-build'
GOENV='/Users/simon/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/simon/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/simon/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/Cellar/go/1.22.2/libexec'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/Cellar/go/1.22.2/libexec/pkg/tool/darwin_amd64'
GOVCS=''
GOVERSION='go1.22.2'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='cc'
CXX='c++'
CGO_ENABLED='1'
GOMOD='/Users/simon/repos/personal/<REMOVED>/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/c2/c287hy7d1n17sjlxsn7ctph00000gn/T/go-build3874607874=/tmp/go-build -gno-record-gcc-switches -fno-common'
@sj14 sj14 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 May 1, 2024
@meatballhat meatballhat self-assigned this May 5, 2024
@meatballhat
Copy link
Member

@sj14 Thank you for reporting this! Differentiating between "unset" and "uninitialized" values, and the precedence of value sources, have long been sources of surprising behavior. I believe that the intended behavior is that flag and env parsing should be completely separate from help text and documentation generation, so I think that treating this as a bug is very fair. I suspect that fixing this the right way will require non-trivial rework of the help text and documentation generation code, which tells me that this work should probably be targeting v3 on the main branch. Does that sound reasonable to you?

@meatballhat meatballhat removed the status/triage maintainers still need to look into this label May 5, 2024
@sj14
Copy link
Author

sj14 commented May 5, 2024

Sound reasonable to me, but maybe it can be documented in v2 as it can easily lead to exposed secrets.

@meatballhat
Copy link
Member

@sj14 Thank you! Does any of the work required interest you? 😇

@sj14
Copy link
Author

sj14 commented May 6, 2024

I may create a PR for the documentation but I can't promise 😆

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
Projects
None yet
Development

No branches or pull requests

2 participants