cli: conventionalize on `<command>` in cmd.Use #7143

grouville · 2024-04-19T19:43:19Z

One of the action items discussed in #7107 (comment)

Unifies the DX around usage across all commands. The convention follows the standards (gh / git CLIs) as well as the defaults from cobra. More context from @helderco in above comment

Proposal

The grammar is as follow:

[]: optional
<>: user input, to differenciate with static args
...: one or more

Another point: [flags] shall always be positioned directly after a command. Or, at least to have a consistent positionning across all commands.

Context

This PR applies the grammar everywhere for consistency. And, fixes the dagger gen and dagger config views command where flags were not positioned in the same order as all the other commands.

1. Open for debate / proposition

Both for functions and call, I took the liberty to modify the usage a bit to be more descriptive.

USAGE
  dagger functions [flags] [<function> [<child-function>...]]

USAGE
  dagger call [flags] [<function> [<child-function>...]]
  dagger call [flags] [<command>]

The origin of this change is that I got suprised when testing dagger functions: I tested to chain two functions at the same level, which the previous usage could suggest:

dagger functions [flags] [<function>]...

I am totally fine on keeping that format too, especially if you feel that it is easier to read and less confusing overall. You are the DX master 👼

The confusing part can be showed with below example. With the previous usage, this could be understood as a valid command, where base and debug are two top-level functions from the mariadb module:
dagger -m github.com/levlaz/daggerverse/[email protected] functions base debug
The proposed usage might be more accurate / precise but less easy to understand at first

The terminology could also be:

<child-function>
<derived-function>
<sub-function>

2. dagger config

It seems that the dagger config subcommand, which is still experimental and with hidden subcommands might need an issue on its own as the behavior of presenting sub-commands and the flags ordering seems a bit different.

Update 04/23/2024

I added a new commit following Cobra's usage

Recommended syntax is as follows:
[ ] identifies an optional argument. Arguments that are not enclosed in brackets are required.
... indicates that you can specify multiple values for the previous argument.
| indicates mutually exclusive information. You can use the argument to the left of the separator or the
argument to the right of the separator. You cannot use both arguments in a single use of the command.
{ } delimits a set of mutually exclusive arguments when one of the arguments is required. If the arguments are
optional, they are enclosed in brackets ([ ]).
Example: add [-F file | -D dir]... [-f format] profile

But without differentiating mandatory and optional arguments, as proposed in your comment below: #7143 (comment). I also reverted the functions and the call usage as they were before

Update 05/02/2024

Following the discussion on the issue, proposed syntax:

[] optionality
<> non optional
every flag / positional argument has to have either [] or <> to differenciate with commands

Also, another inherent rule that you proposed:

Do not show [options] on commands that just have inherited options

helderco · 2024-04-20T02:31:59Z

call and functions are unique in that they have a dynamic and recursive amount of arguments/commands. Even if in functions they're positional arguments, unlike call where they're actual sub-commands, what trumps here is that they both represent the same function chaining that may take a new user a bit of getting used to or learn about.

For other cases like dagger config, just follow what docker does. Just dagger config [flags] [command]. That is:

if it has flags add [flags]
if it has sub-commands add [command]
If you do --help on a sub-command, it's the same, just the path of the current command changes

That's it. It should actually be the default in the template.

Git for example, shows all the options in the usage. In our case I say only if there's a clear benefit in helping the user learn how to use the command, otherwise just leave the default.

grouville · 2024-04-20T07:02:53Z

For other cases like dagger config, just follow what docker does. Just dagger config [flags] [command]. That is:

if it has flags add [flags]

if it has sub-commands add [command]

Ok 👌 👼 . Just to confirm, docker --help shows:

Run 'docker COMMAND --help' for more information on a command.

We keep representing it as such:
dagger [command] --help

But, for the commands with a mandatory input such as dagger install [flags] MODULE, how would you represent it ?
==> Is it dagger install [flags] module ? Or do we not make any distinction: dagger install [flags] [module]

So, to summarize, usage is now:
1. login
dagger login [flags]
2. logout
dagger logout [flags]
3. develop
dagger develop [flags]
4. init
dagger init [flags] [path]
5. install
dagger install [flags] module
6. query
dagger query [flags] [operation]
7. run
dagger run [flags] command
8. version
dagger version [flags]
9. call

dagger call [flags] [function]...
dagger call [flags] [command]

with chained functions:

dagger call base [flags]
dagger call base [flags] [command]

10. functions
dagger functions [flags] [function]...

helderco

Update 04/23/2024

I added a new commit following Cobra's usage

Recommended syntax is as follows:
[ ] identifies an optional argument. Arguments that are not enclosed in brackets are required.

I’m not fully convinced on the second statement yet. It’s more common to use <> as placeholder, see (POSIX) Utility Conventions, or at least put it in caps.

