{"thread":{"id":"31255","subject":"Feature request - discard hunk in add --patch mode","startedAt":"2012-08-15T08:36:22Z","lastAt":"2012-08-23T03:32:02Z","messageCount":7,"participants":["Mina Almasry","Thomas Rast","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"197034","messageId":"BLU0-SMTP37070ADBEC060E1EA15944693B60@phx.gbl","threadId":"31255","inReplyTo":null,"subject":"Feature request - discard hunk in add --patch mode","fromName":"Mina Almasry","fromEmail":"almasry.mina@hotmail.com","sentAt":"2012-08-15T08:36:22Z","receivedAt":"2012-08-15T08:36:22Z","isPatch":false,"sender":{"key":"almasry.mina@hotmail.com","avatar":null},"body":"Hi,\n\nI frequently stage files using git add --patch command and I almost \nalways come across debug code I want to discard, but there is no option \nfor that in the prompt. The result is that I have to run an extra \ncommand after the dialogue ends.\n\nI would like to add a feature to allow users to discard hunks using a \ncommand like r! or d!\n\nI was wondering if that would be a welcome addition to git. I am willing \nto work on the feature myself.\n\nRegards,\nMina\n"},{"id":"197037","messageId":"87txw41okl.fsf@thomas.inf.ethz.ch","threadId":"31255","inReplyTo":"BLU0-SMTP37070ADBEC060E1EA15944693B60@phx.gbl","subject":"Re: Feature request - discard hunk in add --patch mode","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2012-08-15T09:39:38Z","receivedAt":"2012-08-15T09:39:38Z","isPatch":false,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Mina Almasry writes:\n\n> I frequently stage files using git add --patch command and I almost \n> always come across debug code I want to discard, but there is no option \n> for that in the prompt. The result is that I have to run an extra \n> command after the dialogue ends.\n>\n> I would like to add a feature to allow users to discard hunks using a \n> command like r! or d!\n\nThis has come up before, and actually led to the introduction of\n'checkout -p' and 'reset -p':\n\n http://thread.gmane.org/gmane.comp.version-control.git/123854\n\nJudging by the '!' above, you are already aware that this is a dangerous\noption that needs some safeguards. I imagine that would largely account\nfor Junio's safety concerns. So you could pick up from the general\ndirection of Pierre's post, and try to work out something.\n\nHowever, life has become rather more complicated since 2009. Whatever\nyou do also needs to fit nicely with checkout/reset/stash -p. The first\ntwo also take commit arguments, resulting in a total of seven modes of\noperation, defined in %patch_modes.\n\nI imagine one good angle of attack would be to proceed as follows:\n\n* Optionally make add--interactive.perl more aware of what the patches\n say. Ideally there would be a way to reverse the direction of a diff\n (or otherwise manipulate it) in the program itself, instead of having\n to decide this when fetching the patches.\n\n* Change things so that the actions in %patch_modes are a hunk property\n instead of a global state. Of course the actual mode still needs to\n be global state. Not all actions should be possible in all modes;\n figure out a clean way to implement this.\n\n* Design commands in some modes that switch to another mode, and/or set\n another action than the default in the current mode.\n\nNote that r! commands need to also work in singlekey mode, where the 'r'\nis read immediately and you would have to read for another\n(confirmation) key to get the '!'.\n\nGood luck!\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"197049","messageId":"7vvcgk58le.fsf@alter.siamese.dyndns.org","threadId":"31255","inReplyTo":"87txw41okl.fsf@thomas.inf.ethz.ch","subject":"Re: Feature request - discard hunk in add --patch mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-15T18:11:09Z","receivedAt":"2012-08-15T18:11:09Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast writes:\n\n> Mina Almasry writes:\n>\n>> I frequently stage files using git add --patch command and I almost \n>> always come across debug code I want to discard, but there is no option \n>> for that in the prompt. The result is that I have to run an extra \n>> command after the dialogue ends.\n>>\n>> I would like to add a feature to allow users to discard hunks using a \n>> command like r! or d!\n>\n> This has come up before, and actually led to the introduction of\n> 'checkout -p' and 'reset -p':\n>\n> http://thread.gmane.org/gmane.comp.version-control.git/123854\n\nThat is a blast from the past.\n\nWhy is saying \"git checkout .\" too much work, after \"add -p\" that\nyou excluded the debugging cruft?\n\nI actually do this for extra safety, though:\n\n\tgit add -p ;# add everything but exclude debug cruft\n\tgit diff ;# make really sure that this shows only garbage\n\tgit diff | git apply -R ;# and get rid of that garbage\n\nprimarily because I can take the \"git diff\" output in the second\nstep to a file, remove the hunk that I accidentally forgot to add\nin the \"add -p\" stage, and \"apply -R\" to remove only the cruft.\nAfter doing so, \"git diff\" will show the important-but-forgotten\nbit, and I can choose to add it to the next commit, or I can choose\nto leave it in the working tree for a future commit after the\ncurrent index is committed.\n\nBut the above is a tangent side-note to show possibly a better way\nto work, not about adding new operations to \"add -p\".\n"},{"id":"197054","messageId":"7vmx1w56z1.fsf@alter.siamese.dyndns.org","threadId":"31255","inReplyTo":"7vvcgk58le.fsf@alter.siamese.dyndns.org","subject":"Re: Feature request - discard hunk in add --patch mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-15T18:46:10Z","receivedAt":"2012-08-15T18:46:10Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Thomas Rast writes:\n>\n>> This has come up before, and actually led to the introduction of\n>> 'checkout -p' and 'reset -p':\n>>\n>> http://thread.gmane.org/gmane.comp.version-control.git/123854\n>\n> That is a blast from the past.\n>\n> Why is saying \"git checkout .\" too much work, after \"add -p\" that\n> you excluded the debugging cruft?\n\nPlease forget this question. A better way in the form of \"stash -p\"\nwas suggested in the old thread to get rid of debug cruft in the\ntree before an \"add -p\" session (or during a series of \"add -p\"\nsessions).\n\nSo is this still an issue?\n"},{"id":"197060","messageId":"BLU0-SMTP112EF6B9308C55734C4F10293B60@phx.gbl","threadId":"31255","inReplyTo":"7vmx1w56z1.fsf@alter.siamese.dyndns.org","subject":"Re: Feature request - discard hunk in add --patch mode","fromName":"Mina Almasry","fromEmail":"almasry.mina@hotmail.com","sentAt":"2012-08-15T20:58:08Z","receivedAt":"2012-08-15T20:58:08Z","isPatch":false,"sender":{"key":"almasry.mina@hotmail.com","avatar":null},"body":"On 12-08-15 02:46 PM, Junio C Hamano wrote:\n> Junio C Hamano writes:\n>\n>> Thomas Rast writes:\n>>\n>>> This has come up before, and actually led to the introduction of\n>>> 'checkout -p' and 'reset -p':\n>>>\n>>> http://thread.gmane.org/gmane.comp.version-control.git/123854\n>> That is a blast from the past.\n>>\n>> Why is saying \"git checkout .\" too much work, after \"add -p\" that\n>> you excluded the debugging cruft?\n> Please forget this question. A better way in the form of \"stash -p\"\n> was suggested in the old thread to get rid of debug cruft in the\n> tree before an \"add -p\" session (or during a series of \"add -p\"\n> sessions).\n>\n> So is this still an issue?\n>\n>\nI read most of the thread, and I do think it still is. Here are my 2 cents:\n\n1. The alternative commands aren't nearly as time efficient:\n - git checkout . is fast and awesome, but you can't use it if, for \nexample, you have to maintain a dirty working tree\n - git (stash|reset|checkout) -p make you go through (all|most) of \nthe hunks you have to hunt down those 2 lines that say \"echo \n'This line is runningantoeuhaoeuaoae'\"\n\n2. All of the safety concerns can be alleviated with the right \ninterface. I am glad the u option mentioned in the thread did not go \nthrough since I agree it is not ideal. However, if the command is:\n (a) something with a '!', then no one will hit it by mistake, and\n (b) the prompt makes it clear that work is lost\nthen I think it would be fine\n\nThe advantages of a command like this are pretty huge IMO. I can see \nthis being a big time saver.\n\nHow about adding this to the git add -p prompt:\n\n r! - remove this hunk. The hunk is discarded from the working tree, \nand is irrevocably lost.\n"},{"id":"197068","messageId":"7vfw7n4yde.fsf@alter.siamese.dyndns.org","threadId":"31255","inReplyTo":"BLU0-SMTP112EF6B9308C55734C4F10293B60@phx.gbl","subject":"Re: Feature request - discard hunk in add --patch mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-15T21:51:57Z","receivedAt":"2012-08-15T21:51:57Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mina Almasry writes:\n\n> On 12-08-15 02:46 PM, Junio C Hamano wrote:\n> ...\n>> Please forget this question. A better way in the form of \"stash -p\"\n>> was suggested in the old thread to get rid of debug cruft in the\n>> tree before an \"add -p\" session (or during a series of \"add -p\"\n>> sessions).\n>>\n>> So is this still an issue?\n>>\n> I read most of the thread, and I do think it still is. Here are my 2 cents:\n>\n> 1. The alternative commands aren't nearly as time efficient:\n> - git checkout . is fast and awesome, but you can't use it if, for\n> example, you have to maintain a dirty working tree\n> - git (stash|reset|checkout) -p make you go through (all|most) of\n> the hunks you have to hunt down those 2 lines that say\n> \"echo 'This line is runningantoeuhaoeuaoae'\"\n\nYou have to do that _at least once_ anyway, as there is no other way\nfor Git to tell which one is debugging cruft and which one is the\nreal change you value without you telling it. Will return to the\ntopic later.\n\n> 2. All of the safety concerns can be alleviated with the right\n> interface.\n\nThere are three safety concerns I raised in the original thread.\nAmong them, I do not think\n\n (1) new user may mistake \"undo\" to be something safe; and\n (2) experts will continue making typo between \"y\" and \"u\"\n\nare the primary risks of the original patch Thomas pointed out. \n\nA much bigger problem with the approach is (3) letting \"add\" touch\nthe working tree breaks the world model. Both experts and newbies\nalike, people have learned that \"git add\" will never clobber what\nthey have in the working tree and rely on that promise.\n\nAnd your key assignment, command renaming or extra prompting do not\nchange this fundamental issue at all.\n\nLet's step back a bit, and define the problem we are solving.\n\nSuppose you have changes in your working tree that are worth\nmultiple commits, debugging aid, and uncommittable WIP. You want to\ncreate multiple commits, possibly giving each of them the final\ntesting before committing, and want to end up with the WIP (plus\npossibly the debugging aid, as that may still help your WIP) in your\nworking tree.\n\nDo we agree that the goal of the discussion of this thread is to\nmake that process simple, safe, efficient and easy?\n\nNow, back when the original patch by Pierre was proposed, it indeed\nwas cumbersome. You could sift things through by \"add -p\" to build\nthe first commit in the index, commit, and iterate. In each round,\n\"add -p\" step had to skip the same debugging aid and WIP over and\nover again. If you wanted to give the result of \"git add -p\" a\nfinal test before committing, \"stash save -k\" would give you the\nstate you would be committing, but it isn't easy to reintroduce only\nthe debugging aid to the working tree.\n\nSince then \"stash -p\" was added to our toolchest. So theoretically,\nwe should be able to do something like this:\n\n # start from N-commit worth of change, debug and WIP\n git stash save -p debug ;# stash away only the debugging aid\n # now we have only N-commit worth of change and WIP\n git stash save -p wip ;# stash away WIP\n\nThen after that, you need N round of \"git add -p && git commit\".\n\nNow, with what we have already, can we also give final testing\nbefore committing? Each round may now start with:\n\n git add -p ;# prepare the index for the next commit\n git stash save -k ;# save away the changes for later commits\n\nand at this point, your working tree and the index has what you are\nabout to commit. If we can apply the \"debugging aid\" stash we\ncreated earlier to the working tree, that would allow you to test\nthe state you are about to commit with your debugging aid. The\ncommand to do so may be\n\n git stash apply stash@{2}\n\nOnce you are satisfied, you can reset this change out of your\nworking tree with \"checkout .\", and then \"git commit\" to record what\nis in the index.\n\nAnd then you can \"git stash pop\" to bring back the remainder of\nN-commit series worth of changes. You are ready to continue to the\nnext round.\n\nOnce you created N-commit series, you will still have two stash\nentries, one \"debug\" and one \"wip\". You should be able to resurrect\n\"wip\" with\n\n git stash pop\n\njust fine, but there is a little problem after this step. Because\n\"git stash [apply|pop]\" does not want to work on a dirty working\ntree, starting from this state just after popping \"wip\" stash, you\ncannot \"git stash pop\" to have both WIP changes and debugging aid to\nthe working tree.\n\nA topic to improve \"stash [apply/pop]\" to allow it may be a valid\nand useful thing to do.\n\nAs an approximation, without changing any of the current tools,\nhowever, you should be able to do this after creating N-commit\nseries following the above procedure.\n\n\tgit stash pop ;# resurrect \"wip\" to the working tree.\n git add -u ;# and add that to the index temporarily\n git stash pop ;# resurrect \"debug\" to the working tree as well.\n\tgit reset ;# then match the index to HEAD\n\nThat will mix the WIP and debug again in your working tree.\n\nWhy you would want to mix them together, after sifting debug and wip\nusing \"add -p\" already, is a different issue, though (there are\nvalid scenarios why you would want to do so, and there are valid\ncases why you do not have to bother).\n\nSo is this still an issue?\n"},{"id":"197674","messageId":"1345692722.6059.40.camel@mina-asus","threadId":"31255","inReplyTo":"BLU0-SMTP112EF6B9308C55734C4F10293B60@phx.gbl","subject":"Re: Feature request - discard hunk in add --patch mode","fromName":"Mina Almasry","fromEmail":"almasry.mina@gmail.com","sentAt":"2012-08-23T03:32:02Z","receivedAt":"2012-08-23T03:32:02Z","isPatch":false,"sender":{"key":"almasry.mina@gmail.com","avatar":null},"body":"Well I actually didn't know about stash -p when I asked for this, but I\nstill see room for us removing some more friction here:\n\n> # start from N-commit worth of change, debug and WIP\n> git stash save -p debug ;# stash away only the debugging aid\n> # now we have only N-commit worth of change and WIP\n> git stash save -p wip ;# stash away WIP\n> \n> Then after that, you need N round of \"git add -p && git commit\".\n> \n> Now, with what we have already, can we also give final testing\n> before committing? Each round may now start with:\n> \n> git add -p ;# prepare the index for the next commit\n> git stash save -k ;# save away the changes for later commits\n> \nHere you have to _at least_ go through all your hunks once to stash the\ndebug code, once to stash the WIP, and once to add -p and commit.\n\nGoing through hunks is expensive... it takes quite a bit of time. Each\ntime you launch one of those commands you have to go through all hunks\nyou haven't decided on yet - could be a lot of hunks.\n\nNow, what if we had a command where you would go through all your hunks\none, by one, and will give you an option to stage, stash, or discard\nwhat you want as you please? In the above scenario, lets say N was 1,\nthen the scenario would be:\n\n # start from 1-commit worth of change, debug and WIP\n \n # stash debugging and WIP code, \n # and stage the code you want to commit\n git mystery-command\n \n # everything done\n git commit\n\nThen we would reduce 3 -p commands to 1 -p command.\n\nNow what should this command be called? You resist making git-add unsafe\nand I understand that. How about: \n\n 1. Give git add -p options that expose this extra functionality. So:\n\ngit add -p ; # can only stage code\ngit add -p --with-discard ; # does git add -p, plus discards hunks\ngit add -p --with-discard --with-stage ; # plus discard, plus stage\n\nSo in the hunks you can go:\n - discard; # and it would discard it\n - stage ; # and it would stash it into \n\nThe argument I am making is that if a programmer explicitly asks for one\nof those features then git will cooperate, and add becomes unsafe. We\nare trusting that the programmer knows what they are doing. In all other\ncases git add is the same, and will never touch the working tree.\n\nWe can also change git checkout -p and git stash -p so that they behave\nthe same:\n\n git stash -p --with-add; # does everything -p stash, and stages too\n git checkout -p --with-add; # does everything -p checkout, and stages\n\n 2. We can come up with a whole new git command that does this.\nSomething like git patch-process\n\n # start from 1-commit worth of change, debug and WIP\n\n git patch-process ; # Stage, stash, and discard hunks as you please\n\n # ready for commit\n git commit\n\nHow does this sound?\n\nPS: Newbish question: I am looking to help out with git, either with\nthis feature or others. How do I get around getting assigned a work to\ndo? Thanks.\n\nMina\n\nOn Wed, 2012-08-15 at 16:58 -0400, Mina Almasry wrote:\n> On 12-08-15 02:46 PM, Junio C Hamano wrote:\n> > Junio C Hamano writes:\n> >\n> >> Thomas Rast writes:\n> >>\n> >>> This has come up before, and actually led to the introduction of\n> >>> 'checkout -p' and 'reset -p':\n> >>>\n> >>> http://thread.gmane.org/gmane.comp.version-control.git/123854\n> >> That is a blast from the past.\n> >>\n> >> Why is saying \"git checkout .\" too much work, after \"add -p\" that\n> >> you excluded the debugging cruft?\n> > Please forget this question. A better way in the form of \"stash -p\"\n> > was suggested in the old thread to get rid of debug cruft in the\n> > tree before an \"add -p\" session (or during a series of \"add -p\"\n> > sessions).\n> >\n> > So is this still an issue?\n> >\n> >\n> I read most of the thread, and I do think it still is. Here are my 2 cents:\n> \n> 1. The alternative commands aren't nearly as time efficient:\n> - git checkout . is fast and awesome, but you can't use it if, for \n> example, you have to maintain a dirty working tree\n> - git (stash|reset|checkout) -p make you go through (all|most) of \n> the hunks you have to hunt down those 2 lines that say \"echo \n> 'This line is runningantoeuhaoeuaoae'\"\n> \n> 2. All of the safety concerns can be alleviated with the right \n> interface. I am glad the u option mentioned in the thread did not go \n> through since I agree it is not ideal. However, if the command is:\n> (a) something with a '!', then no one will hit it by mistake, and\n> (b) the prompt makes it clear that work is lost\n> then I think it would be fine\n> \n> The advantages of a command like this are pretty huge IMO. I can see \n> this being a big time saver.\n> \n> How about adding this to the git add -p prompt:\n> \n> r! - remove this hunk. The hunk is discarded from the working tree, \n> and is irrevocably lost.\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n> \n"}]}