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

Simon Ruderich· Jun 22, 2017, 12:44 UTC · lore

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

Jeff King· Jun 22, 2017, 14:13 UTC · re: Simon Ruderich · lore

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

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

Show 15 quoted lines

> 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