[PATCH] commit: improve error message in "-a <paths>" case

[PATCH] commit: improve error message in "-a <paths>" case

[Nguyễn Thái Ngọc Duy]

I did something stupid today and got

    $ git commit -a --fixup= @^
    fatal: Paths with -a does not make sense.

which didn't make any sense (at least for the first few seconds).

Include the first path(spec) in the error message to help spot the
problem quicker. Now it shows

    fatal: paths '@^ ...' with -a does not make sense

which should ring some bell because @^ should clearly not be considered
a path.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/commit.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index f17537474a..7f9aa032b6 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1194,7 +1194,8 @@ static int parse_and_validate_options(int argc, const char *argv[],
 	handle_untracked_files_arg(s);
 
 	if (all && argc > 0)
-		die(_("Paths with -a does not make sense."));
+		die(_("paths '%s ...' with -a does not make sense"),
+		    argv[0]);
 
 	if (status_format != STATUS_FORMAT_NONE)
 		dry_run = 1;
-- 
2.21.0.548.gd3c7d92dc2

Re: [PATCH] commit: improve error message in "-a <paths>" case

[Junio C Hamano]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> I did something stupid today and got
>
>     $ git commit -a --fixup= @^
>     fatal: Paths with -a does not make sense.
>
> which didn't make any sense (at least for the first few seconds).
>
> Include the first path(spec) in the error message to help spot the
> problem quicker. Now it shows
>
>     fatal: paths '@^ ...' with -a does not make sense
>
> which should ring some bell because @^ should clearly not be considered
> a path.

Makes sort of sense.  Would it break to blindly use argv[0] if the
user had an explicit double-dash "--" disambiguator?

>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  builtin/commit.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index f17537474a..7f9aa032b6 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1194,7 +1194,8 @@ static int parse_and_validate_options(int argc, const char *argv[],
>  	handle_untracked_files_arg(s);
>  
>  	if (all && argc > 0)
> -		die(_("Paths with -a does not make sense."));
> +		die(_("paths '%s ...' with -a does not make sense"),
> +		    argv[0]);
>  
>  	if (status_format != STATUS_FORMAT_NONE)
>  		dry_run = 1;

Re: [PATCH] commit: improve error message in "-a <paths>" case

[Duy Nguyen]

On Thu, Mar 21, 2019 at 12:49 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
> > I did something stupid today and got
> >
> >     $ git commit -a --fixup= @^
> >     fatal: Paths with -a does not make sense.
> >
> > which didn't make any sense (at least for the first few seconds).
> >
> > Include the first path(spec) in the error message to help spot the
> > problem quicker. Now it shows
> >
> >     fatal: paths '@^ ...' with -a does not make sense
> >
> > which should ring some bell because @^ should clearly not be considered
> > a path.
>
> Makes sort of sense.  Would it break to blindly use argv[0] if the
> user had an explicit double-dash "--" disambiguator?

Good point. I didn't think of this.

$ ./git commit -a --fixup= @^ -- foo
fatal: paths '@^ ...' with -a does not make sense

so on the bright side the error message still reports the right thing.

But parseopt I think does something wrong because @^ should not be
considered a path at all. There should be some other error message
about unrecognized option "@^". I checked argv, argv[0] is "@^",
argv[1] is "foo" and "--" stripped.

This I think is a separate bug. Will look into it when I have more
time (and tagging #leftovers in case people look for something fun and
small to do)
-- 
Duy