Clarify -o/-O help text and fix two log messages - #128
Conversation
- `-o` and `-O` help: say what each option does to the value, and give an example. "Raw strings" and "interpreted strings" did not tell users which one to use. - `migrate -v`: add help text. - `upload_dir`: remove the double space in `Uploading directory`. - `sync --pull`: add the folder name to `Preparing to download N items from Cloudinary folder`, which ended with a space. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
const-cloudinary
left a comment
const-cloudinary
left a comment
There was a problem hiding this comment.
@TalLevAmi, please improve description of some of the parameters, they can be misleading, especially for naive LLMs
| help="Pass an optional parameter as a string, with no parsing. e.g. -o tags a,b") | ||
| @option("-O", "--optional_parameter_parsed", multiple=True, nargs=2, | ||
| help="Pass optional parameters as interpreted strings.") | ||
| help="Pass an optional parameter and parse its value as JSON or a boolean. e.g. -O context '{\"alt\": \"cat\"}'") |
There was a problem hiding this comment.
Example is misleading, provisioning commands do not support context parameter.
There was a problem hiding this comment.
🤖 pr-nanny (automated): You are correct. The provisioning methods do not accept context. The -O example is now -O enabled true (b1875de).
| @argument("params", nargs=-1) | ||
| @option("-o", "--optional_parameter", multiple=True, nargs=2, help="Pass optional parameters as raw strings.") | ||
| @option("-o", "--optional_parameter", multiple=True, nargs=2, | ||
| help="Pass an optional parameter as a string, with no parsing. e.g. -o tags a,b") |
There was a problem hiding this comment.
Example is misleading, provisioning commands do not support tags parameter.
There was a problem hiding this comment.
🤖 pr-nanny (automated): You are correct. The provisioning methods do not accept tags. The -o example is now -o prefix demo (b1875de).
| help="Pass an optional parameter as a string, with no parsing. e.g. -o tags a,b") | ||
| @option("-O", "--optional_parameter_parsed", multiple=True, nargs=2, | ||
| help="Pass optional parameters as interpreted strings.") | ||
| help="Pass an optional parameter and parse its value as JSON or a boolean. e.g. -O context '{\"alt\": \"cat\"}'") |
There was a problem hiding this comment.
Example is misleading, utils commands do not support context parameter.
There was a problem hiding this comment.
🤖 pr-nanny (automated): You are correct. The utils methods do not accept context. The -O example is now -O sign_url true (b1875de).
The provisioning methods do not accept tags or context, and the utils methods do not accept context. The examples now use prefix, enabled and sign_url.
2 participants
Small text fixes found in a usability review of the CLI.
Brief Summary of Changes
-o, --optional_parameterand-O, --optional_parameter_parsedinadmin,uploader,provisioning,utils,upload_dirandsync: the help says that-odoes not parse the value and that-Oparses it as JSON or a boolean, with an example for each. Before, the help said "raw strings" and "interpreted strings".migrate -v: add help text (Log each migrated URL.).upload_dir: the log line wasUploading directory '...'(two spaces) when-ewas not set.sync --pull: the log linePreparing to download N items from Cloudinary folderhad no folder name. It now shows the folder, as the othersynclines do.What does this PR address?
Are tests included?
Reviewer, please note:
Checklist:
Local test run: I ran the full suite on Python 3.8 with Click 8.1.8. No test asserts these strings. 8 tests in
test_cli_agent.pyandtest_cli_config_oauth.pyfail withstderr not separately capturedon this branch and onmastertoo. These tests need Click 8.2, which needs Python 3.10 or later. CI must confirm the full result.🤖 Generated with Claude Code