Sign in

Search issue titles and bodies, change requests and comments. Private repositories you cannot read are not included.

vole
Clone

Read only, no credentials:

https://rodent.software/vole.git

Write over SSH with a registered key:

git clone ssh://git@rodent.software:2222/vole.git

A free and open-source forge that lives in your Git repo.

Make the CLI speak git's dialect

f956874883e9fdbb95f13b5429bb1104dcec2756 · Keenan Nemetz · 18d ago · parent c1e3e3ca

Make the CLI speak git's dialect

The flags were wrong in the way that matters: `-title` is a single-dash long
option, which anything that thinks like getopt reads as `-t -i -t -l -e`. Git's
conventions (gitcli(7)) are `--long`, `-o`, and no bundling, so this replaces the
parser and every call site.

Now:

- `--long value`, `--long=value`, `-o value`, `-ovalue`.
- Short options are never bundled: `-ab` is not `-a -b`. For an option that takes
  a value, `-mm` is the stuck form and means the value "m"; for a boolean, a
  bundle is an error.
- `-o=value` is refused with a message pointing at `-ov` or `--long=v`, because
  silently taking "=value" is the kind of thing that costs an hour later.
- Options come before arguments, and an option after an argument is an error
  naming the argument rather than something to guess about.
- `--` and `--end-of-options` end option parsing; `-h`/`--help` print the
  subcommand's usage and exit 0; usage mistakes exit 129, as git does.
- `-m`/`-F` mirror `git commit`: mutually exclusive, `-F -` reads stdin.
- `-l` is repeatable; comma-separated still works for convenience.
- `issue list --format` takes `%(id)`, `%(shortid)`, `%(state)`, `%(author)`,
  `%(title)`, `%(labels)`, `%(created)` for scripts.
- `vole.author` is consulted before `user.name` for the default author.

Two bugs came out of the change, both caught by tests rather than by reading:

- The new parser stored `--config` as an ordinary value and never copied it to
  the path the commands actually use, so every command silently fell back to
  ./vole.toml. A test that rebuilds an index in a temp directory failed loudly.
- The integration test itself passed `-config`, which the new parser read as
  `-c onfig` — exactly the ambiguity this commit removes, in the wild.

Verified by hand against a live instance: repeatable `-l`, stuck `--title=x`,
`--format`, `issue comment -m text <id>` accepted, `issue comment <id> -m text`
refused with exit 129 and "options must come before arguments", `-h` exiting 0,
and a missing option value exiting 129.

cmd/vole/flags_test.go +209

