{"thread":{"id":"17416","subject":"[PATCH v3 2/2] git-am: minor cleanups","startedAt":"2009-01-28T15:03:09Z","lastAt":"2009-01-28T18:52:51Z","messageCount":9,"participants":["Jay Soffian","Pieter de Bie","Jeff King","Junio C Hamano","Sverre Rabbelier"],"isPatch":true,"patchVersion":3,"patchTotal":2},"messages":[{"id":"102313","messageId":"1233154990-19745-1-git-send-email-jaysoffian@gmail.com","threadId":"17416","inReplyTo":null,"subject":"[PATCH v3 1/2] git-am: emit usage when called w/o arguments and w/o patch on stdin","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-01-28T15:03:09Z","receivedAt":"2009-01-28T15:03:09Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"When git am is called w/o arguments, w/o a patch on stdin and the user hits\nctrl-c, it leaves behind a partially populated $dotest directory. After this\ncommit, it emits usage when called w/o arguments and w/o a patch on stdin.\n\nNoticed by Sverre Rabbelier\n\nSigned-off-by: Jay Soffian \n---\nChange from v2: make Junio happy by no longer removing $dotest if git-am is\ninterupted while mailsplit is running.\n\n git-am.sh | 1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/git-am.sh b/git-am.sh\nindex b1c05c9..92a64b2 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -254,6 +254,7 @@ else\n \t\tdone\n \t\tshift\n \tfi\n+\ttest $# = 0 && test -t 0 && usage\n \tgit mailsplit -d\"$prec\" -o\"$dotest\" -b -- \"$@\" > \"$dotest/last\" || {\n \t\trm -fr \"$dotest\"\n \t\texit 1\n-- \n1.6.1.224.gb56c\n"},{"id":"102312","messageId":"1233154990-19745-2-git-send-email-jaysoffian@gmail.com","threadId":"17416","inReplyTo":"1233154990-19745-1-git-send-email-jaysoffian@gmail.com","subject":"[PATCH v3 2/2] git-am: minor cleanups","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-01-28T15:03:10Z","receivedAt":"2009-01-28T15:03:10Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"Update usage statement to remove a no-longer supported option, and to hide two\noptions (one a no-op, one internal) unless --help-all is used.\n\nUse \"test -t 0\" instead of \"tty -s\" to detect when stdin is a terminal. (test\n-t 0 is used elsewhere in git-am and in other git shell scripts, tty -s is\nnot, and appears to be deprecated by POSIX)\n\nUse \"test ...\" instead of \"[ ... ]\" and \"die \" instead of \"echo \n>&2; exit 1\" to be consistent with rest of script.\n\nSigned-off-by: Jay Soffian \n---\nChange from v2: mention who deprecated \"tty -s\" in commit message.\n\n git-am.sh | 12 ++++--------\n 1 files changed, 4 insertions(+), 8 deletions(-)\n\ndiff --git a/git-am.sh b/git-am.sh\nindex 92a64b2..670fc02 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -8,9 +8,8 @@ OPTIONS_SPEC=\"\\\n git am [options] [|...]\n git am [options] (--resolved | --skip | --abort)\n --\n-d,dotest= (removed -- do not use)\n i,interactive run interactively\n-b,binary (historical option -- no-op)\n+b,binary* (historical option -- no-op)\n 3,3way allow fall back on 3way merging if needed\n s,signoff add a Signed-off-by line to the commit message\n u,utf8 recode into utf8 (default)\n@@ -24,7 +23,7 @@ resolvemsg= override error message when patch failure occurs\n r,resolved to be used after a patch failure\n skip skip the current patch\n abort restore the original branch and abort the patching operation.\n-rebasing (internal use for git-rebase)\"\n+rebasing* (internal use for git-rebase)\"\n \n . git-sh-setup\n prefix=$(git rev-parse --show-prefix)\n@@ -204,7 +203,7 @@ then\n \t\t# unreliable -- stdin could be /dev/null for example\n \t\t# and the caller did not intend to feed us a patch but\n \t\t# wanted to continue unattended.\n-\t\ttty -s\n+\t\ttest -t 0\n \t\t;;\n \t*)\n \t\tfalse\n@@ -281,10 +280,7 @@ fi\n case \"$resolved\" in\n '')\n \tfiles=$(git diff-index --cached --name-only HEAD --) || exit\n-\tif [ \"$files\" ]; then\n-\t echo \"Dirty index: cannot apply patches (dirty: $files)\" >&2\n-\t exit 1\n-\tfi\n+\ttest \"$files\" && die \"Dirty index: cannot apply patches (dirty: $files)\"\n esac\n \n if test \"$(cat \"$dotest/utf8\")\" = t\n-- \n1.6.1.224.gb56c\n"},{"id":"102319","messageId":"780A42F8-E27C-404A-945C-38C16378EF57@ai.rug.nl","threadId":"17416","inReplyTo":"1233154990-19745-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH v3 1/2] git-am: emit usage when called w/o arguments and w/o patch on stdin","fromName":"Pieter de Bie","fromEmail":"pdebie@ai.rug.nl","sentAt":"2009-01-28T16:18:30Z","receivedAt":"2009-01-28T16:18:30Z","isPatch":true,"sender":{"key":"pdebie@ai.rug.nl","avatar":null},"body":"\nOn Jan 28, 2009, at 3:03 PM, Jay Soffian wrote:\n\n> When git am is called w/o arguments, w/o a patch on stdin and the \n> user hits\n> ctrl-c, it leaves behind a partially populated $dotest directory. \n> After this\n> commit, it emits usage when called w/o arguments and w/o a patch on \n> stdin.\n\nFWIW, I sometimes like to run 'git am', paste in a patch and hit ctrl-d.\n\nI can probably retrain my finger to use 'git am -', but I'm not sure \nif that\nworks (after this patch)? At least it's not mentioned in the manpage.\n\n- Pieter\n"},{"id":"102321","messageId":"76718490901280840g7d36aa96u3d0f98a709ac9fde@mail.gmail.com","threadId":"17416","inReplyTo":"780A42F8-E27C-404A-945C-38C16378EF57@ai.rug.nl","subject":"Re: [PATCH v3 1/2] git-am: emit usage when called w/o arguments and w/o patch on stdin","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-01-28T16:40:37Z","receivedAt":"2009-01-28T16:40:37Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Wed, Jan 28, 2009 at 11:18 AM, Pieter de Bie wrote:\n> FWIW, I sometimes like to run 'git am', paste in a patch and hit ctrl-d.\n>\n> I can probably retrain my finger to use 'git am -', but I'm not sure if that\n> works (after this patch)? At least it's not mentioned in the manpage.\n\n\"git am -\" doesn't work (before or after this patch), but \"cat | git am\" will.\n\nj.\n"},{"id":"102326","messageId":"20090128175106.GD8863@coredump.intra.peff.net","threadId":"17416","inReplyTo":"780A42F8-E27C-404A-945C-38C16378EF57@ai.rug.nl","subject":"Re: [PATCH v3 1/2] git-am: emit usage when called w/o arguments and w/o patch on stdin","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-01-28T17:51:06Z","receivedAt":"2009-01-28T17:51:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 28, 2009 at 04:18:30PM +0000, Pieter de Bie wrote:\n\n> FWIW, I sometimes like to run 'git am', paste in a patch and hit ctrl-d.\n\nI have to admit, I usually am opposed to this sort of terminal DWIMmery\nfor exactly that reason. But I don't personally ever cut and paste into\ngit-am, so I was trying not to raise a fuss. ;)\n\n-Peff\n"},{"id":"102330","messageId":"7v3af3thyj.fsf@gitster.siamese.dyndns.org","threadId":"17416","inReplyTo":"76718490901280840g7d36aa96u3d0f98a709ac9fde@mail.gmail.com","subject":"Re: [PATCH v3 1/2] git-am: emit usage when called w/o arguments and w/o patch on stdin","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-28T18:15:48Z","receivedAt":"2009-01-28T18:15:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jay Soffian writes:\n\n> On Wed, Jan 28, 2009 at 11:18 AM, Pieter de Bie wrote:\n>> FWIW, I sometimes like to run 'git am', paste in a patch and hit ctrl-d.\n>>\n>> I can probably retrain my finger to use 'git am -', but I'm not sure if that\n>> works (after this patch)? At least it's not mentioned in the manpage.\n>\n> \"git am -\" doesn't work (before or after this patch), but \"cat | git am\" will.\n\nAnother approach we've taken in other places to avoid the \"Huh?\" that\ntriggered this thread is to do something like:\n\n if there is no argument\n then\n\t\tif reading from tty\n\t then\n\t \techo >&2 \"Reading from terminal, waiting for input...\"\n\t\tfi\n process stdin\t\n\telse\n \tfor arg\n do\n \tprocess $arg\n\t\tdone\n\tfi\n\nUnfortunately, this will invalidate your \"check -t 0 and error out\" patch,\nbut some people may find it easier to work with and more friendly.\n"},{"id":"102334","messageId":"76718490901281026i45c26cb0r978f795a50dede19@mail.gmail.com","threadId":"17416","inReplyTo":"7v3af3thyj.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v3 1/2] git-am: emit usage when called w/o arguments and w/o patch on stdin","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-01-28T18:26:51Z","receivedAt":"2009-01-28T18:26:51Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Wed, Jan 28, 2009 at 1:15 PM, Junio C Hamano wrote:\n> Another approach we've taken in other places to avoid the \"Huh?\" that\n> triggered this thread is to do something like:\n>\n> if there is no argument\n> then\n> if reading from tty\n> then\n> echo >&2 \"Reading from terminal, waiting for input...\"\n> fi\n> process stdin\n> else\n> for arg\n> do\n> process $arg\n> done\n> fi\n>\n> Unfortunately, this will invalidate your \"check -t 0 and error out\" patch,\n> but some people may find it easier to work with and more friendly.\n\nWell perhaps we should just deal w/ctrl-c only and ignore the terminal\ncheck altogether.\n\nj.\n"},{"id":"102336","messageId":"bd6139dc0901281033r27b84f5dx7b55e1cfcf796a76@mail.gmail.com","threadId":"17416","inReplyTo":"76718490901281026i45c26cb0r978f795a50dede19@mail.gmail.com","subject":"Re: [PATCH v3 1/2] git-am: emit usage when called w/o arguments and w/o patch on stdin","fromName":"Sverre Rabbelier","fromEmail":"alturin@gmail.com","sentAt":"2009-01-28T18:33:48Z","receivedAt":"2009-01-28T18:33:48Z","isPatch":true,"sender":{"key":"alturin@gmail.com","avatar":null},"body":"On Wed, Jan 28, 2009 at 19:26, Jay Soffian wrote:\n> Well perhaps we should just deal w/ctrl-c only and ignore the terminal\n> check altogether.\n\nThat would suit my needs; as long as doing \"git am\" and then ^C does\nnot change my worktree and .git directory. Doing a stray \"git am\n--abort\" after aborting does not obliterate my worktree as it does now\n(not nice!), even so, it will change where my branch is currently at!\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"102337","messageId":"7v3af3s1oc.fsf@gitster.siamese.dyndns.org","threadId":"17416","inReplyTo":"1233154990-19745-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH v3 1/2] git-am: emit usage when called w/o arguments and w/o patch on stdin","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-28T18:52:51Z","receivedAt":"2009-01-28T18:52:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jay Soffian writes:\n\n> When git am is called w/o arguments, w/o a patch on stdin and the user hits\n> ctrl-c, it leaves behind a partially populated $dotest directory. After this\n> commit, it emits usage when called w/o arguments and w/o a patch on stdin.\n>\n> Noticed by Sverre Rabbelier\n>\n> Signed-off-by: Jay Soffian \n> ---\n> Change from v2: make Junio happy by no longer removing $dotest if git-am is\n> interupted while mailsplit is running.\n\nSorry, I think I was misleading. I did not mean that \"Also\" change was a\nbad thing to do. It just did not logically belong to this \"is it coming\nfrom the tty?\" issue.\n\nWhile you are looking at \"am\", I have a complaint about a very rough and\ndangerous edge around a relatively new \"usability\" feature.\n\nA scenario goes like this.\n\n (1) You have a patch, try to apply it to one existing topic branch\n\n\t$ git checkout myTopic\n\t$ git am some-patch\n\n (2) You see it does not apply, you realize that the patch is more suited\n for another branch.\n\n\t$ git checkout anotherTopic\n $ git am\n\n (3) It still does not apply, you examine the patch, and find flaws in its\n logic and decide not to apply it at all anywhere.\n\n\t$ git am --abort\n\nAfter this, the tip of anotherTopic is updated to where the tip of myTopic\nwas!\n\nBeing able to remove the state directory $GIT_DIR/rebase-apply from the\ncommand line is very useful when you want to get a clean slate to run\nanother git-am session, and often it is handy to abort the application of\nthe whole series when you do so (the latter is done by resetting the\noriginal branch back to the original commit). But being able to switch\nbranches after starting application of patches from one mailbox is also\nuseful in practice, so at least the latter should not be done blindly when\nthe user asks for --abort. We definitely need some safety here.\n\nPerhaps....\n\n (1) If --abort is asked for and you are on a branch different from where\n you started your \"git am\" session from, error out without doing\n anything.\n\n (2) Add --clean that does what the current --abort does, except for the\n rewinding of the HEAD.\n\nThe second one may be useful regardless of the \"rough and dangerous edge\"\nissue above. The early parts of the series may be useful and you may want\nto keep the result from their application, while discarding the remainder\nof the series.\n\nHmm?\n"}]}