No "invalid option" message with git diff --cached --invalid-option

No "invalid option" message with git diff --cached --invalid-option

[Simon Ruderich]

Hello,

I'm using Git 2.13.1 (from the Debian sid repository) and noticed
the following issue when upgrading.

    $ git diff --compaction-heuristic
    error: invalid option: --compaction-heuristic

    $ git diff --cached --compaction-heuristic
    usage: git diff [<options>] [<commit> [<commit>]] [--] [<path>...]

I know that --compaction-heuristic is no longer supported but I
was using it an alias and was confused that I got no proper error
message warning me which option was wrong.

It seems to happen for any invalid option which is used in
combination with --cached or --staged.

Regards
Simon
-- 
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9

Re: No "invalid option" message with git diff --cached --invalid-option

[Jeff King]

On Thu, Jun 22, 2017 at 02:44:58PM +0200, Simon Ruderich wrote:

> I'm using Git 2.13.1 (from the Debian sid repository) and noticed
> the following issue when upgrading.
> 
>     $ git diff --compaction-heuristic
>     error: invalid option: --compaction-heuristic
> 
>     $ git diff --cached --compaction-heuristic
>     usage: git diff [<options>] [<commit> [<commit>]] [--] [<path>...]
> 
> I know that --compaction-heuristic is no longer supported but I
> was using it an alias and was confused that I got no proper error
> message warning me which option was wrong.
> 
> It seems to happen for any invalid option which is used in
> combination with --cached or --staged.

The diff command splits off into several sub-functions based on whether
we're doing an index-to-file diff, an index-to-tree diff, etc. It looks
like builtin_diff_files() and builtin_diff_index() just handle this case
differently. Probably both should issue an error() mentioning the
unknown option _and_ call usage().

I started on a patch, but maybe this would be good micro-project for
somebody wanting to get their feet wet with contributing to Git. Bonus
points for checking the other functions (builtin_diff_blobs,
builtin_diff_tree, builtin_diff_b_f) and making sure they all give
appropriate messages when showing the usage string.

-Peff