@@ -0,0 +1,209 @@
1 package main
2
3 import (
4 "testing"
5 "time"
6
7 "github.com/rodent-software/vole/internal/issues"
8 )
9
10 func issueForTest() *issues.Issue {
11 return &issues.Issue{
12 ID: "01M5JR2JK29KKMAA1T9MDNTPBP",
13 State: "open",
14 Author: "keenan",
15 Title: "A title",
16 Labels: []string{"bug", "ui"},
17 Created: time.Unix(1700000000, 0).UTC(),
18 }
19 }
20
21 func issueSpecs() []flagSpec {
22 return []flagSpec{
23 {long: "title", short: "t"},
24 {long: "message", short: "m"},
25 {long: "file", short: "F"},
26 {long: "label", short: "l"},
27 {long: "no-push", boolean: true},
28 }
29 }
30
31 func TestFlagForms(t *testing.T) {
32 for name, args := range map[string][]string{
33 "separate long": {"--title", "hello"},
34 "stuck long": {"--title=hello"},
35 "separate short": {"-t", "hello"},
36 "stuck short": {"-thello"},
37 } {
38 f, err := parseFlags(args, issueSpecs()...)
39 if err != nil {
40 t.Fatalf("%s: %v", name, err)
41 }
42 if got := f.one("title"); got != "hello" {
43 t.Fatalf("%s: title = %q", name, got)
44 }
45 }
46 }
47
48 func TestRepeatableFlag(t *testing.T) {
49 f, err := parseFlags([]string{"-l", "bug", "--label", "ui", "-lperf"}, issueSpecs()...)
50 if err != nil {
51 t.Fatal(err)
52 }
53 got := f.all("label")
54 if len(got) != 3 || got[0] != "bug" || got[1] != "ui" || got[2] != "perf" {
55 t.Fatalf("labels = %v", got)
56 }
57 }
58
59 func TestBooleanFlag(t *testing.T) {
60 f, err := parseFlags([]string{"--no-push"}, issueSpecs()...)
61 if err != nil {
62 t.Fatal(err)
63 }
64 if !f.has("no-push") {
65 t.Fatal("--no-push not set")
66 }
67 if _, err := parseFlags([]string{"--no-push=yes"}, issueSpecs()...); err == nil {
68 t.Fatal("a boolean flag accepted a value")
69 }
70 }
71
72 // TestOptionsBeforeArguments is the gitcli rule: options come first, and a
73 // trailing option is a mistake rather than something to guess at.
74 func TestOptionsBeforeArguments(t *testing.T) {
75 if _, err := parseFlags([]string{"01M5", "-m", "text"}, issueSpecs()...); err == nil {
76 t.Fatal("option after argument was accepted")
77 } else if !isUsageErr(err) {
78 t.Fatalf("not a usage error: %v", err)
79 }
80 f, err := parseFlags([]string{"-m", "text", "01M5"}, issueSpecs()...)
81 if err != nil {
82 t.Fatalf("options first failed: %v", err)
83 }
84 if len(f.rest) != 1 || f.rest[0] != "01M5" {
85 t.Fatalf("rest = %v", f.rest)
86 }
87 }
88
89 func TestEndOfOptions(t *testing.T) {
90 for _, term := range []string{"--", "--end-of-options"} {
91 f, err := parseFlags([]string{term, "--title", "-m"}, issueSpecs()...)
92 if err != nil {
93 t.Fatalf("%s: %v", term, err)
94 }
95 if len(f.rest) != 2 || f.rest[0] != "--title" || f.rest[1] != "-m" {
96 t.Fatalf("%s: rest = %v", term, f.rest)
97 }
98 if f.one("title") != "" {
99 t.Fatal("option after the terminator was parsed")
100 }
101 }
102 }
103
104 func TestUsageErrors(t *testing.T) {
105 for name, args := range map[string][]string{
106 "unknown long": {"--nope"},
107 "unknown short": {"-Z"},
108 "missing value": {"--title"},
109 "equals on a short opt": {"-l=perf"},
110 } {
111 _, err := parseFlags(args, issueSpecs()...)
112 if err == nil {
113 t.Fatalf("%s: expected an error", name)
114 }
115 if !isUsageErr(err) {
116 t.Fatalf("%s: error is %T, want usageErr", name, err)
117 }
118 }
119 }
120
121 // TestShortOptionsAreNotBundled: -ab is not -a -b. For an option that takes a
122 // value, "-mm" is the stuck form and means the value "m"; for a boolean it is a
123 // mistake, because bundling is not supported.
124 func TestShortOptionsAreNotBundled(t *testing.T) {
125 f, err := parseFlags([]string{"-mm"}, issueSpecs()...)
126 if err != nil {
127 t.Fatalf("-mm should be the stuck form: %v", err)
128 }
129 if f.one("message") != "m" {
130 t.Fatalf("message = %q, want \"m\"", f.one("message"))
131 }
132
133 boolSpecs := []flagSpec{{long: "quiet", short: "q", boolean: true}}
134 if _, err := parseFlags([]string{"-qq"}, boolSpecs...); err == nil {
135 t.Fatal("bundled boolean short options were accepted")
136 }
137 }
138
139 // TestConfigFlagSetsPath: the parser stores flags generically, so --config has to
140 // be copied to cfgPath explicitly. Missing that silently falls back to vole.toml
141 // in the current directory, which is how a test caught it.
142 func TestConfigFlagSetsPath(t *testing.T) {
143 for _, args := range [][]string{
144 {"--config", "/tmp/x.toml"},
145 {"-c", "/tmp/x.toml"},
146 {"--config=/tmp/x.toml"},
147 } {
148 f, err := parseFlags(args, configFlag())
149 if err != nil {
150 t.Fatal(err)
151 }
152 if f.cfgPath != "/tmp/x.toml" {
153 t.Fatalf("%v: cfgPath = %q", args, f.cfgPath)
154 }
155 }
156 f, err := parseFlags(nil, configFlag())
157 if err != nil {
158 t.Fatal(err)
159 }
160 if f.cfgPath != "vole.toml" {
161 t.Fatalf("default cfgPath = %q", f.cfgPath)
162 }
163 }
164
165 func TestHelpFlag(t *testing.T) {
166 for _, a := range []string{"-h", "--help"} {
167 f, err := parseFlags([]string{a}, issueSpecs()...)
168 if err != nil {
169 t.Fatal(err)
170 }
171 if !f.help {
172 t.Fatalf("%s did not set help", a)
173 }
174 }
175 }
176
177 func TestMessageAndFileAreExclusive(t *testing.T) {
178 f, err := parseFlags([]string{"-m", "text", "-F", "file"}, issueSpecs()...)
179 if err != nil {
180 t.Fatal(err)
181 }
182 if _, err := readBody(f); err == nil {
183 t.Fatal("-m with -F was accepted")
184 } else if !isUsageErr(err) {
185 t.Fatalf("not a usage error: %v", err)
186 }
187 }
188
189 func TestNormalizeLabels(t *testing.T) {
190 got := normalizeLabels([]string{"bug,ui", " perf ", ""})
191 want := []string{"bug", "perf", "ui"}
192 if len(got) != len(want) {
193 t.Fatalf("labels = %v", got)
194 }
195 for i := range want {
196 if got[i] != want[i] {
197 t.Fatalf("labels = %v, want %v", got, want)
198 }
199 }
200 }
201
202 func TestFormatIssue(t *testing.T) {
203 iss := issueForTest()
204 out := formatIssue("%(shortid) %(state) %(labels) %(title)", iss)
205 want := "01M5JR2J open bug,ui A title"
206 if out != want {
207 t.Fatalf("format = %q, want %q", out, want)
208 }
209 }