Re: [GSoC][PATCH v15 7/9] builtin/refs: add verify subcommand

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Aug 06, 2024 at 09:15:28AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@xxxxxx> writes:
> 
> >> +	if (argc)
> >> +		usage(_("'git refs verify' takes no arguments"));
> >
> > Junio has posted a patch series [1] where he wants to get rid of
> > messages that simply say "no arguments" or "too many arguments".
> > ...
> > So I'd propose to make this:
> >
> >     argc = parse_options(argc, argv, prefix, options, verify_usage, 0);
> >     if (argc)
> >             usage(_("unknown argument: '%s'", argv[0]));
> 
> I probably should have said that I am fully behind the intent
> against "too many arguments", but I am not 100% behind the
> particular messaging used in the patch series I sent out.
> 
> One potential complaint I expected to hear, for example, was that "a
> is unknown" given when you said "git cmd a a a a a" is not all that
> clear ;-).  To alleviate, you would have to say "git cmd takes only
> 2 arguments" if 'a' you are complaining about is the third one.
> 
> Also, many people would consider that "unexpected argument" is
> better than "unknown argument".
> 
> I personally think the message above is absolutely clear and good.
> 
> You say that 'git refs verify' takes no arguments, and for somebody
> who said "git refs verify a b c d e", there is no doubt that all of
> these a b c d e are unwanted.  And there is no room to misinterpret
> the message as "'git refs' is ok but 'git refs verify' is already
> unwelcome with extra argument", either [*].
> 
> In short, I think the message in the patch here is good, and it is
> the other "war on 'too many arguments'" series whose messages need
> to be thought further.

Just to clarify: with "the patch" you probably refer to the current
version that Jialuo has, right? In other words, keep the current version
that he has and adapt the message in the future, when we have decided
what to do about those "too many arguments" messages?

If so, then the only two I had were some spurious newlines. I'm not sure
whether these really would be worth rerolling the whole patch series.

Patrick

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux