Re: Bug report: reset -p HEAD
- From
- Maarten de Vries <maarten@de-vri.es>
- Date
- Oct 24, 2013, 23:16 UTC
- Message-ID
- <CAPWpf+zkA68pzR3qx5xma8BNiONwt5o8iNba0x8fmmiDAWpHuw@mail.gmail.com>
- In-Reply-To
- <CAPWpf+x5KUjD2K81pJAsV_XuPkd=BT06qPjvr8s7tEe5YqBt=w@mail.gmail.com>
Some more info: It used to work as intended. Using a bisect shows it has been broken by commit 166ec2e9.
Kinds regards, Maarten de Vries
On 25 October 2013 01:05, Maarten de Vries <maarten@de-vri.es> wrote:
Show 58 quoted lines
> 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):
>> 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:
>> '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,
>> },