{"thread":{"id":"27358","subject":"[PATCH] add-interactive: shortcut for add hunk and quit","startedAt":"2011-05-15T12:55:25Z","lastAt":"2011-05-19T19:42:52Z","messageCount":15,"participants":["Hermann Gausterer","Junio C Hamano","Matthieu Moy","Pete Harlan","Jeff King","Michael J Gruber","Thomas Rast"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"167891","messageId":"20110515125525.GA3014@mrq1.org","threadId":"27358","inReplyTo":null,"subject":"[PATCH] add-interactive: shortcut for add hunk and quit","fromName":"Hermann Gausterer","fromEmail":"git-mailinglist@mrq1.org","sentAt":"2011-05-15T12:55:25Z","receivedAt":"2011-05-15T12:55:25Z","isPatch":true,"sender":{"key":"git-mailinglist@mrq1.org","avatar":null},"body":"combines the two commands \"y\"+\"q\" to one.\ni use this if i know that this is the last hunk to add.\n\nSigned-off-by: Hermann Gausterer \n---\n Documentation/git-add.txt | 1 +\n git-add--interactive.perl | 15 +++++++++++++--\n 2 files changed, 14 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex 9c1d395..76ffd45 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -280,6 +280,7 @@ patch::\n y - stage this hunk\n n - do not stage this hunk\n q - quit; do not stage this hunk nor any of the remaining ones\n+ Q - quit; stage this hunk but none of the remaining ones\n a - stage this hunk and all later hunks in the file\n d - do not stage this hunk nor any of the later hunks in the file\n g - select a hunk to go to\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 4f08fe7..db79556 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -1123,6 +1123,7 @@ sub help_patch_cmd {\n y - $verb this hunk$target\n n - do not $verb this hunk$target\n q - quit; do not $verb this hunk nor any of the remaining ones\n+Q - quit; $verb this hunk but none of the remaining ones\n a - $verb this hunk and all later hunks in the file\n d - do not $verb this hunk nor any of the later hunks in the file\n g - select a hunk to go to\n@@ -1313,7 +1314,7 @@ sub patch_update_file {\n \t\t $hunk[$ix]{TYPE} eq 'deletion' ? ' deletion' :\n \t\t ' this hunk'),\n \t\t $patch_mode_flavour{TARGET},\n-\t\t \" [y,n,q,a,d,/$other,?]? \";\n+\t\t \" [y,n,q,Q,a,d,/$other,?]? \";\n \t\tmy $line = prompt_single_character;\n \t\tif ($line) {\n \t\t\tif ($line =~ /^y/i) {\n@@ -1365,7 +1366,17 @@ sub patch_update_file {\n \t\t\t\t}\n \t\t\t\tnext;\n \t\t\t}\n-\t\t\telsif ($line =~ /^q/i) {\n+\t\t\telsif ($line =~ /^q/) {\n+\t\t\t\tfor ($i = 0; $i < $num; $i++) {\n+\t\t\t\t\tif (!defined $hunk[$i]{USE}) {\n+\t\t\t\t\t\t$hunk[$i]{USE} = 0;\n+\t\t\t\t\t}\n+\t\t\t\t}\n+\t\t\t\t$quit = 1;\n+\t\t\t\tlast;\n+\t\t\t}\n+\t\t\telsif ($line =~ /^Q/) {\n+\t\t\t\t$hunk[$ix]{USE} = 1;\n \t\t\t\tfor ($i = 0; $i < $num; $i++) {\n \t\t\t\t\tif (!defined $hunk[$i]{USE}) {\n \t\t\t\t\t\t$hunk[$i]{USE} = 0;\n-- \n1.7.0.4\n\n"},{"id":"167921","messageId":"7v7h9rpuzb.fsf@alter.siamese.dyndns.org","threadId":"27358","inReplyTo":"20110515125525.GA3014@mrq1.org","subject":"Re: [PATCH] add-interactive: shortcut for add hunk and quit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-15T20:30:32Z","receivedAt":"2011-05-15T20:30:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Hermann Gausterer writes:\n\n> combines the two commands \"y\"+\"q\" to one.\n> i use this if i know that this is the last hunk to add.\n>\n> Signed-off-by: Hermann Gausterer \n> ---\n> Documentation/git-add.txt | 1 +\n> git-add--interactive.perl | 15 +++++++++++++--\n> 2 files changed, 14 insertions(+), 2 deletions(-)\n\nIt feels a bit _too_ narrow a usecase to me.\n\nI am personally not very interested in the feature itself, but even if I\nwere, I wouldn't be happy to see an implementation that duplicates a\ntrivial existing loop without refactoring to add maintenance burden.\n\nThanks.\n"},{"id":"168003","messageId":"20110516162611.GA6960@mrq1.org","threadId":"27358","inReplyTo":"7v7h9rpuzb.fsf@alter.siamese.dyndns.org","subject":"[PATCH] add-interactive: shortcut to add hunk and quit","fromName":"Hermann Gausterer","fromEmail":"git-mailinglist@mrq1.org","sentAt":"2011-05-16T16:26:11Z","receivedAt":"2011-05-16T16:26:11Z","isPatch":true,"sender":{"key":"git-mailinglist@mrq1.org","avatar":null},"body":"this combines the two commands \"y\"+\"q\" to one.\ni use this if i know that this is the last hunk to add.\n\nSigned-off-by: Hermann Gausterer \n---\n Documentation/git-add.txt | 1 +\n git-add--interactive.perl | 6 +++++-\n 2 files changed, 6 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex 9c1d395..76ffd45 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -280,6 +280,7 @@ patch::\n y - stage this hunk\n n - do not stage this hunk\n q - quit; do not stage this hunk nor any of the remaining ones\n+ Q - quit; stage this hunk but none of the remaining ones\n a - stage this hunk and all later hunks in the file\n d - do not stage this hunk nor any of the later hunks in the file\n g - select a hunk to go to\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 4f08fe7..67d0b48 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -1123,6 +1123,7 @@ sub help_patch_cmd {\n y - $verb this hunk$target\n n - do not $verb this hunk$target\n q - quit; do not $verb this hunk nor any of the remaining ones\n+Q - quit; $verb this hunk but none of the remaining ones\n a - $verb this hunk and all later hunks in the file\n d - do not $verb this hunk nor any of the later hunks in the file\n g - select a hunk to go to\n@@ -1313,7 +1314,7 @@ sub patch_update_file {\n \t\t $hunk[$ix]{TYPE} eq 'deletion' ? ' deletion' :\n \t\t ' this hunk'),\n \t\t $patch_mode_flavour{TARGET},\n-\t\t \" [y,n,q,a,d,/$other,?]? \";\n+\t\t \" [y,n,q,Q,a,d,/$other,?]? \";\n \t\tmy $line = prompt_single_character;\n \t\tif ($line) {\n \t\t\tif ($line =~ /^y/i) {\n@@ -1366,6 +1367,9 @@ sub patch_update_file {\n \t\t\t\tnext;\n \t\t\t}\n \t\t\telsif ($line =~ /^q/i) {\n+\t\t\t\tif ($line =~ /^Q/) {\n+\t\t\t\t\t$hunk[$ix]{USE} = 1;\n+\t\t\t\t}\n \t\t\t\tfor ($i = 0; $i < $num; $i++) {\n \t\t\t\t\tif (!defined $hunk[$i]{USE}) {\n \t\t\t\t\t\t$hunk[$i]{USE} = 0;\n-- \n1.7.0.4\n\n"},{"id":"168005","messageId":"vpq4o4uwqin.fsf@bauges.imag.fr","threadId":"27358","inReplyTo":"20110516162611.GA6960@mrq1.org","subject":"Re: [PATCH] add-interactive: shortcut to add hunk and quit","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2011-05-16T16:37:20Z","receivedAt":"2011-05-16T16:37:20Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Hermann Gausterer writes:\n\n> this combines the two commands \"y\"+\"q\" to one.\n> i use this if i know that this is the last hunk to add.\n\n(please capitalize the \"I\", and actually, avoid saying \"I\" in a commit\nmessage)\n\nI'm not convinced this is useful enough to deserve a new command. The\nhelp message already starts being scary ...\n\n> + Q - quit; stage this hunk but none of the remaining ones\n\nThe explanation shouldn't start with \"quit\" I think. I'd say basically\n\"stage this hunk and quit\" or \"stage this hunk but none of the remaining\nones\".\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"168041","messageId":"7vboz1j4k9.fsf@alter.siamese.dyndns.org","threadId":"27358","inReplyTo":"vpq4o4uwqin.fsf@bauges.imag.fr","subject":"Re: [PATCH] add-interactive: shortcut to add hunk and quit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-17T05:09:58Z","receivedAt":"2011-05-17T05:09:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> Hermann Gausterer writes:\n>\n>> this combines the two commands \"y\"+\"q\" to one.\n>> i use this if i know that this is the last hunk to add.\n>\n> (please capitalize the \"I\", and actually, avoid saying \"I\" in a commit\n> message)\n>\n> I'm not convinced this is useful enough to deserve a new command. The\n> help message already starts being scary ...\n>\n>> + Q - quit; stage this hunk but none of the remaining ones\n>\n> The explanation shouldn't start with \"quit\" I think. I'd say basically\n> \"stage this hunk and quit\" or \"stage this hunk but none of the remaining\n> ones\".\n\nI agree with both points. Other than that, the changes in this round looks\ngood to me.\n\nHermann, care to re-roll for the last time?\n"},{"id":"168050","messageId":"20110517071232.GA19396@mrq1.org","threadId":"27358","inReplyTo":"7vboz1j4k9.fsf@alter.siamese.dyndns.org","subject":"[PATCH] add-interactive: shortcut to add hunk and quit","fromName":"Hermann Gausterer","fromEmail":"git-mailinglist@mrq1.org","sentAt":"2011-05-17T07:12:32Z","receivedAt":"2011-05-17T07:12:32Z","isPatch":true,"sender":{"key":"git-mailinglist@mrq1.org","avatar":null},"body":"this combines the two \"add -i\" commands \"y\"+\"q\" to one.\n\nSigned-off-by: Hermann Gausterer \n---\n Documentation/git-add.txt | 1 +\n git-add--interactive.perl | 6 +++++-\n 2 files changed, 6 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex 9c1d395..329b720 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -280,6 +280,7 @@ patch::\n y - stage this hunk\n n - do not stage this hunk\n q - quit; do not stage this hunk nor any of the remaining ones\n+ Q - stage this hunk but none of the remaining ones\n a - stage this hunk and all later hunks in the file\n d - do not stage this hunk nor any of the later hunks in the file\n g - select a hunk to go to\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 4f08fe7..157a8a7 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -1123,6 +1123,7 @@ sub help_patch_cmd {\n y - $verb this hunk$target\n n - do not $verb this hunk$target\n q - quit; do not $verb this hunk nor any of the remaining ones\n+Q - $verb this hunk but none of the remaining ones\n a - $verb this hunk and all later hunks in the file\n d - do not $verb this hunk nor any of the later hunks in the file\n g - select a hunk to go to\n@@ -1313,7 +1314,7 @@ sub patch_update_file {\n \t\t $hunk[$ix]{TYPE} eq 'deletion' ? ' deletion' :\n \t\t ' this hunk'),\n \t\t $patch_mode_flavour{TARGET},\n-\t\t \" [y,n,q,a,d,/$other,?]? \";\n+\t\t \" [y,n,q,Q,a,d,/$other,?]? \";\n \t\tmy $line = prompt_single_character;\n \t\tif ($line) {\n \t\t\tif ($line =~ /^y/i) {\n@@ -1366,6 +1367,9 @@ sub patch_update_file {\n \t\t\t\tnext;\n \t\t\t}\n \t\t\telsif ($line =~ /^q/i) {\n+\t\t\t\tif ($line =~ /^Q/) {\n+\t\t\t\t\t$hunk[$ix]{USE} = 1;\n+\t\t\t\t}\n \t\t\t\tfor ($i = 0; $i < $num; $i++) {\n \t\t\t\t\tif (!defined $hunk[$i]{USE}) {\n \t\t\t\t\t\t$hunk[$i]{USE} = 0;\n-- \n1.7.0.4\n\n"},{"id":"168106","messageId":"4DD369C3.4070806@pcharlan.com","threadId":"27358","inReplyTo":"20110517071232.GA19396@mrq1.org","subject":"Re: [PATCH] add-interactive: shortcut to add hunk and quit","fromName":"Pete Harlan","fromEmail":"pgit@pcharlan.com","sentAt":"2011-05-18T06:40:03Z","receivedAt":"2011-05-18T06:40:03Z","isPatch":true,"sender":{"key":"pgit@pcharlan.com","avatar":null},"body":"On 05/17/2011 12:12 AM, Hermann Gausterer wrote:\n> this combines the two \"add -i\" commands \"y\"+\"q\" to one.\n\n...\n\n> y - stage this hunk\n> n - do not stage this hunk\n> q - quit; do not stage this hunk nor any of the remaining ones\n> + Q - stage this hunk but none of the remaining ones\n> a - stage this hunk and all later hunks in the file\n> d - do not stage this hunk nor any of the later hunks in the file\n> g - select a hunk to go to\n\nIf \"q\" means \"quit\", I would expect \"Q\" to mean something like \"quit immediately\" (perhaps even undoing earlier adds), not \"do something that 'q' wouldn't do, and then quit\".\n\nPerhaps \"o\" (for \"stage exactly [o]ne commit\"), or \"t\" for \"stage [t]his commit\" would be reasonable alternatives?\n\n--Pete\n"},{"id":"168108","messageId":"20110518064515.GA29612@sigill.intra.peff.net","threadId":"27358","inReplyTo":"4DD369C3.4070806@pcharlan.com","subject":"Re: [PATCH] add-interactive: shortcut to add hunk and quit","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-05-18T06:45:15Z","receivedAt":"2011-05-18T06:45:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 17, 2011 at 11:40:03PM -0700, Pete Harlan wrote:\n\n> On 05/17/2011 12:12 AM, Hermann Gausterer wrote:\n> > this combines the two \"add -i\" commands \"y\"+\"q\" to one.\n> \n> ...\n> \n> > y - stage this hunk\n> > n - do not stage this hunk\n> > q - quit; do not stage this hunk nor any of the remaining ones\n> > + Q - stage this hunk but none of the remaining ones\n> > a - stage this hunk and all later hunks in the file\n> > d - do not stage this hunk nor any of the later hunks in the file\n> > g - select a hunk to go to\n> \n> If \"q\" means \"quit\", I would expect \"Q\" to mean something like \"quit\n> immediately\" (perhaps even undoing earlier adds), not \"do something\n> that 'q' wouldn't do, and then quit\".\n\nI agree. There was some discussion in another thread recently of the\natomicity of git-add (right now it applies the changes to each file\nafter all of its hunks are done). I would expect \"q\" to be \"quit and\napply what I told you so far\" and \"Q\" to be \"quit and do not apply\nanything\".\n\n> Perhaps \"o\" (for \"stage exactly [o]ne commit\"), or \"t\" for \"stage\n> [t]his commit\" would be reasonable alternatives?\n\nWe could also allow multiple commands at once, like \"yq\" (even in\nsingle-key mode, this would do the same thing).\n\n-Peff\n"},{"id":"168118","messageId":"20110518084352.GA14922@mrq1.org","threadId":"27358","inReplyTo":"4DD369C3.4070806@pcharlan.com","subject":"Re: [PATCH] add-interactive: shortcut to add hunk and quit","fromName":"Hermann Gausterer","fromEmail":"git-mailinglist@mrq1.org","sentAt":"2011-05-18T08:43:52Z","receivedAt":"2011-05-18T08:43:52Z","isPatch":true,"sender":{"key":"git-mailinglist@mrq1.org","avatar":null},"body":"On Tue, May 17, 2011 at 11:40:03PM -0700, Pete Harlan wrote:\n> Perhaps \"o\" (for \"stage exactly [o]ne commit\"), or \"t\" for \"stage [t]his commit\" would be reasonable alternatives?\n\nthis option just adds and quits. so the \"best\" name would be \"l\"\nfor \"last\" ...\n"},{"id":"168125","messageId":"4DD390AF.9020705@drmicha.warpmail.net","threadId":"27358","inReplyTo":"20110518064515.GA29612@sigill.intra.peff.net","subject":"Re: [PATCH] add-interactive: shortcut to add hunk and quit","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-05-18T09:26:07Z","receivedAt":"2011-05-18T09:26:07Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jeff King venit, vidit, dixit 18.05.2011 08:45:\n> On Tue, May 17, 2011 at 11:40:03PM -0700, Pete Harlan wrote:\n> \n>> On 05/17/2011 12:12 AM, Hermann Gausterer wrote:\n>>> this combines the two \"add -i\" commands \"y\"+\"q\" to one.\n>>\n>> ...\n>>\n>>> y - stage this hunk\n>>> n - do not stage this hunk\n>>> q - quit; do not stage this hunk nor any of the remaining ones\n>>> + Q - stage this hunk but none of the remaining ones\n>>> a - stage this hunk and all later hunks in the file\n>>> d - do not stage this hunk nor any of the later hunks in the file\n>>> g - select a hunk to go to\n>>\n>> If \"q\" means \"quit\", I would expect \"Q\" to mean something like \"quit\n>> immediately\" (perhaps even undoing earlier adds), not \"do something\n>> that 'q' wouldn't do, and then quit\".\n> \n> I agree. There was some discussion in another thread recently of the\n> atomicity of git-add (right now it applies the changes to each file\n> after all of its hunks are done). I would expect \"q\" to be \"quit and\n> apply what I told you so far\" and \"Q\" to be \"quit and do not apply\n> anything\".\n> \n>> Perhaps \"o\" (for \"stage exactly [o]ne commit\"), or \"t\" for \"stage\n>> [t]his commit\" would be reasonable alternatives?\n> \n> We could also allow multiple commands at once, like \"yq\" (even in\n> single-key mode, this would do the same thing).\n\nSo instead of having to\n\npress y press q\n\nI can now\n\nhold SHIFT press q\n\nSeeing the gain in that fails me completely. Also, why doesn't \"yd\"\ndeserve a shortcut? I would expect that to be used more often, as in:\n\"Yes, that was the hunk I wanted to add from this file, but what other\nfiles have changes\"?\n\nMichael\n"},{"id":"168141","messageId":"7vr57wc9ja.fsf@alter.siamese.dyndns.org","threadId":"27358","inReplyTo":"4DD390AF.9020705@drmicha.warpmail.net","subject":"Re: [PATCH] add-interactive: shortcut to add hunk and quit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-18T15:28:57Z","receivedAt":"2011-05-18T15:28:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n>> We could also allow multiple commands at once, like \"yq\" (even in\n>> single-key mode, this would do the same thing).\n>\n> So instead of having to\n>\n> press y press q\n>\n> I can now\n>\n> hold SHIFT press q\n>\n> Seeing the gain in that fails me completely. Also, why doesn't \"yd\"\n> deserve a shortcut? I would expect that to be used more often, as in:\n> \"Yes, that was the hunk I wanted to add from this file, but what other\n> files have changes\"?\n\nThanks, I agree that \"Q\" as proposed is not very useful and looks too much\nlike a hack that caters to one special user from that point of view.\n\nI've also been wondering why nobody has asked for \"5y\", which I often find\nlacking.\n\nWhen you have a set of changes with many hunks to sift through, before\ngoing into an \"add -p\" session, you often have pretty good idea of hunks\nin which part of the files are to go to the commit you are currently\nbuilding. I often find myself saying \"ah, from here there are many hunks I\nwant, and it is totally safe for me to apply 5 or so from here without\nlooking.\"\n\nI think \"single-key\" was a poorly designed attempt to improve productivity\nthe (\"y\" )*5 into \"y\"*5, while sacrificing the safety net when you\nare trying to pick and decide one by one (like the accident Thomas had\nrecently during \"checkout -p\"). If I can say \"5y\", think for half a second\nto make sure I typed what I meant, and , to apply 5 upcoming hunks in\none go, I think I would be as efficient as the productivity optimization\nthe single-key offers, while still protecting me from mistakes made by fat\nfingers.\n"},{"id":"168202","messageId":"201105191216.51709.trast@student.ethz.ch","threadId":"27358","inReplyTo":"7vr57wc9ja.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] add-interactive: shortcut to add hunk and quit","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2011-05-19T10:16:51Z","receivedAt":"2011-05-19T10:16:51Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Junio C Hamano wrote:\n> \n> I think \"single-key\" was a poorly designed attempt to improve productivity\n> the (\"y\" )*5 into \"y\"*5\n\nActually for me it more often is\n\n y RET n RET *think* y RET s RET n RET ...\n\n> while sacrificing the safety net when you\n> are trying to pick and decide one by one (like the accident Thomas had\n> recently during \"checkout -p\"). If I can say \"5y\", think for half a second\n> to make sure I typed what I meant, and , to apply 5 upcoming hunks in\n> one go, I think I would be as efficient as the productivity optimization\n> the single-key offers, while still protecting me from mistakes made by fat\n> fingers.\n\nThere's nothing stopping us from implementing number prefixes in\nsingle-key mode, since numbers do not have any meaning yet.\n\nAfter my little accident I'm also considering an (optional?) safety\nquestion at the end when in checkout -p mode, since it's inherently\ndestructive. Of course that first requires changing the whole\noperation to be atomic.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"168205","messageId":"20110519110259.GA11507@sigill.intra.peff.net","threadId":"27358","inReplyTo":"201105191216.51709.trast@student.ethz.ch","subject":"Re: [PATCH] add-interactive: shortcut to add hunk and quit","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-05-19T11:02:59Z","receivedAt":"2011-05-19T11:02:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 19, 2011 at 12:16:51PM +0200, Thomas Rast wrote:\n\n> Junio C Hamano wrote:\n> > \n> > I think \"single-key\" was a poorly designed attempt to improve productivity\n> > the (\"y\" )*5 into \"y\"*5\n> \n> Actually for me it more often is\n> \n> y RET n RET *think* y RET s RET n RET ...\n\nYeah. I personally find the concept of \"5y\" crazy; how do you know that\nit is 5, and not 4 or 6, if you haven't yet seen them?\n\nBut that just means I don't have any use for it; I don't have a real\nobjection to it.\n\n> After my little accident I'm also considering an (optional?) safety\n> question at the end when in checkout -p mode, since it's inherently\n> destructive. Of course that first requires changing the whole\n> operation to be atomic.\n\nI think a confirmation question is a bad idea. It helps with\nfat-fingering, but not much else. 99% of the time you will say \"yes\",\nbecause of course you just looked through the changes and want to\nfinalize them. So you will start to hit \"y\" without looking or thinking,\nand it becomes a mere annoyance, until the time you _do_ actually lose\nsome data by hitting \"y\" without thinking.\n\nAt least that's what would happen to me. :)\n\nI think a much better safety valve is to store the user's worktree state\nthat we are about to destroy. Then when they accidentally erase\nsomething, whether they realize it immediately, or even 5 minutes later,\nit is recoverable. And in the common case where everything goes well,\nthey needn't be bothered at all.\n\nThis fits much better with other git recovery mechanisms, too, which\ntend to be one of:\n\n 1. Store the previous state, and optionally instruct the user on how\n to recover in the case of error (e.g., reflogs, the new orphan\n checkout warning).\n\n 2. Force the user to give confirmation (e.g., \"branch -D\"), but _only_\n if we have detected some abnormally dangerous situation (e.g., you\n are deleting a branch that hasn't been merged anywhere). The user\n is more likely to pay attention and think about the confirmation\n because we _don't_ ask every time, and because we are giving them\n additional information that will help in making the decision.\n\n-Peff\n"},{"id":"168250","messageId":"7v7h9m7arx.fsf@alter.siamese.dyndns.org","threadId":"27358","inReplyTo":"20110519110259.GA11507@sigill.intra.peff.net","subject":"Re: [PATCH] add-interactive: shortcut to add hunk and quit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-19T19:25:38Z","receivedAt":"2011-05-19T19:25:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Thu, May 19, 2011 at 12:16:51PM +0200, Thomas Rast wrote:\n>\n>> Junio C Hamano wrote:\n>> > \n>> > I think \"single-key\" was a poorly designed attempt to improve productivity\n>> > the (\"y\" )*5 into \"y\"*5\n>> \n>> Actually for me it more often is\n>> \n>> y RET n RET *think* y RET s RET n RET ...\n>\n> Yeah. I personally find the concept of \"5y\" crazy; how do you know that\n> it is 5, and not 4 or 6, if you haven't yet seen them?\n\nThat one is surprisingly easy to answer. Before I decide to use\n\"incremental\", I've seen the diff at least once but more often number of\ntimes. I know where things are when I start my incremental sessions, and\n\"5\" (just an example) is something I would use when I think I know there\nare 8 or 9, i.e. a number that will surely undershoot but will get me\nto the end sooner. An alternative would be something akin to \"/\"\nbut that adds, instead of skips.\n\n> I think a confirmation question is a bad idea. It helps with\n> fat-fingering, but not much else.\n\nI agree, but fat-fingering is a real problem single-key mode introduces,\nand that is why I suggested a similar final confirmation only for 'a' in\nthe single-key mode.\n\n> I think a much better safety valve is to store the user's worktree state\n> that we are about to destroy. Then when they accidentally erase\n> something, whether they realize it immediately, or even 5 minutes later,\n> it is recoverable. And in the common case where everything goes well,\n> they needn't be bothered at all.\n\nIntereting.\n\nWhere does the data go (perhaps to \"stash create\", not \"stash save\"), and\nwhere would we plug that in (\"checkout -p\" codepath only)?\n"},{"id":"168254","messageId":"20110519194252.GA26144@sigill.intra.peff.net","threadId":"27358","inReplyTo":"7v7h9m7arx.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] add-interactive: shortcut to add hunk and quit","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-05-19T19:42:52Z","receivedAt":"2011-05-19T19:42:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 19, 2011 at 12:25:38PM -0700, Junio C Hamano wrote:\n\n> > Yeah. I personally find the concept of \"5y\" crazy; how do you know that\n> > it is 5, and not 4 or 6, if you haven't yet seen them?\n> \n> That one is surprisingly easy to answer. Before I decide to use\n> \"incremental\", I've seen the diff at least once but more often number of\n> times. I know where things are when I start my incremental sessions, and\n> \"5\" (just an example) is something I would use when I think I know there\n> are 8 or 9, i.e. a number that will surely undershoot but will get me\n> to the end sooner. An alternative would be something akin to \"/\"\n> but that adds, instead of skips.\n\nOK, I figured it was something like that. I still think it's a little\ncrazy, but hey, if it works for you, who am I to tell you you're wrong.\n\n> > I think a much better safety valve is to store the user's worktree state\n> > that we are about to destroy. Then when they accidentally erase\n> > something, whether they realize it immediately, or even 5 minutes later,\n> > it is recoverable. And in the common case where everything goes well,\n> > they needn't be bothered at all.\n> \n> Intereting.\n> \n> Where does the data go (perhaps to \"stash create\", not \"stash save\"), and\n> where would we plug that in (\"checkout -p\" codepath only)?\n\nYeah, definitely not \"stash save\", as we consider the contents of the\nstash list to be under user control. Even \"stash create\" is a bit of an\noverkill, as for \"checkout -p\" we don't care about the index state (er,\nwait, do we? I guess for \"checkout -p $some_commit\", we will be munging\nboth work-tree and index).\n\nUsing \"stash create\", we could easily print a \"by the way, here is your\nprevious state\" message. But I think I prefer a stash-like reflog of\nstates. Then for the common case (you _didn't_ screw up), there is no\nextra cruft printed. Plus, you can go back and recover 5 minutes later,\nwhen you have closed that terminal window and only then realize you\nmessed something up.\n\nSo maybe there should be another stash-like ref at refs/worktree (or\nrefs/WORKTREE?). Then it would expire naturally according to the usual\nreflog expiration rules. We could also write to it during \"git reset\n--hard\", which suffers the same safety issue.\n\nYou could also stash the index state during \"git reset --mixed\" and \"git\nreset -p\". That is not as big an issue, though, as you are only ever\nthrowing away the work of adding things to the index (for that matter,\none could do the same thing on \"git add\"). You may lose a minute or two\nof sorting changes, but you will never lose actual data, as you can with\n\"checkout\" or \"checkout -p\".\n\nObviously this safety valve incurs a performance penalty. Probably it\nshould be optional via config for each callsite. In general, I wouldn't\nexpect it to be too expensive, though. The biggest part will be the \"git\nadd\" of new content; but in theory, this is stuff you might have\ncommitted anyway, so it's probably not that big.\n\nYou could put a similar safety valve in \"git clean\", but it may be much\nmore expensive, since it is by definition files that you have _not_\nmarked to be tracked by git. So they may be large binary cruft.\n\n-Peff\n"}]}