Re: [PATCH/RFC] setup: update error message to be more meaningful

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

 



Hi,

Kaartic Sivaraam wrote:

> The error message shown when a flag is found when expecting a
> filename wasn't clear as it didn't communicate what was wrong
> using the 'suitable' words in *all* cases.
>
> Correct case,
>
>         $ git rev-parse commit.c --flags
>         commit.c
>         --flags
>         fatal: bad flag '--flags' used after filename
>
> Incorrect case,
>
>         $ git grep "test" -n
>         fatal: bad flag '-n' used after filename
>
> Change the error message to be general and communicative.

Thanks for writing this.  These examples describe *what* the behavior
is but don't describe *why* it is wrong or what is expected in its
place.

For an initial guess: in the example

	git grep test -n

it is confusing that it says "bad flag used after filename" because
test isn't even a filename!  At first glance, I would imagine that any
of the following behaviors would be nicer:

 1. Treat the command as "git grep -e test -n", or in other words
    "do what I mean" since it is clear enough, at least to humans.

 2. Focus on "argument" instead of "filename" so that the message
    could still apply: something like

	fatal: option '-n' must come before non-option arguments

[...]
> --- a/setup.c
> +++ b/setup.c
> @@ -230,7 +230,7 @@ void verify_filename(const char *prefix,
>  		     int diagnose_misspelt_rev)
>  {
>  	if (*arg == '-')
> -		die("bad flag '%s' used after filename", arg);
> +		die("found flag '%s' in place of a filename", arg);

Probably because of the background I am missing described above, it's
not clear to me that the new message is any better (or worse) than the
existing one.  The old message with "after filename" has the virtue of
explaining why an option is not expected there.

Thanks and hope that helps,
Jonathan



[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