| Message ID | pull.1220.git.git.1645853661519.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | fix: include the type flag in the cli docs | expand |
On 26/02/22 12.34, Matheus Felipe via GitGitGadget wrote: > From: Matheus Felipe <matheusfelipeog@protonmail.com> > > When the `git config --global --help` command is invoked, > the cli documentation is shown in the terminal with a small > error in one of the values of the Type group, which is the > absence of the type flag in the `--type` argument. > This commit fixes that. > What about the commit message below? ``` The usage help for --type option of `git config` is missing `type` in the argument placeholder (`<>`). Add it. ``` > - OPT_CALLBACK('t', "type", &type, "", N_("value is given this type"), option_parse_type), > + OPT_CALLBACK('t', "type", &type, N_("type"), N_("value is given this type"), option_parse_type), The help should be `give the value the specified type`.
Bagas Sanjaya <bagasdotme@gmail.com> writes: > On 26/02/22 12.34, Matheus Felipe via GitGitGadget wrote: >> From: Matheus Felipe <matheusfelipeog@protonmail.com> >> When the `git config --global --help` command is invoked, >> the cli documentation is shown in the terminal with a small >> error in one of the values of the Type group, which is the >> absence of the type flag in the `--type` argument. >> This commit fixes that. >> > > What about the commit message below? > > ``` > The usage help for --type option of `git config` is missing `type` > in the argument placeholder (`<>`). Add it. > ``` It is more concise, and at the same time points out the problem being addressed a lot more explicitly. Much better. >> - OPT_CALLBACK('t', "type", &type, "", N_("value is given this type"), option_parse_type), >> + OPT_CALLBACK('t', "type", &type, N_("type"), N_("value is given this type"), option_parse_type), > > > The help should be `give the value the specified type`. I am not sure if this is much of an improvement. $ git config --type=bool junk.flag 0 $ git config junk.flag false uses the type information to turn "0" into "false" before it writes the value set to the variable to the file, while $ git config junk.flag 0 $ git config junk.flag 0 $ git config --type=bool junk.flag false shows that a stored value of "0" can be turned into "false" when showing. "Give the value the specified type" does not capture the essense in either direction. Before setting or showing, convert the value to its canonical representation according to the given type. is what we want to convey, but it is quote a mouthful as-is. Saying "Assume the value is of this type" would strongly imply "Convert ... to its canonical reporesentation", and the current "value is given this type" may be a close enough and shorter approximation of it. I dunno.
On Sun, Feb 27 2022, Junio C Hamano wrote: > Bagas Sanjaya <bagasdotme@gmail.com> writes: > >> On 26/02/22 12.34, Matheus Felipe via GitGitGadget wrote: >>> From: Matheus Felipe <matheusfelipeog@protonmail.com> >>> When the `git config --global --help` command is invoked, >>> the cli documentation is shown in the terminal with a small >>> error in one of the values of the Type group, which is the >>> absence of the type flag in the `--type` argument. >>> This commit fixes that. >>> >> >> What about the commit message below? >> >> ``` >> The usage help for --type option of `git config` is missing `type` >> in the argument placeholder (`<>`). Add it. >> ``` > > It is more concise, and at the same time points out the problem > being addressed a lot more explicitly. Much better. > >>> - OPT_CALLBACK('t', "type", &type, "", N_("value is given this type"), option_parse_type), >>> + OPT_CALLBACK('t', "type", &type, N_("type"), N_("value is given this type"), option_parse_type), >> >> >> The help should be `give the value the specified type`. > > I am not sure if this is much of an improvement. > > $ git config --type=bool junk.flag 0 > $ git config junk.flag > false > > uses the type information to turn "0" into "false" before it writes > the value set to the variable to the file, while > > $ git config junk.flag 0 > $ git config junk.flag > 0 > $ git config --type=bool junk.flag > false > > shows that a stored value of "0" can be turned into "false" when > showing. "Give the value the specified type" does not capture the > essense in either direction. > > Before setting or showing, convert the value to its canonical > representation according to the given type. > > is what we want to convey, but it is quote a mouthful as-is. > > Saying "Assume the value is of this type" would strongly imply > "Convert ... to its canonical reporesentation", and the current > "value is given this type" may be a close enough and shorter > approximation of it. I dunno. Perhaps: "coerce (on read and write) <value> to <type>" or: "coerce (on read & write) <value> to <type>" or: "coerce (on rw) <value> to <type>" For the short help, depending on how verbose we'd like to be? In any case a follow-up fix, just the "" to "type" being proposed here is orthagonal & looks good to me.
> On 26/02/22 12.34, Matheus Felipe via GitGitGadget wrote: > > > From: Matheus Felipe matheusfelipeog@protonmail.com > > When the `git config --global --help` command is invoked, > > the cli documentation is shown in the terminal with a small > > error in one of the values of the Type group, which is the > > absence of the type flag in the `--type` argument. > > This commit fixes that. > > What about the commit message below? > > ``` > The usage help for --type option of `git config` is missing `type` > in the argument placeholder (`<>`). Add it. > ``` This commit message just got better, thanks. I will update with this message. > > - OPT_CALLBACK('t', "type", &type, "", N_("value is given this type"), option_parse_type), > > + OPT_CALLBACK('t', "type", &type, N_("type"), N_("value is given this type"), option_parse_type), > > The help should be `give the value the specified type`. About the help message, I believe the current one is adequate, as Junio C Hamano well described. It is also worth remembering that PR proposes a change from "" to "type" only, as recalled by Ævar Arnfjörð Bjarmason.
diff --git a/builtin/config.c b/builtin/config.c index 542d8d02b2b..2aea465466b 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -151,7 +151,7 @@ static struct option builtin_config_options[] = { OPT_BIT(0, "get-color", &actions, N_("find the color configured: slot [default]"), ACTION_GET_COLOR), OPT_BIT(0, "get-colorbool", &actions, N_("find the color setting: slot [stdout-is-tty]"), ACTION_GET_COLORBOOL), OPT_GROUP(N_("Type")), - OPT_CALLBACK('t', "type", &type, "", N_("value is given this type"), option_parse_type), + OPT_CALLBACK('t', "type", &type, N_("type"), N_("value is given this type"), option_parse_type), OPT_CALLBACK_VALUE(0, "bool", &type, N_("value is \"true\" or \"false\""), TYPE_BOOL), OPT_CALLBACK_VALUE(0, "int", &type, N_("value is decimal number"), TYPE_INT), OPT_CALLBACK_VALUE(0, "bool-or-int", &type, N_("value is --bool or --int"), TYPE_BOOL_OR_INT),