In dagger install module how do you know if module is something you have to type or replace? dagger install <module> looks much clearer to me. If the angle brackets are just ugly, that’s why I actually prefer dagger install MODULE, but as I’ve pointed out before, that may require more changes to go against Cobra’s default.

How about this, can you tell which is a command and which is an argument (assume only global flags exist here)?

dagger config views name set value

How about now?

dagger config views <name> set <value>

In the first one you rely on names that helps your brain fill in the blank, while in the second it’s more immediately obvious.

Lastly, I want to point out that adding [flags] may help understand where the split is:

dagger run command
# vs
dagger run [flags] command

But then it may not apply in some case so I try to picture this without it:

dagger run command
# vs
dagger run <command>

But without differentiating mandatory and optional arguments, as proposed in your comment below: #7143 (comment).

That’s not what I suggested.

For other cases like dagger config, just follow what docker does. Just dagger config [flags] [command]. That is:

if it has flags add [flags]

if it has sub-commands add [command]

Ok 👌 👼 . Just to confirm, docker --help shows:
Run 'docker COMMAND --help' for more information on a command.
We keep representing it as such: dagger [command] --help

This is correct, but find me a docker command that has subcommands and does something on its own. Most docker commands that have subcommands seem to be used only for grouping those subcommands, so not something you’d use directly. In that case it makes sense to mark the subcommand as required.

But dagger call and dagger config have valid usages without subcommands, thus the [] in [command].

But, for the commands with a mandatory input such as dagger install [flags] MODULE, how would you represent it ? ==> Is it dagger install [flags] module ? Or do we not make any distinction: dagger install [flags] [module]

Yes, we make the distinction.

To be clear, when I gave docker’s example it wasn’t to replicate the syntax or to forcibly adopt the “required” subcommand explicitly, it was to replicate the simplicity of delegating details to subcommands’ --help, rather than be more verbose on the parent command’s usage.

You always need to take into account if you’re comparing apples to apples in regards to the syntax of different tools/clis.

helderco · 2024-04-24T00:20:15Z

cmd/dagger/call.go

