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

[Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>]

Hello Jonathan Nieder,

Thanks for the reply!

Jonathan Nieder 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.
> 
I've fixed that. The new commit message is found at the end of this
message.

> 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.
> 
Sorry, I actually didn't that. Could you rephrase it a little.

>  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
> 
I used "filename" as the function indeed check if the argument given to
it is a filename. How about,

    fatal: expecting filename; found '-n'

???

In the context it looks as follows,

    $ git grep "some random regex" -n
    fatal: expected filename; found '-n' 

    $ git rev-parse --flags
    README.md
    --flags
    fatal: expected filename,
    found '--flags'


> 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.
> 
That's surprising, I thought the phrase "in place of filename" was more
explanatory as it doesn't make assumptions about the previous arguments
and specifies what was expected.

> Thanks and hope that helps,
Yep


The modified commit message is found below. If it still seems to be
missing the *why*, let me know.

    setup: update error message to be more meaningful
    
    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.
    
            $ git ls-files
            README.md
            test-file
    
    Correct case,
    
            $ git rev-parse README.md --flags
            README.md
            --flags
            fatal: bad flag '--flags' used after filename
    
    Incorrect case,
    
            $ git grep "some random regex" -n
            fatal: bad flag '-n' used after filename
    
    The above case is incorrect as "some random regex" isn't a filename
    in this case.
    
    Change the error message to be general and communicative. This results
    in the following output,
    
            $ git rev-parse README.md --flags
            README.md
            --flags
            fatal: found flag '--flags' in place of a filename
    
            $ git grep "some random regex" -n
            fatal: found flag '-n' in place of a filename

    -- 
    Kaartic