{"thread":{"id":"16772","subject":"[PATCH] Add git-edit-index.perl","startedAt":"2008-12-17T20:47:49Z","lastAt":"2008-12-18T21:40:34Z","messageCount":8,"participants":["Neil Roberts","Jeff King","Johannes Schindelin","Miklos Vajna","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"98181","messageId":"20081217204749.GA18261@janet.wally","threadId":"16772","inReplyTo":null,"subject":"[PATCH] Add git-edit-index.perl","fromName":"Neil Roberts","fromEmail":"bpeeluk@yahoo.co.uk","sentAt":"2008-12-17T20:47:49Z","receivedAt":"2008-12-17T20:47:49Z","isPatch":true,"sender":{"key":"bpeeluk@yahoo.co.uk","avatar":"https://gravatar.com/avatar/91a7a1ae011c2431594cf617b8c5e8af439673b724caa86836f22c51254deb93?d=mp&s=160"},"body":"This script can be used to edit a file in the index without affecting\nyour working tree. It checkouts a copy of the file to a temporary file\nand runs an editor on it. If the editor completes successfully with a\nnon-empty file then it updates the index with the new data.\n\nThis is useful to fine tune the results from git add -p. For example\nsometimes your unrelated changes are too close together and\ngit-add--interactive will refuse to split them up. Using this script\nyou can add both the changes and later edit the index file to\ntemporarily remove one of the changes.\n\nSigned-off-by: Neil Roberts \n---\n .gitignore | 1 +\n Makefile | 1 +\n git-edit-index.perl | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 100 insertions(+), 0 deletions(-)\n create mode 100755 git-edit-index.perl\n\ndiff --git a/.gitignore b/.gitignore\nindex d9adce5..251d90b 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -36,6 +36,7 @@ git-diff-files\n git-diff-index\n git-diff-tree\n git-describe\n+git-edit-index\n git-fast-export\n git-fast-import\n git-fetch\ndiff --git a/Makefile b/Makefile\nindex 5158197..77ee97f 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -275,6 +275,7 @@ SCRIPT_PERL += git-archimport.perl\n SCRIPT_PERL += git-cvsexportcommit.perl\n SCRIPT_PERL += git-cvsimport.perl\n SCRIPT_PERL += git-cvsserver.perl\n+SCRIPT_PERL += git-edit-index.perl\n SCRIPT_PERL += git-relink.perl\n SCRIPT_PERL += git-send-email.perl\n SCRIPT_PERL += git-svn.perl\ndiff --git a/git-edit-index.perl b/git-edit-index.perl\nnew file mode 100755\nindex 0000000..a5d9886\n--- /dev/null\n+++ b/git-edit-index.perl\n@@ -0,0 +1,98 @@\n+#!/usr/bin/perl -w\n+#\n+# Copyright 2008 Neil Roberts \n+#\n+# GPL v2 (See COPYING)\n+#\n+# Opens an editor on a copy of a file in the index and updates it when\n+# the editor is finished. This can be used to fine tune to results of\n+# git add -p\n+\n+use strict;\n+use warnings;\n+use Git;\n+\n+sub usage {\n+\tprint <...\n+EOT\n+\texit(1);\n+}\n+\n+sub delete_temp_files {\n+ # Delete the temporary files created by checkout-index\n+ # --temp. The output from checkout-index should be passed as\n+ # arguments\n+ foreach my $fnfull (@_) {\n+ my ($tmp_fn, $fn) = split(/\\t/, $fnfull);\n+ unlink($tmp_fn);\n+ }\n+}\n+\n+sub check_file_size {\n+ my ($fn) = @_;\n+ my ($dev, $ino, $mode, $nlink, $uid, $gid, $rdev, $size,\n+ $atime, $mtime, $ctime, $blksize, $blocks) = stat($fn);\n+\n+ $size;\n+}\n+\n+usage unless @ARGV;\n+\n+my $repo = Git->repository();\n+\n+my %file_modes;\n+\n+my $editor = $ENV{GIT_EDITOR}\n+ || $repo->config(\"core.editor\")\n+ || $ENV{VISUAL}\n+ || $ENV{EDITOR}\n+ || \"vi\";\n+\n+# Create a temporary copy of each file in the index\n+my @file_list = $repo->command(qw(checkout-index --temp --), @ARGV);\n+\n+# Get the current mode of each file\n+foreach my $fnfull (@file_list) {\n+ my ($tmp_fn, $fn) = split(/\\t/, $fnfull);\n+ my ($file_details) = $repo->command_oneline(qw(ls-files --stage --),\n+ $fn);\n+ unless (defined($file_details) && $file_details =~ /\\A([0-7]{6}) /)\n+ {\n+ delete_temp_files(@file_list);\n+ die(\"$fn is not in the index\");\n+ }\n+\n+ $file_modes{$fn} = $1;\n+}\n+\n+# Edit each file\n+foreach my $fnfull (@file_list) {\n+ my ($tmp_fn, $fn) = split(/\\t/, $fnfull);\n+\n+ unless (system($editor, $tmp_fn) == 0\n+ && check_file_size($tmp_fn)) {\n+ # If the editor failed, the file has disappeared or it\n+ # has zero size then give up\n+ delete_temp_files(@file_list);\n+ die(\"Editor failed or file has zero size\");\n+ }\n+}\n+\n+# Add each file back to the index\n+foreach my $fnfull (@file_list) {\n+ my ($tmp_fn, $fn) = split(/\\t/, $fnfull);\n+\n+ my $hash = $repo->command_oneline(qw(hash-object -w --), $tmp_fn);\n+\n+ unless (defined($hash) && $hash =~ /\\A[0-9a-f]{40}\\z/) {\n+ delete_temp_files(@file_list);\n+ die(\"Failed to add new file\");\n+ }\n+\n+ $repo->command(qw(update-index --cacheinfo),\n+ $file_modes{$fn}, $hash, $fn);\n+}\n+\n+# Clean up the temporary files\n+delete_temp_files(@file_list);\n-- \n1.5.6.3\n"},{"id":"98225","messageId":"20081218043734.GD20749@coredump.intra.peff.net","threadId":"16772","inReplyTo":"20081217204749.GA18261@janet.wally","subject":"Re: [PATCH] Add git-edit-index.perl","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-12-18T04:37:34Z","receivedAt":"2008-12-18T04:37:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Dec 17, 2008 at 08:47:49PM +0000, Neil Roberts wrote:\n\n> This script can be used to edit a file in the index without affecting\n> your working tree. It checkouts a copy of the file to a temporary file\n> and runs an editor on it. If the editor completes successfully with a\n> non-empty file then it updates the index with the new data.\n\nHmm. Neat idea. I have used add-interactive's \"e\"dit patch option to do\na similar thing, but it is often unwieldy (e.g., just yesterday I had a\npatch that removed about 30 lines and added 1 -- rather than munging the\ndiff, it would have been simpler to re-add the line in a staged\nversion).\n\nThinking out loud: One thing that would make this more useful would be\nproviding the content of the work tree file in some way. Like seeing the\nwhole file but with \"conflict markers\" showing two versions of each\nhunk, like:\n\n a line in both files\n <<<<<<< staged\n a line only in the staged version\n =======\n a line only in the working tree version\n >>>>>>> working tree\n\nand then you can edit around that. Of course, then you _have_ to\nedit every hunk since you have just stuck the conflict marker cruft. I\nguess a savvy user could just open both versions in their editor and\npull content from one buffer to the other other.\n\n> This is useful to fine tune the results from git add -p. For example\n> sometimes your unrelated changes are too close together and\n> git-add--interactive will refuse to split them up. Using this script\n> you can add both the changes and later edit the index file to\n> temporarily remove one of the changes.\n\nHave you tried using the edit-patch option in \"git add -p\"? I'm curious\nif you would like that better for your cases, or if you find this way\nmore natural.\n\n> .gitignore | 1 +\n> Makefile | 1 +\n> git-edit-index.perl | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++\n\nI have to wonder if this wouldn't be better as part of\ngit-add--interactive? I guess technically you aren't \"adding\" from the\nwork tree since it is purely looking at the staged version. But it seems\nto be part of the same workflow, as you are munging content in the\nindex.\n\n> +sub check_file_size {\n> + my ($fn) = @_;\n> + my ($dev, $ino, $mode, $nlink, $uid, $gid, $rdev, $size,\n> + $atime, $mtime, $ctime, $blksize, $blocks) = stat($fn);\n> +\n> + $size;\n> +}\n\nFYI, a shorthand for this is:\n\n (stat $fn))[7];\n\nor you may consider:\n\n use File::stat;\n stat($fn)->size;\n\nwhich is nicely readable.\n\n> + unless (system($editor, $tmp_fn) == 0\n> + && check_file_size($tmp_fn)) {\n> + # If the editor failed, the file has disappeared or it\n> + # has zero size then give up\n> + delete_temp_files(@file_list);\n> + die(\"Editor failed or file has zero size\");\n\nI guess you are aborting on a zero-size file to allow the user a chance\nto say \"oops, I want to scrap the changes I've saved so far\". But you\nare disallowing making a file empty by this process. Which I guess is\nnot all that common, but it still seems restrictive. I wonder if asking\nfor confirmation might make more sense.\n\nAlso, does it make sense to delete the temp files if the editor failed?\nThe user may have put work into the file, but we are not successfully\nupdating the index; so we may be deleting useful work that could be\nrecovered.\n\n> + unless (defined($hash) && $hash =~ /\\A[0-9a-f]{40}\\z/) {\n> + delete_temp_files(@file_list);\n> + die(\"Failed to add new file\");\n\nAgain, if we fail to hash for whatever reason, we should not delete the\nuseful work that the user might have done.\n\n-Peff\n"},{"id":"98251","messageId":"alpine.DEB.1.00.0812181446430.6952@intel-tinevez-2-302","threadId":"16772","inReplyTo":"20081218043734.GD20749@coredump.intra.peff.net","subject":"Re: [PATCH] Add git-edit-index.perl","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-12-18T13:48:39Z","receivedAt":"2008-12-18T13:48:39Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 17 Dec 2008, Jeff King wrote:\n\n> On Wed, Dec 17, 2008 at 08:47:49PM +0000, Neil Roberts wrote:\n> \n> > This script can be used to edit a file in the index without affecting \n> > your working tree. It checkouts a copy of the file to a temporary file \n> > and runs an editor on it. If the editor completes successfully with a \n> > non-empty file then it updates the index with the new data.\n> \n> Hmm. Neat idea.\n\nYes, it is a neat idea. But I always keep in mind what Junio had to say \nabout my \"add -e\" thing (that I use pretty frequently myself): you will \nput something into the index that has _never_ been tested.\n\nWould we really want to bless such a workflow with \"official\" support?\n\nCiao,\nDscho\n"},{"id":"98254","messageId":"20081218140411.GB6706@coredump.intra.peff.net","threadId":"16772","inReplyTo":"alpine.DEB.1.00.0812181446430.6952@intel-tinevez-2-302","subject":"Re: [PATCH] Add git-edit-index.perl","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-12-18T14:04:11Z","receivedAt":"2008-12-18T14:04:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 18, 2008 at 02:48:39PM +0100, Johannes Schindelin wrote:\n\n> Yes, it is a neat idea. But I always keep in mind what Junio had to say \n> about my \"add -e\" thing (that I use pretty frequently myself): you will \n> put something into the index that has _never_ been tested.\n> \n> Would we really want to bless such a workflow with \"official\" support?\n\nThat is definitely something to be concerned about. Which is why my\nworkflow is something like:\n\n $ hack hack hack\n $ while ! git diff; do\n git add -p\n git commit\n done\n $ for i in `git rev-list origin..`; do\n git checkout $i && make test || barf\n done\n\nThat is, it is not inherently a problem to put something untested into\nthe index as long as you are doing it so that you can go back and test\nlater.\n\nIt _would_ be a nicer workflow to say \"I don't want these changes yet\"\nand selectively put them elsewhere, test what's in the working tree,\ncommit, and then grab some more changes from your stash. But we don't\nhave interactive stashing and unstashing yet, which would be required\nfor that.\n\n-Peff\n"},{"id":"98260","messageId":"alpine.DEB.1.00.0812181723340.6952@intel-tinevez-2-302","threadId":"16772","inReplyTo":"20081218140411.GB6706@coredump.intra.peff.net","subject":"Re: [PATCH] Add git-edit-index.perl","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-12-18T16:24:00Z","receivedAt":"2008-12-18T16:24:00Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 18 Dec 2008, Jeff King wrote:\n\n> It _would_ be a nicer workflow to say \"I don't want these changes yet\" \n> and selectively put them elsewhere, test what's in the working tree, \n> commit, and then grab some more changes from your stash. But we don't \n> have interactive stashing and unstashing yet, which would be required \n> for that.\n\ngit stash -i... Yes, I'd like that!\n\nCiao,\nDscho\n"},{"id":"98261","messageId":"20081218163654.GR5691@genesis.frugalware.org","threadId":"16772","inReplyTo":"alpine.DEB.1.00.0812181723340.6952@intel-tinevez-2-302","subject":"Re: [PATCH] Add git-edit-index.perl","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-12-18T16:36:54Z","receivedAt":"2008-12-18T16:36:54Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Thu, Dec 18, 2008 at 05:24:00PM +0100, Johannes Schindelin wrote:\n> Hi,\n> \n> On Thu, 18 Dec 2008, Jeff King wrote:\n> \n> > It _would_ be a nicer workflow to say \"I don't want these changes yet\" \n> > and selectively put them elsewhere, test what's in the working tree, \n> > commit, and then grab some more changes from your stash. But we don't \n> > have interactive stashing and unstashing yet, which would be required \n> > for that.\n> \n> git stash -i... Yes, I'd like that!\n\nOr git checkout -i?\n\nFor the cases when you did two changes in a file, you realize one of\nthem is not yet necessary, but you don't want to stage/commit the other\nchange yet.\n"},{"id":"98277","messageId":"alpine.DEB.1.00.0812182046060.6952@intel-tinevez-2-302","threadId":"16772","inReplyTo":"20081218163654.GR5691@genesis.frugalware.org","subject":"Re: [PATCH] Add git-edit-index.perl","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-12-18T19:47:13Z","receivedAt":"2008-12-18T19:47:13Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 18 Dec 2008, Miklos Vajna wrote:\n\n> On Thu, Dec 18, 2008 at 05:24:00PM +0100, Johannes Schindelin wrote:\n> \n> > On Thu, 18 Dec 2008, Jeff King wrote:\n> > \n> > > It _would_ be a nicer workflow to say \"I don't want these changes \n> > > yet\" and selectively put them elsewhere, test what's in the working \n> > > tree, commit, and then grab some more changes from your stash. But \n> > > we don't have interactive stashing and unstashing yet, which would \n> > > be required for that.\n> > \n> > git stash -i... Yes, I'd like that!\n> \n> Or git checkout -i?\n\nFrankly, I need to move changes away much more often. Plus, you could \nhave what you wished for with a \"git checkout -- && git stash -i\". \nIt's just that you would move out the changes you would not want yet.\n\nCiao,\nDscho\n"},{"id":"98289","messageId":"7vprjpf9gt.fsf@gitster.siamese.dyndns.org","threadId":"16772","inReplyTo":"20081218140411.GB6706@coredump.intra.peff.net","subject":"Re: [PATCH] Add git-edit-index.perl","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-12-18T21:40:34Z","receivedAt":"2008-12-18T21:40:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Thu, Dec 18, 2008 at 02:48:39PM +0100, Johannes Schindelin wrote:\n>\n>> Yes, it is a neat idea. But I always keep in mind what Junio had to say \n>> about my \"add -e\" thing (that I use pretty frequently myself): you will \n>> put something into the index that has _never_ been tested.\n>> \n>> Would we really want to bless such a workflow with \"official\" support?\n\nBack in stone ages of git, there wasn't usable tool support to make random\nunproven commits, later to be tested separately before releasing. The old\naversion to committing something that has never existed as a whole in the\nwork tree comes from those days.\n\nThe world has changed quite a bit since then, and I do not think the\nargument holds anymore when better tool support for \"commit first,\nvalidate and fix-up as needed later\" workflow is available.\n\n> That is definitely something to be concerned about. Which is why my\n> workflow is something like:\n>\n> $ hack hack hack\n> $ while ! git diff; do\n> git add -p\n> git commit\n> done\n> $ for i in `git rev-list origin..`; do\n> git checkout $i && make test || barf\n> done\n>\n> That is, it is not inherently a problem to put something untested into\n> the index as long as you are doing it so that you can go back and test\n> later.\n\nYeah, I do not think there is anything inherently wrong about it, either.\n"}]}