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

Add documentation on tricks and traps of using optional options #1332

Merged
merged 12 commits into from
Sep 13, 2020

Conversation

heyjiawei
Copy link
Contributor

Pull Request

Extra documentation: tricks and traps with options with optional values #1315

Problem

Solution

ChangeLog

@heyjiawei heyjiawei changed the base branch from master to develop August 18, 2020 16:09
There is parsing ambiguity when using option as boolean flag and also having it accept operands and subcommands.

```
program.command('example')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest change the example to taking an argument, and no subcommands. I think that will be the common problem case, whether it is a single level command, or down in a subcommand.

Also, I would like it to look like it does something. My boring description you worked from was `example -o optionalValue' but it is more fun when the program looks more interesting. I struggle to come up with nice examples!

For this section we just need one optional value and one optional argument, so the command should make sense with one, or the other, or both.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh what is the difference between optional value and optional argument? I though they are the same thing
.option("-o, --option [optionalValue]")

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, that raises another thing I had been thinking about: terminology!

When I wrote "optional argument" I was intending to mean a non-option argument, sometimes called an "operand" or "positional argument". I am not very familiar with either term, and wonder whether "non-option argument" is simple and clear.

What I meant was a non-option argument, like:

program.arguments('[file]');

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll dump some research on terminology in here for reference. I haven't decided what terms to recommend yet!

A number of sources use "positional" as a qualifier, which I assume is meaning in particular that the order matters.


https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap12.html
The utility in the example is named utility_name. It is followed by options, option-arguments, and operands. The arguments that consist of characters and single letters or digits, such as 'a', are known as "options" (or, historically, "flags"). Certain options are followed by an "option-argument", as shown with [ -c option_argument]. The arguments following the last options and option-arguments are named "operands".

https://docs.python.org/3/library/argparse.html
The add_argument() method must know whether an optional argument, like -f or --foo, or a positional argument, like a list of filenames, is expected. The first arguments passed to add_argument() must therefore be either a series of flags, or a simple argument name. For example, an optional argument could be created like:

yargs
.options()
.command()
.positional()

https://oclif.io/docs
Arguments are positional arguments passed to the command.
Flag options are non-positional arguments passed to the command. Flags can either be option flags which take an argument, or boolean flags which do not. An option flag must have an argument.

https://github.com/spf13/cobra#concepts
Cobra is built on a structure of commands, arguments & flags.
Commands represent actions, Args are things and Flags are modifiers for those actions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay, I understand the term you are referring to now. I added a terminology section explaining only the terms used in this document.

I usually clump the terms together if they mean about the same thing. This is good if we come from different backgrounds because we would have something easy to cross-reference to.

I also add a new terminology if I find myself giving it an alias; in this case the borrowed alias "optional options" though I'm not sure if the terminology is well explained

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we get the terms nice and clean, I'll be updating the README to be more consistent! I have been trying hard to be clear there, but I haven't got it consistent yet.

Mentioning terms used in other places is potentially useful, but I don't want "our" terms and "other" terms just listed mixed together. Perhaps, "sometimes called", like:

operand (sometimes called a positional argument)

I do not see "operand" used very much apart from the Open Group, and currently considering "command argument" to match up quite nicely with Command.arguments().

I had been trying to reduce use of "argument" because seemed different before and after parsing, but maybe qualifying could work. So there are CLI arguments before parsing (e.g. process.args), option arguments, and command arguments.

@shadowspawn
Copy link
Collaborator

I'll work on a couple of things at at a time. I have quite a lot I want to cover with this document and it is a bit subtle, and I'm working some of it out as we go. You can pass over parts or the whole thing if you aren't finding it worthwhile to work on.

# Tricks and traps when using options with optional values

There are potential challenges using options with optional values. They seem quite attractive and the README used to use them more than options with require values but in practice, they are a bit tricky and aren't a free choice.

Copy link
Collaborator

@shadowspawn shadowspawn Aug 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This advice applies to variadic options too. First go at adding mention of variadic...

Options can have a single optional value, but variadic options also take extra option values and have the same parsing complications.

.option('-c, --copies [number]') // 0 or 1 value
.option('h, --headers [files]') // 0 or more values
.option('-s, --source <files>') // 1 or more values

@shadowspawn
Copy link
Collaborator

I wondered about explaining how option values are processed by Commander. I'll write something down as a first draft...

Commander looks for optional values in a "greedy" way — the next argument is taken unless it starts with a dash. This works very naturally when there are only options and option values. However, when there is a mixture of non-options and options in the arguments, the user needs to do a bit extra to clarify which are plain arguments and not option values.

Copy link
Contributor Author

@heyjiawei heyjiawei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

explaining how option values are processed by Commander.

I suggest adding a state machine diagram so users can see how the data flows and trace it

To reduce such ambiguity, you can do the following:

1. always use `--` before operands
2. add your options after operands
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After reading the terminology, I realise this is wrong. Shouldn't it be "add your options before operands" ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The posix convention is that options always come before operands. The GNU utility convention allows options to come before or after the operands. Commander follows the GNU convention and allows options before or after the operands. So by putting the options last, the option values do not get confused with the operands.

So instead of writing for a --sugar option taking an option count:

brew --sugar -- tea

you can write without needing the dash-dash:

brew tea --sugar

There is parsing ambiguity when using option as boolean flag and also having it accept operands and subcommands.

```
program.command('example')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay, I understand the term you are referring to now. I added a terminology section explaining only the terms used in this document.

I usually clump the terms together if they mean about the same thing. This is good if we come from different backgrounds because we would have something easy to cross-reference to.

I also add a new terminology if I find myself giving it an alias; in this case the borrowed alias "optional options" though I'm not sure if the terminology is well explained

@shadowspawn shadowspawn self-assigned this Aug 25, 2020
@shadowspawn
Copy link
Collaborator

(I will hopefully spend some time this weekend to get deeper into this.)

@shadowspawn
Copy link
Collaborator

shadowspawn commented Aug 30, 2020

I have made a start on a big edit! I'll get further into the document before I do a Pull Request against your branch. I left the code examples out of the terminology because I wanted to try focus on the command line rather than the code.

https://github.com/shadowspawn/commander.js/blob/feature/more-about-options-fork/docs/options-taking-varying-arguments.md

Copy link
Collaborator

@shadowspawn shadowspawn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@shadowspawn
Copy link
Collaborator

Thanks @heyjiawei

FYI @abetomo , I have not decided where to put the terminology section which is why it has a "work in progress" comment. I'm thinking on a separate page, and link to it from this page and the README. I'm try that out after this PR lands, and link to this page from the README too.

Copy link
Collaborator

@abetomo abetomo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍
Thanks.

@shadowspawn shadowspawn merged commit 3998dd6 into tj:develop Sep 13, 2020
@shadowspawn shadowspawn removed their assignment Sep 13, 2020
@shadowspawn shadowspawn added the pending release Merged into a branch for a future release, but not released yet label Sep 13, 2020
@shadowspawn shadowspawn removed the pending release Merged into a branch for a future release, but not released yet label Oct 25, 2020
@shadowspawn
Copy link
Collaborator

Released in Commander v6.2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants