{"thread":{"id":"6776","subject":"[PATCH] Fix cg-commit -p to not touch the working tree","startedAt":"2007-02-12T03:14:31Z","lastAt":"2007-02-12T22:01:57Z","messageCount":4,"participants":["Sam Vilain","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"34234","messageId":"20070212031923.D20B913A382@magnus.utsl.gen.nz","threadId":"6776","inReplyTo":null,"subject":"[PATCH] Fix cg-commit -p to not touch the working tree","fromName":"Sam Vilain","fromEmail":"sam.vilain@catalyst.net.nz","sentAt":"2007-02-12T03:14:31Z","receivedAt":"2007-02-12T03:14:31Z","isPatch":true,"sender":{"key":"sam@vilain.net","avatar":"https://gravatar.com/avatar/8fc840ca854dbf6f7065b4335e3b934951c1dca3b11db688e95e471901f8f4a8?d=mp&s=160"},"body":"Previously, the working tree state was modified with `patch', which\nwas a fragile operation.  Do everything with `git-apply --cached\n--index' instead.\n---\n cg-commit |   33 +++++++++++++++++----------------\n 1 files changed, 17 insertions(+), 16 deletions(-)\n\ndiff --git a/cg-commit b/cg-commit\nindex 360d5e6..f676e1f 100755\n--- a/cg-commit\n+++ b/cg-commit\n@@ -575,22 +575,6 @@ else\n fi\n rm \"$LOGMSG2\"\n \n-if [ \"$review\" ]; then\n-\tif ! cmp -s \"$PATCH\" \"$PATCH2\"; then\n-\t\techo \"Reverting the original patch...\"\n-\t\tif ! cg-patch -R < \"$PATCH\"; then\n-\t\t\tdie \"unable to revert the original patch; the original patch is available in $PATCH, your edited patch is available in $PATCH2, your log message is in $LOGMSG, your working copy is in undefined state now and the world is about to end in ten minutes, have a nice day\"\n-\t\tfi\n-\t\techo \"Applying the edited patch...\"\n-\t\tif ! cg-patch < \"$PATCH2\"; then\n-\t\t\t# FIXME: Do something better to alleviate this situation.\n-\t\t\t# At least restore the tree to the original state.\n-\t\t\tdie \"unable to apply the edited patch; the original patch is available in $PATCH, your edited patch is available in $PATCH2, your log message is in $LOGMSG, your working copy is in undefined state now and the world is about to end in five minutes, have a nice day\"\n-\t\tfi\n-\tfi\n-fi\n-\n-\n precommit_update()\n {\n \tqueueN=(); queueD=(); queueM=();\n@@ -620,6 +604,23 @@ if [ ! \"$ignorecache\" ]; then\n \tprecommit_update \"${commitfiles[@]}\" || die \"update-cache failed\"\n fi\n \n+if [ \"$review\" ]; then\n+\tif ! cmp -s \"$PATCH\" \"$PATCH2\"; then\n+\t\tgit-read-tree HEAD\n+\t\twhile ! git-apply --check --cached --index \"$PATCH2\"\n+\t\tdo\n+\t\t\techo \"patch tried was:\"\n+\t\t\tcat $PATCH2\n+\t\t\techo \"your patch does not apply cleanly, re-edit it!\"\n+\t\t\techo -n \"sleeping 5s...\"\n+\t\t\tsleep 4\n+\t\t\techo \"get it right this time OK?\"\n+\t\t\tsleep 1\n+\t\t\t${EDITOR:-vi} \"$PATCH2\"\n+\t\tdone\n+\t\tgit-apply --cached --index \"$PATCH2\"\n+\tfi\n+fi\n \n oldhead=\n oldheadname=\"$(git-symbolic-ref HEAD)\"\n-- \n1.5.0.rc2.g84cf66-dirty\n"},{"id":"34240","messageId":"7v8xf4atoe.fsf@assigned-by-dhcp.cox.net","threadId":"6776","inReplyTo":"20070212031923.D20B913A382@magnus.utsl.gen.nz","subject":"Re: [PATCH] Fix cg-commit -p to not touch the working tree","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-02-12T04:25:37Z","receivedAt":"2007-02-12T04:25:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sam Vilain <sam.vilain@catalyst.net.nz> writes:\n\n> Previously, the working tree state was modified with `patch', which\n> was a fragile operation.  Do everything with `git-apply --cached\n> --index' instead.\n\nI do not use Cogito so I do not know what behaviour is wanted\nhere, but '--cached --index' is same as saying just '--cached'\nas far as I know.  It will patch against the index and should\nnot touch working tree.  If the original used 'patch' to apply,\nI suspect it wanted to touch the working tree (and possibly, it\nwanted to leave the index alone?), so --cached might be\ncompletely wrong thing to use here?\n"},{"id":"34315","messageId":"45D0CC74.9020606@vilain.net","threadId":"6776","inReplyTo":"7v8xf4atoe.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Fix cg-commit -p to not touch the working tree","fromName":"Sam Vilain","fromEmail":"sam@vilain.net","sentAt":"2007-02-12T20:22:12Z","receivedAt":"2007-02-12T20:22:12Z","isPatch":true,"sender":{"key":"sam@vilain.net","avatar":"https://gravatar.com/avatar/8fc840ca854dbf6f7065b4335e3b934951c1dca3b11db688e95e471901f8f4a8?d=mp&s=160"},"body":"Junio C Hamano wrote:\n>> Previously, the working tree state was modified with `patch', which\n>> was a fragile operation.  Do everything with `git-apply --cached\n>> --index' instead.\n>>     \n> I do not use Cogito so I do not know what behaviour is wanted\n> here, but '--cached --index' is same as saying just '--cached'\n> as far as I know.  It will patch against the index and should\n> not touch working tree.  If the original used 'patch' to apply,\n> I suspect it wanted to touch the working tree (and possibly, it\n> wanted to leave the index alone?), so --cached might be\n> completely wrong thing to use here?\n>   \n\nThe context is that \"cg-commit -p\", a kind of poor man's interactive\ncommit that lets you preview changes that are to be committed in 'diff'\nform, and edit the patch to be applied. Many users expect this command\nto behave this way; they're quite surprised and sometimes even miffed\nwhen the changes they deleted from the patch are gone from their working\ncopy.\n\nSam.\n"},{"id":"34326","messageId":"7vtzxr6nmy.fsf@assigned-by-dhcp.cox.net","threadId":"6776","inReplyTo":"45D0CC74.9020606@vilain.net","subject":"Re: [PATCH] Fix cg-commit -p to not touch the working tree","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-02-12T22:01:57Z","receivedAt":"2007-02-12T22:01:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sam Vilain <sam@vilain.net> writes:\n\n> Junio C Hamano wrote:\n>>> Previously, the working tree state was modified with `patch', which\n>>> was a fragile operation.  Do everything with `git-apply --cached\n>>> --index' instead.\n>>>     \n>> I do not use Cogito so I do not know what behaviour is wanted\n>> here, but '--cached --index' is same as saying just '--cached'\n>> as far as I know.  It will patch against the index and should\n>> not touch working tree.  If the original used 'patch' to apply,\n>> I suspect it wanted to touch the working tree (and possibly, it\n>> wanted to leave the index alone?), so --cached might be\n>> completely wrong thing to use here?\n>>   \n>\n> The context is that \"cg-commit -p\", a kind of poor man's interactive\n> commit that lets you preview changes that are to be committed in 'diff'\n> form, and edit the patch to be applied. Many users expect this command\n> to behave this way; they're quite surprised and sometimes even miffed\n> when the changes they deleted from the patch are gone from their working\n> copy.\n\nAh, I see that's why you do want to leave the working tree\nuntouched.  I think '--cached' alone is the right way to spell\nit (strictly speaking, giving --index and --cached should error\nout, although the current implementation does not).\n"}]}