Skip to content

Commit ce47fcf

Browse files
Make Flag[T] a pointer in prep for some future work (#209)
* Make Flag a pointer type in prep for some future work * Try out mise as a task runner
1 parent eddca5d commit ce47fcf

10 files changed

Lines changed: 129 additions & 153 deletions

File tree

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@
2020
# Task
2121
!Taskfile.yml
2222

23+
# mise
24+
!mise.toml
25+
2326
# Git
2427
!.gitattributes
2528
!.gitignore

Taskfile.yml

Lines changed: 0 additions & 127 deletions
This file was deleted.

docs/img/cancel.gif

76 Bytes
Loading

docs/img/namedargs.gif

999 Bytes
Loading

docs/img/quickstart.gif

-28 Bytes
Loading

docs/img/subcommands.gif

-859 Bytes
Loading

internal/flag/flag.go

Lines changed: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import (
2020
"go.followtheprocess.codes/cli/internal/parse"
2121
)
2222

23-
var _ Value = Flag[string]{} // This will fail if we violate our Value interface
23+
var _ Value = &Flag[string]{} // This will fail if we violate our Value interface
2424

2525
// Flag represents a single command line flag.
2626
type Flag[T flag.Flaggable] struct {
@@ -35,53 +35,51 @@ type Flag[T flag.Flaggable] struct {
3535
//
3636
// The name should be as it appears on the command line, e.g. "force" for a --force flag. An optional
3737
// shorthand can be created by setting short to a single letter value, e.g. "f" to also create a -f version of "force".
38-
func New[T flag.Flaggable](p *T, name string, short rune, usage string, config Config[T]) (Flag[T], error) {
38+
func New[T flag.Flaggable](p *T, name string, short rune, usage string, config Config[T]) (*Flag[T], error) {
3939
if err := validateFlagName(name); err != nil {
40-
return Flag[T]{}, fmt.Errorf("invalid flag name %q: %w", name, err)
40+
return nil, fmt.Errorf("invalid flag name %q: %w", name, err)
4141
}
4242

4343
if err := validateFlagShort(short); err != nil {
44-
return Flag[T]{}, fmt.Errorf("invalid shorthand for flag %q: %w", name, err)
44+
return nil, fmt.Errorf("invalid shorthand for flag %q: %w", name, err)
4545
}
4646

4747
if p == nil {
48-
p = new(T)
48+
return nil, fmt.Errorf("flag %q: target pointer must not be nil", name)
4949
}
5050

5151
*p = config.DefaultValue
5252

53-
flag := Flag[T]{
53+
return &Flag[T]{
5454
value: p,
5555
name: name,
5656
usage: usage,
5757
short: short,
5858
envVar: config.EnvVar,
59-
}
60-
61-
return flag, nil
59+
}, nil
6260
}
6361

6462
// Name returns the name of the [Flag].
65-
func (f Flag[T]) Name() string {
63+
func (f *Flag[T]) Name() string {
6664
return f.name
6765
}
6866

6967
// Short returns the shorthand registered for the flag (e.g. -d for --delete), or
7068
// NoShortHand if the flag should be long only.
71-
func (f Flag[T]) Short() rune {
69+
func (f *Flag[T]) Short() rune {
7270
return f.short
7371
}
7472

7573
// Usage returns the usage line for the flag.
76-
func (f Flag[T]) Usage() string {
74+
func (f *Flag[T]) Usage() string {
7775
return f.usage
7876
}
7977

8078
// Default returns the default value for the flag, as a string.
8179
//
8280
// If the flag's default is unset (i.e. the zero value for its type),
8381
// an empty string is returned.
84-
func (f Flag[T]) Default() string {
82+
func (f *Flag[T]) Default() string {
8583
// Special case a --help flag, because if we didn't, when you call --help
8684
// it would show up with a default of true because you've passed it
8785
// so it's value is true here
@@ -94,13 +92,13 @@ func (f Flag[T]) Default() string {
9492

9593
// EnvVar returns the name of the environment variable associated with this flag,
9694
// or an empty string if none was configured.
97-
func (f Flag[T]) EnvVar() string {
95+
func (f *Flag[T]) EnvVar() string {
9896
return f.envVar
9997
}
10098

10199
// IsSlice reports whether the flag holds a slice value that accumulates repeated
102100
// calls to Set. Returns false for []byte and net.IP, which are parsed atomically.
103-
func (f Flag[T]) IsSlice() bool {
101+
func (f *Flag[T]) IsSlice() bool {
104102
if f.value == nil {
105103
return false
106104
}
@@ -118,7 +116,7 @@ func (f Flag[T]) IsSlice() bool {
118116
// NoArgValue returns a string representation of value the flag should hold
119117
// when it is given no arguments on the command line. For example a boolean flag
120118
// --delete, when passed without arguments implies --delete true.
121-
func (f Flag[T]) NoArgValue() string {
119+
func (f *Flag[T]) NoArgValue() string {
122120
switch f.Type() {
123121
case format.TypeBool:
124122
// Boolean flags imply passing true, "--force" vs "--force true"
@@ -135,7 +133,7 @@ func (f Flag[T]) NoArgValue() string {
135133
// part of [Value], allowing a flag to print itself.
136134
//
137135
//nolint:cyclop // No other way of doing this realistically
138-
func (f Flag[T]) String() string {
136+
func (f *Flag[T]) String() string {
139137
if f.value == nil {
140138
return format.Nil
141139
}
@@ -217,7 +215,7 @@ func (f Flag[T]) String() string {
217215
}
218216

219217
// Type returns a string representation of the type of the Flag.
220-
func (f Flag[T]) Type() string { //nolint:cyclop // No other way of doing this realistically
218+
func (f *Flag[T]) Type() string { //nolint:cyclop // No other way of doing this realistically
221219
if f.value == nil {
222220
return format.Nil
223221
}
@@ -295,7 +293,7 @@ func (f Flag[T]) Type() string { //nolint:cyclop // No other way of doing this r
295293
// Set sets a [Flag] value based on string input, i.e. parsing from the command line.
296294
//
297295
//nolint:gocognit,maintidx // No other way of doing this realistically
298-
func (f Flag[T]) Set(str string) error {
296+
func (f *Flag[T]) Set(str string) error {
299297
if f.value == nil {
300298
return fmt.Errorf("cannot set value %s, flag.value was nil", str)
301299
}

internal/flag/flag_test.go

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1059,14 +1059,12 @@ func TestFlagValidation(t *testing.T) {
10591059

10601060
func TestFlagNilSafety(t *testing.T) {
10611061
t.Run("with new", func(t *testing.T) {
1062-
// Should be impossible to make a nil pointer dereference when using .New
1062+
// Passing a nil target is an error
10631063
var bang *bool
10641064

1065-
flag, err := flag.New(bang, "bang", 'b', "Nil go bang?", flag.Config[bool]{})
1066-
test.Ok(t, err)
1067-
1068-
test.Equal(t, flag.String(), "false")
1069-
test.Equal(t, flag.Type(), "bool")
1065+
f, err := flag.New(bang, "bang", 'b', "Nil go bang?", flag.Config[bool]{})
1066+
test.Err(t, err)
1067+
test.Equal(t, f, nil)
10701068
})
10711069

10721070
t.Run("composite literal", func(t *testing.T) {

internal/flag/set.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,15 @@ func NewSet() *Set {
3131
}
3232

3333
// AddToSet adds a flag to the given Set.
34-
func AddToSet[T flag.Flaggable](set *Set, f Flag[T]) error {
34+
func AddToSet[T flag.Flaggable](set *Set, f *Flag[T]) error {
3535
if set == nil {
3636
return errors.New("cannot add flag to a nil set")
3737
}
3838

39+
if f == nil {
40+
return errors.New("cannot add nil flag to a set")
41+
}
42+
3943
name := f.Name()
4044
short := f.Short()
4145

mise.toml

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
[tools]
2+
go = "1.26"
3+
golangci-lint = "latest"
4+
typos = "latest"
5+
vhs = "latest"
6+
"aqua:charmbracelet/freeze" = "latest"
7+
fd = "latest"
8+
"go:go.uber.org/nilaway/cmd/nilaway" = "latest"
9+
"go:golang.org/x/pkgsite/cmd/pkgsite" = "latest"
10+
11+
[vars]
12+
COV_DATA = "coverage.out"
13+
14+
[tasks.tidy]
15+
description = "Tidy dependencies in go.mod and go.sum"
16+
run = "go mod tidy"
17+
sources = ["**/*.go", "go.mod", "go.sum"]
18+
19+
[tasks.fmt]
20+
description = "Run go fmt on all source files"
21+
run = "golangci-lint fmt ./..."
22+
sources = ["**/*.go", ".golangci.yml", "**/*.md"]
23+
24+
[tasks.test]
25+
description = "Run the test suite"
26+
usage = '''
27+
arg "[args]..." help="Extra args to pass to go test"
28+
'''
29+
# -race needs CGO (https://go.dev/doc/articles/race_detector#Requirements)
30+
env = { CGO_ENABLED = "1" }
31+
run = "go test -race ./... {{arg(name='args', default='')}}"
32+
sources = ["**/*.go", "**/testdata/**/*", "go.mod", "go.sum"]
33+
34+
[tasks.bench]
35+
description = "Run all project benchmarks"
36+
usage = '''
37+
arg "[args]..." help="Extra args to pass to go test"
38+
'''
39+
run = "go test ./... -run None -benchmem -bench . {{arg(name='args', default='')}}"
40+
sources = ["**/*.go"]
41+
42+
[tasks.lint]
43+
description = "Run the linters and auto-fix if possible"
44+
depends = ["fmt"]
45+
run = [
46+
"golangci-lint run --fix",
47+
"typos",
48+
"nilaway ./...",
49+
]
50+
sources = ["**/*.go", ".golangci.yml"]
51+
52+
[tasks.docs]
53+
description = "Render the pkg docs locally"
54+
raw = true
55+
run = "pkgsite -open"
56+
57+
[tasks.demo]
58+
description = "Render the demo gifs in parallel"
59+
run = [
60+
'for file in ./docs/src/*.tape; do vhs "$file" & done; wait',
61+
"freeze ./examples/cover/main.go --config ./docs/src/freeze.json --output ./docs/img/demo.png --show-line-numbers",
62+
]
63+
sources = ["./docs/src/*.tape", "**/*.go"]
64+
outputs = ["./docs/img/*.gif", "./docs/img/demo.png"]
65+
66+
[tasks.cov]
67+
description = "Calculate test coverage (pass --open to view as HTML in a browser)"
68+
usage = '''
69+
flag "--open" help="Open the coverage report in a browser"
70+
'''
71+
run = '''
72+
go test -race -cover -covermode atomic -coverprofile {{vars.COV_DATA}} ./...
73+
if [ "${usage_open:-false}" = "true" ]; then
74+
go tool cover -html {{vars.COV_DATA}}
75+
fi
76+
'''
77+
sources = ["**/*.go", "**/testdata/**/*", "go.mod", "go.sum"]
78+
outputs = ["{{vars.COV_DATA}}"]
79+
80+
[tasks.check]
81+
description = "Run tests and linting in one"
82+
depends = ["test", "lint"]
83+
84+
[tasks.sloc]
85+
description = "Print lines of code"
86+
run = "fd . -e go | xargs wc -l | sort -nr | head"
87+
88+
[tasks.clean]
89+
description = "Remove build artifacts and other clutter"
90+
run = [
91+
"go clean ./...",
92+
"rm -rf *.out",
93+
]
94+
95+
[tasks.update]
96+
description = "Updates dependencies in go.mod and go.sum"
97+
run = [
98+
"go get -u ./...",
99+
"go mod tidy",
100+
]

0 commit comments

Comments
 (0)