Skip to content
  • Johannes Schindelin's avatar
    stash -p: (partially) fix bug concerning split hunks · 77234361
    Johannes Schindelin authored and Junio C Hamano's avatar Junio C Hamano committed
    
    
    When trying to stash part of the worktree changes by splitting a hunk
    and then only partially accepting the split bits and pieces, the user
    is presented with a rather cryptic error:
    
    	error: patch failed: <file>:<line>
    	error: test: patch does not apply
    	Cannot remove worktree changes
    
    and the command would fail to stash the desired parts of the worktree
    changes (even if the `stash` ref was actually updated correctly).
    
    We even have a test case demonstrating that failure, carrying it for
    four years already.
    
    The explanation: when splitting a hunk, the changed lines are no longer
    separated by more than 3 lines (which is the amount of context lines
    Git's diffs use by default), but less than that. So when staging only
    part of the diff hunk for stashing, the resulting diff that we want to
    apply to the worktree in reverse will contain those changes to be
    dropped surrounded by three context lines, but since the diff is
    relative to HEAD rather than to the worktree, these context lines will
    not match.
    
    Example time. Let's assume that the file README contains these lines:
    
    	We
    	the
    	people
    
    and the worktree added some lines so that it contains these lines
    instead:
    
    	We
    	are
    	the
    	kind
    	people
    
    and the user tries to stash the line containing "are", then the command
    will internally stage this line to a temporary index file and try to
    revert the diff between HEAD and that index file. The diff hunk that
    `git stash` tries to revert will look somewhat like this:
    
    	@@ -1776,3 +1776,4
    	 We
    	+are
    	 the
    	 people
    
    It is obvious, now, that the trailing context lines overlap with the
    part of the original diff hunk that the user did *not* want to stash.
    
    Keeping in mind that context lines in diffs serve the primary purpose of
    finding the exact location when the diff does not apply precisely (but
    when the exact line number in the file to be patched differs from the
    line number indicated in the diff), we work around this by reducing the
    amount of context lines: the diff was just generated.
    
    Note: this is not a *full* fix for the issue. Just as demonstrated in
    t3701's 'add -p works with pathological context lines' test case, there
    are ambiguities in the diff format. It is very rare in practice, of
    course, to encounter such repeated lines.
    
    The full solution for such cases would be to replace the approach of
    generating a diff from the stash and then applying it in reverse by
    emulating `git revert` (i.e. doing a 3-way merge). However, in `git
    stash -p` it would not apply to `HEAD` but instead to the worktree,
    which makes this non-trivial to implement as long as we also maintain a
    scripted version of `add -i`.
    
    Signed-off-by: default avatarJohannes Schindelin <johannes.schindelin@gmx.de>
    Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
    77234361