| Message ID | 20240724111116.91615-1-ash@kambanaria.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 988587124846c7c1d8ab094995604840ac6cbe74 |
| Headers | show |
| Series | [1/1] show-ref: improve short help messages of options | expand |
On Wed, Jul 24, 2024 at 02:11:11PM +0300, Alexander Shopov wrote: > diff --git a/builtin/show-ref.c b/builtin/show-ref.c > index 839a5c29f3..85700caae9 100644 > --- a/builtin/show-ref.c > +++ b/builtin/show-ref.c > @@ -293,8 +293,8 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix) > struct show_one_options show_one_opts = {0}; > int verify = 0, exists = 0; > const struct option show_ref_options[] = { > - OPT_BOOL(0, "tags", &patterns_opts.tags_only, N_("only show tags (can be combined with branches)")), > - OPT_BOOL(0, "branches", &patterns_opts.branches_only, N_("only show branches (can be combined with tags)")), > + OPT_BOOL(0, "tags", &patterns_opts.tags_only, N_("only show tags (can be combined with --branches)")), > + OPT_BOOL(0, "branches", &patterns_opts.branches_only, N_("only show branches (can be combined with --tags)")), > OPT_HIDDEN_BOOL(0, "heads", &patterns_opts.branches_only, > N_("deprecated synonym for --branches")), > OPT_BOOL(0, "exists", &exists, N_("check for reference existence without resolving")), Agreed, I think that this is a sensible change. In practice, this feature can also be combined with `--head`, so in that spirit we might even change it to: "only show tags (can be combined with --branches and --head)" Not sure though whether this is getting too noisy? Patrick
Patrick Steinhardt <ps@pks.im> writes: > Agreed, I think that this is a sensible change. In practice, this > feature can also be combined with `--head`, so in that spirit we might > even change it to: > > "only show tags (can be combined with --branches and --head)" > > Not sure though whether this is getting too noisy? It is somewhat an oxymoron that "*only* show X" can be combined with "*only* show Y" in the first place. For a reader to accept it without finding it awkward, the reader must understand that (1) the command shows by default everything, but (2) if any of these "only show" options are given, the command stops showing everything and the user can pick which subset of "only show" options to give, which work additively. But if the reader knows that much already, it is redundant to say "can be combined with", isn't it? So, I dunno.
On Thu, Jul 25, 2024 at 08:03:06AM -0700, Junio C Hamano wrote: > Patrick Steinhardt <ps@pks.im> writes: > > > Agreed, I think that this is a sensible change. In practice, this > > feature can also be combined with `--head`, so in that spirit we might > > even change it to: > > > > "only show tags (can be combined with --branches and --head)" > > > > Not sure though whether this is getting too noisy? > > It is somewhat an oxymoron that "*only* show X" can be combined with > "*only* show Y" in the first place. Yeah, this is the root of the problem why those braces are required in the first place. But that being said, the option is somewhat funny because it indeed limits what we show to only show tags. The fact that it can be combined with other options to expand on what it shows doesn't change that, so I think "only" is okay-ish even though it certainly is not perfect. > For a reader to accept it without finding it awkward, the reader > must understand that > > (1) the command shows by default everything, but > > (2) if any of these "only show" options are given, the command > stops showing everything and the user can pick which subset of > "only show" options to give, which work additively. > > But if the reader knows that much already, it is redundant to say > "can be combined with", isn't it? Hum. I personally find the way that this is worded intuitive and think that I would find it helpful when reading it as a less-knowledgeable user. But naturally, I'm quite biased and may be too unimaginative to come up with a better wording. Meanwhile, I think that the proposed change strictly improves this message and thus don't see a reason not to take it. Unless we see somebody come up with a less-awkward solution, that is. Patrick
Patrick Steinhardt <ps@pks.im> writes: > Meanwhile, I think that the proposed change strictly improves this > message and thus don't see a reason not to take it. Unless we see > somebody come up with a less-awkward solution, that is. Oh, that part is uncontroversial. The wording without double-dashes has been with us forever since the command was updated to use the parse-options API by 69932bc6 (show-ref: migrate to parse-options, 2009-06-20), and so far nobody complained about it, so while "strictly improves" may be true (i.e. the delta may not be negative), the degree of improvement may not be all that much to be worth the combined brain cycles we are spending on this topic. Taking the patch would not make the future work any harder to truly improve it further, so I do not see any reason not to take it. Thanks.
diff --git a/builtin/show-ref.c b/builtin/show-ref.c index 839a5c29f3..85700caae9 100644 --- a/builtin/show-ref.c +++ b/builtin/show-ref.c @@ -293,8 +293,8 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix) struct show_one_options show_one_opts = {0}; int verify = 0, exists = 0; const struct option show_ref_options[] = { - OPT_BOOL(0, "tags", &patterns_opts.tags_only, N_("only show tags (can be combined with branches)")), - OPT_BOOL(0, "branches", &patterns_opts.branches_only, N_("only show branches (can be combined with tags)")), + OPT_BOOL(0, "tags", &patterns_opts.tags_only, N_("only show tags (can be combined with --branches)")), + OPT_BOOL(0, "branches", &patterns_opts.branches_only, N_("only show branches (can be combined with --tags)")), OPT_HIDDEN_BOOL(0, "heads", &patterns_opts.branches_only, N_("deprecated synonym for --branches")), OPT_BOOL(0, "exists", &exists, N_("check for reference existence without resolving")),
Trivial change to indicate that branches and tags are real options that can be used combined to get more information. This helps with linting translations and prompting the user that the terms represent options. Signed-off-by: Alexander Shopov <ash@kambanaria.org> --- builtin/show-ref.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)