Fwd: Bug report: reset -p HEAD

From: Maarten de Vries <maarten@de-vri.es>
Date: Oct 24, 2013, 23:05 UTC
Message-ID: <CAPWpf+x5KUjD2K81pJAsV_XuPkd=BT06qPjvr8s7tEe5YqBt=w@mail.gmail.com>
In-Reply-To: <CAPWpf+xqutvhq1jyVkxr6LyKsANTCS6M=vj5XY=EgUfiS3Z8xg@mail.gmail.com>

Hi,

I noticed that reset -p HEAD is inconsistent with checkout -p HEAD. When running checkout -p you are asked to discard hunks from the index and worktree, but when running reset -p you are asked to apply hunks to the index. It would make more sense if reset -p asked to discard (reversed) hunks from the index.

Digging a bit further, it looks like reset -p is actually intended to show hunks to discard when resetting to HEAD. The git-add--interactive.perl script has different cases for resetting to the head and for resetting to anything else. However, builtin/reset.c always passes a hash to run_add_interactive, even if HEAD is provided explicitly on the command line or no revision is given. As a result, the special case for resetting to the HEAD is never triggered and git-add--interactive.perl always asks to apply hunks rather than discard the reverse hunks.

The offending part in builtin/reset.c is on line 307. It's the bit with sha1_to_hex(sha1):

Show 5 quoted lines

>     if (patch_mode) {
>         if (reset_type != NONE)
>             die(_("--patch is incompatible with --{hard,mixed,soft}"));
>         return run_add_interactive(sha1_to_hex(sha1), "--patch=reset", &pathspec);
>     }

I'm not familiar enough with the git source, but it's probably a fairly trivial fix for someone who is.

Kind regards, Maarten de Vries

P.S.

This bit in git-add--interactive.perl convinced me that resetting to HEAD interactively should be handled separately:

Show 20 quoted lines

>     'reset_head' => {
>         DIFF => 'diff-index -p --cached',
>         APPLY => sub { apply_patch 'apply -R --cached', @_; },
>         APPLY_CHECK => 'apply -R --cached',
>         VERB => 'Unstage',
>         TARGET => '',
>         PARTICIPLE => 'unstaging',
>         FILTER => 'index-only',
>         IS_REVERSE => 1,
>     },
>     'reset_nothead' => {
>         DIFF => 'diff-index -R -p --cached',
>         APPLY => sub { apply_patch 'apply --cached', @_; },
>         APPLY_CHECK => 'apply --cached',
>         VERB => 'Apply',
>         TARGET => ' to index',
>         PARTICIPLE => 'applying',
>         FILTER => 'index-only',
>         IS_REVERSE => 0,
>     },