{"thread":{"id":"2713","subject":"[PROBE] cg-commit: show and enable editing of changes with --review","startedAt":"2005-11-30T00:01:31Z","lastAt":"2005-12-01T13:20:25Z","messageCount":3,"participants":["Jonas Fonseca","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"12963","messageId":"20051130000131.GC5365@diku.dk","threadId":"2713","inReplyTo":null,"subject":"[PROBE] cg-commit: show and enable editing of changes with --review","fromName":"Jonas Fonseca","fromEmail":"fonseca@diku.dk","sentAt":"2005-11-30T00:01:31Z","receivedAt":"2005-11-30T00:01:31Z","isPatch":false,"sender":{"key":"fonseca@diku.dk","avatar":"https://gravatar.com/avatar/f82f3ad698717c51873b020c750a92438c820a24056dc39fe4d07baa10a92264?d=mp&s=160"},"body":"Show changes being commited as a patch appended to the commit message\nbuffer. If the original patch is different from the patch extracted from\nthe commit message file the original patch will be reverted and the edited\npatch applied before completing the commit.\n\nDue to limitations with cg-patch this can only be used when commiting\nfrom the project root directory. The error handling if the either the\noriginal patch or the edited patch does not apply is not optimal, since\ncg-patch will not report errors properly.\n\nSigned-off-by: Jonas Fonseca <fonseca@diku.dk>\n\n---\ncommit 5031c74c4716793784314f836f4ceb6bcfa6698a\ntree d01b7a66054352a89c637d58c7ab6454a26ba700\nparent dd872816a04f6462686e282b00d3e1523e3ecf4c\nauthor Jonas Fonseca <fonseca@diku.dk> Wed, 30 Nov 2005 00:41:06 +0100\ncommitter Jonas Fonseca <fonseca@antimatter.localdomain> Wed, 30 Nov 2005 00:41:06 +0100\n\n TODO      |    4 ----\n cg-commit |   63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--\n 2 files changed, 61 insertions(+), 6 deletions(-)\n\ndiff --git a/TODO b/TODO\nindex 777c728..cffa753 100644\n--- a/TODO\n+++ b/TODO\n@@ -79,10 +79,6 @@ cg-*patch should be pre-1.0.)\n \t\t  whitespace errors and stuff; hooks are good for this too,\n \t\t  but I think it's good to have internal support for the\n \t\t  basic stuff.\n-\t-- post 1.0 --\n-\t* Patch-altering cg-commit\n-\t\tYou can already alter the list of files to be committed,\n-\t\tlet's optionally show the patch and let you alter it, too.\n \n * cg-push over HTTP\n \ndiff --git a/cg-commit b/cg-commit\nindex 9f8d5d5..1a3c361 100755\n--- a/cg-commit\n+++ b/cg-commit\n@@ -63,6 +63,12 @@\n #\tOptionally, specify the exact name and email to sign off with by\n #\tpassing: `--signoff=\"Author Name <user@example.com>\"`.\n #\n+# --review:: Show and enable editing of changes being committed\n+#\tShow changes being commited as a patch appended to the commit message\n+#\tbuffer. Changes made to the patch will be reapplied before completing\n+#\tthe commit. Only makes sense with interactive commit message editing.\n+#\tNote, can only be used when commiting from the project root directory.\n+#\n # FILES\n # -----\n # $GIT_DIR/author::\n@@ -153,6 +159,7 @@ ignorecache=\n infoonly=\n commitalways=\n missingok=\n+review=\n signoff=\n copy_commit=\n msgs=()\n@@ -172,6 +179,9 @@ while optparse; do\n \t\tforce=1\n \telif optparse -q; then\n \t\tquiet=1\n+\telif optparse --review; then\n+\t\t[ \"$_git_relpath\" ] && die \"--review can only be used when run from project root\"\n+\t\treview=1\n \telif optparse -s || optparse --signoff; then\n \t\t[ \"$signoff\" ] || signoff=\"$(git-var GIT_AUTHOR_IDENT | sed 's/> .*/>/')\"\n \telif optparse --signoff=; then\n@@ -187,6 +197,11 @@ done\n \n [ \"$ignorecache\" ] || cg-object-id HEAD >/dev/null 2>&1 || die \"no previous commit; use -C for the initial commit\"\n \n+if [ \"$review\" ]; then\n+\tPATCH=$(mktemp -t gitci.XXXXXX)\n+\tPATCH2=$(mktemp -t gitci.XXXXXX)\n+fi\n+\n if [ \"$ARGS\" -o \"$_git_relpath\" ]; then\n \t[ \"$ignorecache\" ] && die \"-C and listing files to commit does not make sense\"\n \t[ -s $_git/merging ] && die \"cannot commit individual files when merging\"\n@@ -201,6 +216,7 @@ if [ \"$ARGS\" -o \"$_git_relpath\" ]; then\n \t\tsed 's/^\\([^\t]*\\)\\(.\t.*\\)\\(\t.*\\)*$/\"\\2\"/'))\n \tcustomfiles=1\n \n+\t[ \"$review\" ] && < $filter | path_xargs git-diff-index -r -p HEAD > $PATCH\n \trm $filter\n \n else\n@@ -227,6 +243,8 @@ else\n \t\tfi\n \tfi\n \n+\t[ \"$review\" ] && git-diff-index -r -p HEAD > $PATCH\n+\n \tmerging=\n \t[ -s $_git/merging ] && merging=$(cat $_git/merging | sed 's/^/-p /')\n fi\n@@ -347,7 +365,21 @@ if [ ! \"$ignorecache\" ]; then\n \tfi\n fi\n echo \"CG: -----------------------------------------------------------------------\" >>$LOGMSG\n-echo \"CG: vim: textwidth=75\" >>$LOGMSG\n+\n+if [ \"$review\" ]; then\n+\t{\n+\t\techo \"CG:\"\n+\t\techo \"CG: Change summary:\"\n+\t\techo \"CG:\"\n+\t\tgit-apply --stat --summary < $PATCH | sed 's/^/CG: /'\n+\t\techo \"CG:\"\n+\t\tcat $PATCH\n+\t} >>$LOGMSG\n+\n+\tftdiff=\"filetype=diff\"\n+fi\n+\n+echo \"CG: vim: textwidth=75 $ftdiff\" >>$LOGMSG\n \n cp $LOGMSG $LOGMSG2\n if tty -s; then\n@@ -359,6 +391,7 @@ if tty -s; then\n \t\t\t\tread -p 'Abort or commit? [ac] ' choice\n \t\t\t\tif [ \"$choice\" = \"a\" ] || [ \"$choice\" = \"q\" ]; then\n \t\t\t\t\trm $LOGMSG $LOGMSG2\n+\t\t\t\t\t[ \"$review\" ] && rm $PATCH $PATCH2\n \t\t\t\t\techo \"Commit message not modified, commit aborted\" >&2\n \t\t\t\t\tif [ \"$merging\" ]; then\n \t\t\t\t\t\tcat >&2 <<__END__\n@@ -400,9 +433,35 @@ else\n \tcat >$LOGMSG2\n fi\n # Remove heading and trailing blank lines.\n-grep -v ^CG: $LOGMSG2 | git-stripspace >$LOGMSG\n+if [ ! \"$review\" ]; then\n+\tgrep -v ^CG: $LOGMSG2 | git-stripspace >$LOGMSG\n+else\n+\tsed '/^CG: Change summary:/,$d' < $LOGMSG2 | grep -v ^CG: | git-stripspace >$LOGMSG\n+\tsed -n '/^CG: Change summary:/,$p' < $LOGMSG2 | grep -v ^CG: > $PATCH2\n+fi\n rm $LOGMSG2\n \n+if [ \"$review\" ]; then\n+\terror_msg=\n+\tif ! cmp -s $PATCH $PATCH2; then\n+\t\techo \"Updating changes to edited patch\"\n+\t\t# FIXME: Can only be run from the top level\n+\t\t# FIXME: Is very 'fragile' error handling. We should probably\n+\t\t# save the original patch in a local file for recovery?\n+\t\tif ! cg-patch -R < $PATCH; then\n+\t\t\tbackup=$(mktemp commit-backup.XXXXXX)\n+\t\t\tcp $PATCH $backup\n+\t\t\terror_msg=\"unable to revert original patch, backup saved to $backup\"\n+\t\telif ! cg-patch < $PATCH2; then\n+\t\t\tbackup=$(mktemp commit-backup.XXXXXX)\n+\t\t\tcp $PATCH2 $backup\n+\t\t\terror_msg=\"unable to apply edited patch, backup saved to $backup\"\n+\t\tfi\n+\tfi\n+\trm $PATCH $PATCH2\n+\t[ \"$error_msg\" ] && { rm $LOGMSG; die \"$error_msg\" }\n+fi\n+\n \n precommit_update()\n {\n\n-- \nJonas Fonseca\n"},{"id":"12965","messageId":"7vbr03564t.fsf@assigned-by-dhcp.cox.net","threadId":"2713","inReplyTo":"20051130000131.GC5365@diku.dk","subject":"Re: [PROBE] cg-commit: show and enable editing of changes with --review","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-11-30T00:30:42Z","receivedAt":"2005-11-30T00:30:42Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonas Fonseca <fonseca@diku.dk> writes:\n\n> Show changes being commited as a patch appended to the commit message\n> buffer. If the original patch is different from the patch extracted from\n> the commit message file the original patch will be reverted and the edited\n> patch applied before completing the commit.\n>\n> Due to limitations with cg-patch this can only be used when commiting\n> from the project root directory. The error handling if the either the\n> original patch or the edited patch does not apply is not optimal, since\n> cg-patch will not report errors properly.\n\nI do not do Porcelains, I am not a Cogito user, and I generally\ndo not like encouraging people who are playing an individual\ndeveloper role to commit something that has never existed in\ntheir working tree (hence by definition never been tested),\nbut...\n\n> +\t\techo \"Updating changes to edited patch\"\n> +\t\t# FIXME: Can only be run from the top level\n> +\t\t# FIXME: Is very 'fragile' error handling. We should probably\n> +\t\t# save the original patch in a local file for recovery?\n> +\t\tif ! cg-patch -R < $PATCH; then\n> +\t\t\tbackup=$(mktemp commit-backup.XXXXXX)\n> +\t\t\tcp $PATCH $backup\n> +\t\t\terror_msg=\"unable to revert original patch, backup saved to $backup\"\n\nI suspect at least you should be able to use checkout-index for\nthe first one instead of cg-patch.\n"},{"id":"13044","messageId":"20051201132025.GA3874@diku.dk","threadId":"2713","inReplyTo":"7vbr03564t.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PROBE] cg-commit: show and enable editing of changes with --review","fromName":"Jonas Fonseca","fromEmail":"fonseca@diku.dk","sentAt":"2005-12-01T13:20:25Z","receivedAt":"2005-12-01T13:20:25Z","isPatch":false,"sender":{"key":"fonseca@diku.dk","avatar":"https://gravatar.com/avatar/f82f3ad698717c51873b020c750a92438c820a24056dc39fe4d07baa10a92264?d=mp&s=160"},"body":"Junio C Hamano <junkio@cox.net> wrote Tue, Nov 29, 2005:\n> Jonas Fonseca <fonseca@diku.dk> writes:\n> \n> > Show changes being commited as a patch appended to the commit message\n> > buffer. If the original patch is different from the patch extracted from\n> > the commit message file the original patch will be reverted and the edited\n> > patch applied before completing the commit.\n> >\n> > Due to limitations with cg-patch this can only be used when commiting\n> > from the project root directory. The error handling if the either the\n> > original patch or the edited patch does not apply is not optimal, since\n> > cg-patch will not report errors properly.\n> \n> I do not do Porcelains,\n\nI am curious since you have also used this phrase in the past: didn't\nyou do your own jit porcelain once and are you saying that git does not\nship with it's own porcelain?\n\n> I am not a Cogito user, and I generally\n> do not like encouraging people who are playing an individual\n> developer role to commit something that has never existed in\n> their working tree (hence by definition never been tested),\n> but...\n\nI see your point. I mostly intend to use it for fixing small typos in\ncomments and documentation. It can help to cure some \"post-commit\nstress\".  Also I hope you will agree that it is always a good idea to\nreview your changes one more time although I agree it might be a little\nsilly.\n\n> > +\t\techo \"Updating changes to edited patch\"\n> > +\t\t# FIXME: Can only be run from the top level\n> > +\t\t# FIXME: Is very 'fragile' error handling. We should probably\n> > +\t\t# save the original patch in a local file for recovery?\n> > +\t\tif ! cg-patch -R < $PATCH; then\n> > +\t\t\tbackup=$(mktemp commit-backup.XXXXXX)\n> > +\t\t\tcp $PATCH $backup\n> > +\t\t\terror_msg=\"unable to revert original patch, backup saved to $backup\"\n> \n> I suspect at least you should be able to use checkout-index for\n> the first one instead of cg-patch.\n\nOh, great, thanks. I guess I should work some more on the git docs to\nget to know these kind of things.\n\n-- \nJonas Fonseca\n"}]}