revise 'Options' documentation#3192
Conversation
|
I will have to take a closer look at this. At first glance you made some good grammatical fixes, but also made some changes that obscure what is meant. Sometimes it's best not to use the most technical terms available because people may not know what you mean. |
|
I appreciate the feedback, and after re-reading I definitely agree that some of the changes need more work (particularly the Option Decorator section). I have some ideas for improvements that might be a nicer middle ground, but I'll wait for your closer review before making any changes. |
| @@ -24,7 +24,7 @@ Useful and often used kwargs are: | |||
|
|
|||
There was a problem hiding this comment.
I generally avoid using callback unless I have to since a lot of python developers are not software engineers and so probably won't know what it means.
There was a problem hiding this comment.
Agreed that "decorated function" is better than "callback function"
|
@brihall I will got through some of the PR. Will hopefully get to the rest sometime soon. Don't be put off by the lots of changes, I like what I see so far! |
|
@Rowlando13 Not put off at all. I really appreciate you taking the time to give feedback, and the changes were definitely necessary. I applied some small changes in the unreviewed sections so that they're more aligned with your first round of suggestions. |
docs/options.md
Outdated
| 1. If a positional argument name does not have a prefix, it is chosen. | ||
| 2. If a positional argument name starts with with two dashes, the first one given is chosen. | ||
| 3. The first positional argument prefixed with one dash is chosen otherwise. | ||
| 1. If a positional argument is a valid [Python identifier], it is chosen. |
There was a problem hiding this comment.
If a positional argument is a valid [Python identifier] (and thus does not have dashes), it is chosen.
docs/options.md
Outdated
| 2. If a positional argument name starts with with two dashes, the first one given is chosen. | ||
| 3. The first positional argument prefixed with one dash is chosen otherwise. | ||
| 1. If a positional argument is a valid [Python identifier], it is chosen. | ||
| 2. If multiple positional arguments are prefixed with `--`, the one that was declared first is chosen. |
There was a problem hiding this comment.
If multiple positional arguments are prefixed with --, the first one declared is chosen.
docs/options.md
Outdated
| 3. Otherwise, the first positional argument prefixed with `-` is chosen. | ||
|
|
||
| The chosen positional argument is converted to lower case, up to two dashes are removed from the beginning, and other dashes are converted to underscores to get the function argument name. | ||
| [Python identifier]: https://docs.python.org/3/reference/lexical_analysis.html#identifiers |
There was a problem hiding this comment.
Inline this. Python identifier is not likely to be reused.
docs/options.md
Outdated
| ``` | ||
|
|
||
| By using a tuple literal as type, `nargs` gets automatically set to the | ||
| By using a tuple literal as `type`, `nargs` gets automatically set to the |
There was a problem hiding this comment.
By using a tuple literal as the type, nargs gets automatically set to the
docs/options.md
Outdated
|
|
||
| Click can deal with prefix characters besides `-` for options. Click can use | ||
| `/`, `+` as well as others. Note that alternative prefix characters are generally used very sparingly if at all within POSIX. | ||
| Click can deal with prefix characters besides `-` for options, including `/` and `+`. Note that alternative prefix characters are generally used very sparingly if at all within POSIX. |
There was a problem hiding this comment.
Click can deal with prefix characters besides - for options, including / and +, as well as others.
There was a problem hiding this comment.
This is an intentional effort to document / and +, and that there are other but not name them. As it stands many other prefix characters are supported, but I would like to keep it an implementation detail.
|
@brihall Review complete. |
|
@Rowlando13 Thanks! Pushed your suggestions |
|
@Rowlando13 is that one ready to be merged into |
|
@kdeldycke It is. I just have not had a chance to confirm all the requested changes were made so I can click merge. |
|
@brihall I finally got a chance to do a final read through and hit merge. I really appreciate the effort. This section is way better on account of you help. |
3 participants
#3175
If #3191 is approved I'd be happy to go back and handle any merge conflicts caused by this PR's changes.
Clarity improvements
The "Option Decorator" section refers to "positional arguments" of the
click.option()decorator and to the "function argument" of the decorated callback function. Using "positional arguments" as shorthand for "positional arguments passed to theoption()decorator" introduces ambiguity, because the decorated function can also accept positional arguments (and in fact does in every provided example). Using "function" as shorthand for "decorated function" is also ambiguous. References to the positional arguments forclick.option()are changed to "parameter declarations" (for consistency with the API docs), and "function argument" is replaced with the more specific "callback argument".When describing the steps to infer the callback argument name from the parameter declarations, the following is misleading (possibly outdated?):
The actual criteria is that the argument is a valid identifier, i.e.
str.isidentifier()returnsTrue(seeOption._parse_decls()). A string that is "prefixed" with_would therefore be chosen. The step is updated to reflect this and links to Python's "Lexical analysis" documentation.Several sections use "pass in" back-to-back when referencing
option()arguments and command line usage. This makes it unclear when the command line usage is being referenced. When referring tooption(), instead use "declare" for positional arguments or "set" for keyword arguments.Consistency improvements
All instances of "underlying function" are replaced with "decorated function".
References to "dash(es)" are replaced with
-or--. This establishes consistency with the "Other Prefix Characters" section and allows for several sentences to be reworded for concision.Miscellaneous
Minor grammar and concision edits throughout.