|
| 1 | +# Code Style |
| 2 | + |
| 3 | +Thank you for contributing to the Viam CLI! Please keep in mind the following best practices |
| 4 | +when structuring your code. |
| 5 | + |
| 6 | +## Make Arguments Typeful |
| 7 | +Your CLI command's action func should be constructed with `createCommandWithT` and should put |
| 8 | +all command line argument values in a typed struct. This ensures type correctness and reduces |
| 9 | +the risks associated with having contributors manually parse args in each function they write. |
| 10 | + |
| 11 | +### Examples |
| 12 | +<details> |
| 13 | +<summary>Good:</summary> |
| 14 | + |
| 15 | +```golang |
| 16 | +type fooArgs struct { |
| 17 | + Bar int |
| 18 | + Baz string |
| 19 | +} |
| 20 | + |
| 21 | +func fooAction(ctx cli.Context, args foo) error { |
| 22 | + bar := args.Bar |
| 23 | + baz := args.Baz |
| 24 | + ... |
| 25 | +} |
| 26 | + |
| 27 | +... |
| 28 | + |
| 29 | +cli.Command{ |
| 30 | + ... |
| 31 | + Flags: []cli.Flag{ |
| 32 | + &cli.StringFlag{ |
| 33 | + Name: Baz, |
| 34 | + }, |
| 35 | + &cli.IntFlag{ |
| 36 | + Name: Bar, |
| 37 | + }, |
| 38 | + }, |
| 39 | + Action: createCommandWithT[fooArgs](fooAction), |
| 40 | + ... |
| 41 | +} |
| 42 | +``` |
| 43 | +</details> |
| 44 | + |
| 45 | +<details> |
| 46 | +<summary>Bad:</summary> |
| 47 | + |
| 48 | +```golang |
| 49 | +func fooAction(ctx cli.Context) error { |
| 50 | + bar := ctx.Int("Bar") |
| 51 | + baz := ctx.String("Baz) |
| 52 | + ... |
| 53 | +} |
| 54 | +
|
| 55 | +... |
| 56 | +
|
| 57 | +cli.Command{ |
| 58 | + Flags: []cli.Flag{ |
| 59 | + &cli.StringFlag{ |
| 60 | + Name: BazFlag, |
| 61 | + }, |
| 62 | + &cli.IntFlag{ |
| 63 | + Name: BarFlag, |
| 64 | + }, |
| 65 | + }, |
| 66 | + Action: fooAction, |
| 67 | + ... |
| 68 | +} |
| 69 | +``` |
| 70 | +</details> |
| 71 | +
|
| 72 | +## Avoid Flag Bloat |
| 73 | +We have a significant number of flags in the CLI already, many of which (e.g., `organization` or |
| 74 | +`location`) are used in multiple functions. Instead of adding duplicate flags, prefer reusing |
| 75 | +existing flags. If necessary, rename then to indicate their more generic usage. |
| 76 | +
|
| 77 | +### Example |
| 78 | +<details> |
| 79 | +<summary>Good:</summary> |
| 80 | +
|
| 81 | +```diff |
| 82 | +-const yourSpecialFlag = "some-cool-flag" |
| 83 | ++const generalSpecialFlag = "some-cool-flag" |
| 84 | +``` |
| 85 | +</details> |
| 86 | +
|
| 87 | +<details> |
| 88 | +<summary>Bad:</summary> |
| 89 | +
|
| 90 | +```golang |
| 91 | +const yourSpecialFlag = "some-cool-flag" |
| 92 | +... |
| 93 | +const mySpecialFlag = "some-cool-flag" |
| 94 | +``` |
| 95 | +</details> |
| 96 | +
|
| 97 | +## Hide Help From Commands With Subcommands |
| 98 | +When a parent level command with child commands exists (e.g., `viam organizations` has `list`, |
| 99 | +`logo`, `api-key`, and others as child commands), it makes little sense to show a `help` command |
| 100 | +since the parent level command doesn't do anything on its own. |
| 101 | +
|
| 102 | +### Example |
| 103 | +<details> |
| 104 | +<summary>Necessary diff:</summary> |
| 105 | +
|
| 106 | +```diff |
| 107 | +cli.Command{ |
| 108 | + ... |
| 109 | + Name: "my-parent-command", |
| 110 | ++ HideHelpCommand: true, |
| 111 | + Subcommands: []*cli.Command{ |
| 112 | + { |
| 113 | + Name: "my-child-command1", |
| 114 | + ... |
| 115 | + }, |
| 116 | + { |
| 117 | + Name: "my-child-command2", |
| 118 | + ... |
| 119 | + }, |
| 120 | + }, |
| 121 | +} |
| 122 | +``` |
| 123 | +</details> |
| 124 | +
|
| 125 | +## Create Usage Text With Helper Func, Only Specify Required Args |
| 126 | +When creating usage text for a CLI command, use the `createUsageText` convenience method |
| 127 | +to generate text. Be sure to provide the fully qualified command (less `viam`) as your first |
| 128 | +argument, and only include actually required flags in the `requiredFlags` argument. |
| 129 | +
|
| 130 | +Additionally, be sure to use the `formatAcceptedValues` convenience method for defining usage |
| 131 | +text on a flag where only a discrete set of values are permitted. |
| 132 | +
|
| 133 | +### Example |
| 134 | +<details> |
| 135 | +<summary>Diff:</summary> |
| 136 | +
|
| 137 | +```diff |
| 138 | +cli.Command{ |
| 139 | + ... |
| 140 | + Name: "my-parent-command", |
| 141 | + Subcommands: []*cli.Command{ |
| 142 | + Name: "my-child-command", |
| 143 | + Flags: []cli.Flag{ |
| 144 | + &cli.StringFlag{ |
| 145 | + Name: requiredFlag, |
| 146 | + Required: true, |
| 147 | ++ Usage: formatAcceptedValues("passes some required value", 'foo', 'bar, 'baz') |
| 148 | +- Usage: "passes some required value. must be either 'foo', 'bar', or 'baz'" |
| 149 | + }, |
| 150 | + &cli.StringFlag{ |
| 151 | + Name: optionalFlag, |
| 152 | + }, |
| 153 | + }, |
| 154 | ++ UsageText: createUsageText("my-parent-command my-child-command", []string{requiredFlag}, true, false), |
| 155 | +- UsageText: createUsageText("my-child-command", []string{requiredFlag, optionalFlag}, false, false), |
| 156 | + ... |
| 157 | + } |
| 158 | +} |
| 159 | +``` |
| 160 | +</details> |
| 161 | +
|
| 162 | +## Use `DefaultText` Field For Defining Default values |
| 163 | +Instead of adding extra text to the description of what a field does, we should use the automated |
| 164 | +formatting provided by the `DefaultText` field. |
| 165 | +
|
| 166 | +### Examples |
| 167 | +<details> |
| 168 | +<summary>Good:</summary> |
| 169 | +
|
| 170 | +```golang |
| 171 | +cli.StringFlag{ |
| 172 | + Name: fooFlag, |
| 173 | + Usage: "sets value of Foo", |
| 174 | + DefaultText: "foo", |
| 175 | +} |
| 176 | +``` |
| 177 | +</details> |
| 178 | +
|
| 179 | +<details> |
| 180 | +<summary>Bad:</summary> |
| 181 | +
|
| 182 | +```golang |
| 183 | +cli.StringFlag{ |
| 184 | + Name: fooFlag, |
| 185 | + Usage: "sets value of Foo (defaults to foo)", |
| 186 | +} |
| 187 | +``` |
| 188 | +</details> |
0 commit comments