@@ -16,7 +16,7 @@ var outputPath string
 var jsonOutput bool

 var callCmd = &FuncCommand{
- Name: "call [flags] [FUNCTION]...",
+ Name: "call [flags] [function]...",


This will produce:

dagger call [flags] [function]... dagger call [flags] [command]

Which isn't right because here, "function" and "command" are the same thing. As per the conventions:

When multiple synopsis lines are given for a utility, it is an indication that the utility has mutually-exclusive arguments.

This happens because you're describing function as a positional argument, when it's actually a subcommand. In the template, cobra sees that it has subcommands so it adds the second line.

Docker, for example, doesn't print two lines. It always prints the same if it has subcommands:

{{- if not .HasSubCommands}} {{.UseLine}}{{end}} {{- if .HasSubCommands}} {{ .CommandPath}}{{- if .HasAvailableFlags}} [OPTIONS]{{end}} COMMAND{{end}}

In our case, we'd have to check if the command is required or not. That's a plus in docker for consistency that we don't have.

In this case the subcommand is indeed optional, but you mark that by making the usage about the call command itself and not include children. If you change the usage to just call and remove the square brackets in [command] in the template, it produces this which is more correct:

dagger call [flags] dagger call <command> [flags]

But Cobra's default is actually [command] and in the cases where the command is actually required, then the first line doesn't make sense.

Since we have a mix (sometims required, sometimes optional), we may have to use a helper function to make a single usage line instead, that detects which applies. But I suggest keeping it for another PR and simply keep the usage here as:

Suggested change

Name: "call [flags] [function]...",

Name: "call",

Now that [flags] was renamed to [options], we need to add it explicitly. As for the two usage lines in the template, I don't think we can generalize because we have a command that requires a subcommand, and another where it's optional. Better be explicit in cmd.Use. I pushed an update with these changes so I can merge this quickly:

- Name: "call", + Name: "call [flags] [command]",

Notice that I'll rename [command] to [function] here in another PR.

cmd/dagger/module.go

cmd/dagger/run.go

cmd/dagger/watch.go

docs/current_docs/cli/979595-reference.mdx

cmd/shim/main.go

helderco · 2024-04-27T11:24:53Z

@grouville, let me know when you're ready for another review 🙏

Unifies the DX around usage across all commands. The convention follows the standards (gh / git CLIs) as well as the defaults from cobra. The grammar is as follow: - []: optional - <>: user input, when argument is not optional, to differenciate with static args - ...: one or more This commit applies it everywhere and unifies according to this grammar. Signed-off-by: grouville <[email protected]>

cmd/dagger/call.go

cmd/dagger/query.go

Signed-off-by: Helder Correia <[email protected]>

helderco

I pushed an update to get this merged quickly

Sometimes a subcommand is required (`dagger call`), and other times it's optional (`dagger config`). Best make it explicit in `Use`. Signed-off-by: Helder Correia <[email protected]>

Signed-off-by: Helder Correia <[email protected]>

* cli: conventionalize on <command> in cmd.Use Unifies the DX around usage across all commands. The convention follows the standards (gh / git CLIs) as well as the defaults from cobra. The grammar is as follow: - []: optional - <>: user input, when argument is not optional, to differenciate with static args - ...: one or more This commit applies it everywhere and unifies according to this grammar. Signed-off-by: grouville <[email protected]> * Fix incorrections Signed-off-by: Helder Correia <[email protected]> * Remove second use line Sometimes a subcommand is required (`dagger call`), and other times it's optional (`dagger config`). Best make it explicit in `Use`. Signed-off-by: Helder Correia <[email protected]> * Fix required mark when flag has no description Signed-off-by: Helder Correia <[email protected]> --------- Signed-off-by: grouville <[email protected]> Signed-off-by: Helder Correia <[email protected]> Co-authored-by: grouville <[email protected]> Co-authored-by: Helder Correia <[email protected]>

grouville requested a review from helderco April 19, 2024 19:43

grouville force-pushed the cli-conventionalize-command branch from 8d797c3 to dd63730 Compare April 19, 2024 19:49

grouville mentioned this pull request Apr 20, 2024

De-clutter dagger call --help #6943

Open

13 tasks

grouville force-pushed the cli-conventionalize-command branch from 465bd9e to f2bdbf3 Compare April 23, 2024 17:32

grouville mentioned this pull request Apr 23, 2024

cli: rename flags to options #7170

Merged

helderco reviewed Apr 24, 2024

View reviewed changes

grouville force-pushed the cli-conventionalize-command branch from f2bdbf3 to 1c3c1cc Compare April 27, 2024 00:55

grouville force-pushed the cli-conventionalize-command branch from 1c3c1cc to 5e64783 Compare May 2, 2024 19:55

grouville requested a review from helderco May 2, 2024 20:15

grouville force-pushed the cli-conventionalize-command branch 2 times, most recently from ff2db86 to 532c6f9 Compare May 3, 2024 17:22

grouville mentioned this pull request May 4, 2024

CLI: simplify and clarify usage text #7277

Open

helderco changed the title ~~cli: conventionalize on <command> in cmd.Use~~ cli: conventionalize on <command> in cmd.Use May 5, 2024

helderco reviewed May 5, 2024

View reviewed changes

cmd/dagger/call.go Outdated Show resolved Hide resolved

cmd/dagger/query.go Outdated Show resolved Hide resolved

Fix incorrections

cb16f6e

Signed-off-by: Helder Correia <[email protected]>

helderco force-pushed the cli-conventionalize-command branch from 532c6f9 to cac69b6 Compare May 5, 2024 22:59

helderco approved these changes May 5, 2024

View reviewed changes

helderco added 2 commits May 5, 2024 23:10

Remove second use line

a8cb391

Sometimes a subcommand is required (`dagger call`), and other times it's optional (`dagger config`). Best make it explicit in `Use`. Signed-off-by: Helder Correia <[email protected]>

Fix required mark when flag has no description

0573d7c

Signed-off-by: Helder Correia <[email protected]>

helderco force-pushed the cli-conventionalize-command branch from cac69b6 to 0573d7c Compare May 5, 2024 23:11

helderco merged commit 8b658fd into dagger:main May 5, 2024
60 of 64 checks passed

helderco mentioned this pull request May 5, 2024

cli: use "functions" and "arguments" instead of "commands" and "options" in dagger call --help #7286

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cli: conventionalize on `<command>` in cmd.Use #7143

cli: conventionalize on `<command>` in cmd.Use #7143

grouville commented Apr 19, 2024 •

edited

helderco commented Apr 20, 2024

grouville commented Apr 20, 2024 •

edited

helderco left a comment •

edited

helderco Apr 24, 2024

helderco May 5, 2024 •

edited

helderco commented Apr 27, 2024

helderco left a comment

cli: conventionalize on <command> in cmd.Use #7143

cli: conventionalize on <command> in cmd.Use #7143

Conversation

grouville commented Apr 19, 2024 • edited

Proposal

Context

1. Open for debate / proposition

2. dagger config

Update 04/23/2024

Update 05/02/2024

helderco commented Apr 20, 2024

grouville commented Apr 20, 2024 • edited

helderco left a comment • edited

Choose a reason for hiding this comment

Update 04/23/2024

helderco Apr 24, 2024

Choose a reason for hiding this comment

helderco May 5, 2024 • edited

Choose a reason for hiding this comment

helderco commented Apr 27, 2024

helderco left a comment

Choose a reason for hiding this comment

cli: conventionalize on `<command>` in cmd.Use #7143

cli: conventionalize on `<command>` in cmd.Use #7143

grouville commented Apr 19, 2024 •

edited

grouville commented Apr 20, 2024 •

edited

helderco left a comment •

edited

helderco May 5, 2024 •

edited