From: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
To: Jonathan Nieder <jrnieder@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH/RFC] setup: update error message to be more meaningful
Date: Wed, 26 Jul 2017 18:38:57 +0530 [thread overview]
Message-ID: <1501074537.1792.2.camel@gmail.com> (raw)
In-Reply-To: <20170725212110.GG13924@aiede.mtv.corp.google.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
next prev parent reply other threads:[~2017-07-26 13:08 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-25 17:57 [PATCH/RFC] setup: update error message to be more meaningful Kaartic Sivaraam
2017-07-25 18:16 ` Kaartic Sivaraam
2017-07-25 21:21 ` Jonathan Nieder
2017-07-26 13:08 ` Kaartic Sivaraam [this message]
2017-07-26 20:09 ` Junio C Hamano
2017-07-29 2:44 ` Kaartic Sivaraam
2017-07-29 3:53 ` Junio C Hamano
2017-07-29 12:13 ` Kaartic Sivaraam
2017-07-29 12:41 ` [PATCH] " Kaartic Sivaraam
2017-07-29 16:10 ` [PATCH/RFC] " Junio C Hamano
2017-07-30 10:47 ` Kaartic Sivaraam
2017-07-30 11:03 ` Kaartic Sivaraam
2017-10-02 17:30 ` [PATCH v2] " Kaartic Sivaraam
2017-10-03 0:32 ` Junio C Hamano
2017-10-04 14:18 ` Kaartic Sivaraam
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1501074537.1792.2.camel@gmail.com \
--to=kaarticsivaraam91196@gmail.com \
--cc=git@vger.kernel.org \
--cc=jrnieder@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).