* Feature request - git add --patch should have an option to discard or drop a hunk Inbox @ 2021-01-07 16:43 Jon Sagotsky 2021-01-08 9:10 ` Jeff King 0 siblings, 1 reply; 4+ messages in thread From: Jon Sagotsky @ 2021-01-07 16:43 UTC (permalink / raw) To: git I frequently make changes to a number of files. I use `git add -p` to select a number of changes and wrap them up in a commit. Then I run it again to build another commit. Wash, rinse, repeat. Sometimes I make a change I don't intend to keep. Maybe it's a typo, or some debug code, or my linter does something naughty. As described, I have to skip that hunk once per commit I'm building. Usually this is merely inconvenient, but each pass through `git add -p` runs the risk of me mistakenly adding unintended changes to my commit. I could use `e` to edit away that hunk, but for some odd reason that feels too tedious. What I'd really like as a small quality of life enhancement is the ability to discard a hunk so that it's deleted immediately and I don't have to skip it again. `d` is taken, so I suggest `x` as a shortcut. Thanks! Jon ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Feature request - git add --patch should have an option to discard or drop a hunk Inbox 2021-01-07 16:43 Feature request - git add --patch should have an option to discard or drop a hunk Inbox Jon Sagotsky @ 2021-01-08 9:10 ` Jeff King 2021-01-08 15:21 ` Johannes Schindelin 0 siblings, 1 reply; 4+ messages in thread From: Jeff King @ 2021-01-08 9:10 UTC (permalink / raw) To: Jon Sagotsky; +Cc: git On Thu, Jan 07, 2021 at 11:43:45AM -0500, Jon Sagotsky wrote: > I frequently make changes to a number of files. I use `git add -p` to > select a number of changes and wrap them up in a commit. Then I run > it again to build another commit. Wash, rinse, repeat. > > Sometimes I make a change I don't intend to keep. Maybe it's a typo, > or some debug code, or my linter does something naughty. As > described, I have to skip that hunk once per commit I'm building. > Usually this is merely inconvenient, but each pass through `git add > -p` runs the risk of me mistakenly adding unintended changes to my > commit. I do something similar and have run into the same problem. There's an old discussion here going into some ideas: https://lore.kernel.org/git/EE89F0A1-1C07-4597-B654-035F657AD09F@me.com/ The interesting bits are I think: - some tools let you do this already (magit was mentioned there, and I'd expect vim plugins like fugitive can probably do the same; tig can also do so from its "stage" interface) - it probably wouldn't be _too_ hard to implement, because we already drive those actions from the same code that is invoked by "add -p" and "git checkout -p"; the difference is just which program we feed the hunks to. - in the most general form of the tool, it would let you take a pass through the hunks and annotating them. The simple common form is two annotations: stage these ones, discard those ones. There may be room for a more generalized tool, or it may just be over-complicating things. The generalized form probably shouldn't be "add -p". The simplified one (just adding "discard this hunk from the worktree") could perhaps be, though it does feel a little weird for "git add" to modify working tree files. -Peff ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Feature request - git add --patch should have an option to discard or drop a hunk Inbox 2021-01-08 9:10 ` Jeff King @ 2021-01-08 15:21 ` Johannes Schindelin 2021-01-08 16:15 ` Jeff King 0 siblings, 1 reply; 4+ messages in thread From: Johannes Schindelin @ 2021-01-08 15:21 UTC (permalink / raw) To: Jeff King; +Cc: Jon Sagotsky, git Hi Peff, On Fri, 8 Jan 2021, Jeff King wrote: > On Thu, Jan 07, 2021 at 11:43:45AM -0500, Jon Sagotsky wrote: > > > I frequently make changes to a number of files. I use `git add -p` to > > select a number of changes and wrap them up in a commit. Then I run > > it again to build another commit. Wash, rinse, repeat. > > > > Sometimes I make a change I don't intend to keep. Maybe it's a typo, > > or some debug code, or my linter does something naughty. As > > described, I have to skip that hunk once per commit I'm building. > > Usually this is merely inconvenient, but each pass through `git add > > -p` runs the risk of me mistakenly adding unintended changes to my > > commit. > > I do something similar and have run into the same problem. There's an > old discussion here going into some ideas: > > https://lore.kernel.org/git/EE89F0A1-1C07-4597-B654-035F657AD09F@me.com/ > > The interesting bits are I think: > > - some tools let you do this already (magit was mentioned there, and > I'd expect vim plugins like fugitive can probably do the same; tig > can also do so from its "stage" interface) > > - it probably wouldn't be _too_ hard to implement, because we already > drive those actions from the same code that is invoked by "add -p" > and "git checkout -p"; the difference is just which program we feed > the hunks to. > > - in the most general form of the tool, it would let you take a pass > through the hunks and annotating them. The simple common form is two > annotations: stage these ones, discard those ones. There may be room > for a more generalized tool, or it may just be over-complicating > things. > > The generalized form probably shouldn't be "add -p". The simplified > one (just adding "discard this hunk from the worktree") could > perhaps be, though it does feel a little weird for "git add" to > modify working tree files. We do have `git stash -p`, which I used sometimes to discard such unwanted pieces. However, `git stash -p` suffers from a couple implementation details (see e.g. https://lore.kernel.org/git/20200731165140.29197-1-alban.gruin@gmail.com/) More often than not, I am actually using `git add -p` to commit those changes instead, using the commit message `TO-DROP` and later using `rebase -i` to actually drop them. I could imagine that it should not be _too_ hard to implement an action in `git add -p` to allow stashing a given hunk. The biggest challenge there would be to make sure that potentially edited, to-be-stashed changes won't interfere with staging what was intended (because `git add -p` batches the changing per-file, and if the index changes while picking what to stage, things might conflict). Maybe an Outreachy project? Ciao, Dscho ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Feature request - git add --patch should have an option to discard or drop a hunk Inbox 2021-01-08 15:21 ` Johannes Schindelin @ 2021-01-08 16:15 ` Jeff King 0 siblings, 0 replies; 4+ messages in thread From: Jeff King @ 2021-01-08 16:15 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Jon Sagotsky, git On Fri, Jan 08, 2021 at 04:21:05PM +0100, Johannes Schindelin wrote: > > - in the most general form of the tool, it would let you take a pass > > through the hunks and annotating them. The simple common form is two > > annotations: stage these ones, discard those ones. There may be room > > for a more generalized tool, or it may just be over-complicating > > things. > > > > The generalized form probably shouldn't be "add -p". The simplified > > one (just adding "discard this hunk from the worktree") could > > perhaps be, though it does feel a little weird for "git add" to > > modify working tree files. > > We do have `git stash -p`, which I used sometimes to discard such unwanted > pieces. However, `git stash -p` suffers from a couple implementation > details (see e.g. > https://lore.kernel.org/git/20200731165140.29197-1-alban.gruin@gmail.com/) Yeah, and I often will take a first pass with "stash -p" to get rid of unwanted hunks, and _then_ start dividing up the hunks into commits that I want. But that requires multiple passes. I think the point is that you often see the undesirable hunk (that you'd like to stash or discard) while you're making another pass, and would like to deal with it then and there, instead of separately. To be clear, I'm not objecting to a "-p" command modifying the working tree or anything like that. Just that mixing the modification of the working tree into "add -p" feels weird, because of the "add" half, not the "-p" half. :) Conceptually it's a separate action: take a single pass over the hunks and decide what to do with them. But I do agree that in practice that's the operation people are trying to do with "add -p", so it may be a practical place to put it. > More often than not, I am actually using `git add -p` to commit those > changes instead, using the commit message `TO-DROP` and later using > `rebase -i` to actually drop them. That works OK, too, but suffers from the same multi-pass problem. You have to do a separate pass to put them in a TO-DROP commit, and likely you've already marked a bunch of other hunks to be staged for a real commit. > I could imagine that it should not be _too_ hard to implement an action in > `git add -p` to allow stashing a given hunk. The biggest challenge there > would be to make sure that potentially edited, to-be-stashed changes won't > interfere with staging what was intended (because `git add -p` batches the > changing per-file, and if the index changes while picking what to stage, > things might conflict). > > Maybe an Outreachy project? Yeah, it's probably about the right size. The actual mechanics are pretty easy. Here's a really ugly proof-of-concept in the perl script: diff --git a/git-add--interactive.perl b/git-add--interactive.perl index 96426a53c6..528688e553 100755 --- a/git-add--interactive.perl +++ b/git-add--interactive.perl @@ -106,6 +106,7 @@ sub colored { APPLY_CHECK => 'apply --cached', FILTER => 'file-only', IS_REVERSE => 0, + DISCARD => sub { apply_patch 'apply -R', @_ }, }, 'stash' => { DIFF => 'diff-index -p HEAD', @@ -1568,6 +1569,9 @@ sub patch_update_file { print; } } + if (exists $patch_mode_flavour{DISCARD}) { + $other .= ',x'; + } my $type = $num ? $hunk[$ix]{TYPE} : $head->{TYPE}; print colored $prompt_color, "(", ($ix+1), "/", ($num ? $num : 1), ") ", sprintf(__($patch_update_prompt_modes{$patch_mode}{$type}), $other); @@ -1761,6 +1765,18 @@ sub patch_update_file { splice @hunk, $ix, 1, $newhunk; } } + elsif ($line =~ /^x/) { + unless ($other =~ /x/) { + error_msg("don't know how to discard in this mode"); + } + # should probably save up discards and apply at + # the end? + my @patch = reassemble_patch($head->{TEXT}, + @{$hunk[$ix]->{TEXT}}); + $patch_mode_flavour{DISCARD}->(@patch); + # should mark hunk as used so we don't + # redisplay it + } else { help_patch_cmd($other); next; I left this intentionally ugly because I do not even want to consider implementing this for real in the perl version, not when we're so close to getting rid of it (but I'm familiar enough with it and it's fast enough to hack on that I did not want to even try the ugly thing in the C code). It's just meant to show that we basically have all the low-level parts available already. There are some obvious unfinished bits in the comments, but I agree the really tricky parts will be corner cases: what happens to edited hunks or ones which overlap. It may all Just Work because we're applying to two different spots, so overlap isn't a problem. But I haven't thought deeply about it. -Peff ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-01-08 16:17 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-01-07 16:43 Feature request - git add --patch should have an option to discard or drop a hunk Inbox Jon Sagotsky 2021-01-08 9:10 ` Jeff King 2021-01-08 15:21 ` Johannes Schindelin 2021-01-08 16:15 ` Jeff King
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).