[PATCH] Add git-edit-index.perl

[PATCH] Add git-edit-index.perl

[Neil Roberts]

This script can be used to edit a file in the index without affecting
your working tree. It checkouts a copy of the file to a temporary file
and runs an editor on it. If the editor completes successfully with a
non-empty file then it updates the index with the new data.

This is useful to fine tune the results from git add -p. For example
sometimes your unrelated changes are too close together and
git-add--interactive will refuse to split them up. Using this script
you can add both the changes and later edit the index file to
temporarily remove one of the changes.

Signed-off-by: Neil Roberts <bpeeluk@yahoo.co.uk>
---
 .gitignore          |    1 +
 Makefile            |    1 +
 git-edit-index.perl |   98 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 100 insertions(+), 0 deletions(-)
 create mode 100755 git-edit-index.perl

diff --git a/.gitignore b/.gitignore
index d9adce5..251d90b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -36,6 +36,7 @@ git-diff-files
 git-diff-index
 git-diff-tree
 git-describe
+git-edit-index
 git-fast-export
 git-fast-import
 git-fetch
diff --git a/Makefile b/Makefile
index 5158197..77ee97f 100644
--- a/Makefile
+++ b/Makefile
@@ -275,6 +275,7 @@ SCRIPT_PERL += git-archimport.perl
 SCRIPT_PERL += git-cvsexportcommit.perl
 SCRIPT_PERL += git-cvsimport.perl
 SCRIPT_PERL += git-cvsserver.perl
+SCRIPT_PERL += git-edit-index.perl
 SCRIPT_PERL += git-relink.perl
 SCRIPT_PERL += git-send-email.perl
 SCRIPT_PERL += git-svn.perl
diff --git a/git-edit-index.perl b/git-edit-index.perl
new file mode 100755
index 0000000..a5d9886
--- /dev/null
+++ b/git-edit-index.perl
@@ -0,0 +1,98 @@
+#!/usr/bin/perl -w
+#
+# Copyright 2008 Neil Roberts <bpeeluk@yahoo.co.uk>
+#
+# GPL v2 (See COPYING)
+#
+# Opens an editor on a copy of a file in the index and updates it when
+# the editor is finished. This can be used to fine tune to results of
+# git add -p
+
+use strict;
+use warnings;
+use Git;
+
+sub usage {
+	print <<EOT;
+git edit-index <file>...
+EOT
+	exit(1);
+}
+
+sub delete_temp_files {
+        # Delete the temporary files created by checkout-index
+        # --temp. The output from checkout-index should be passed as
+        # arguments
+        foreach my $fnfull (@_) {
+                my ($tmp_fn, $fn) = split(/\t/, $fnfull);
+                unlink($tmp_fn);
+        }
+}
+
+sub check_file_size {
+        my ($fn) = @_;
+        my ($dev, $ino, $mode, $nlink, $uid, $gid, $rdev, $size,
+            $atime, $mtime, $ctime, $blksize, $blocks) = stat($fn);
+
+        $size;
+}
+
+usage unless @ARGV;
+
+my $repo = Git->repository();
+
+my %file_modes;
+
+my $editor = $ENV{GIT_EDITOR}
+    || $repo->config("core.editor")
+    || $ENV{VISUAL}
+    || $ENV{EDITOR}
+    || "vi";
+
+# Create a temporary copy of each file in the index
+my @file_list = $repo->command(qw(checkout-index --temp --), @ARGV);
+
+# Get the current mode of each file
+foreach my $fnfull (@file_list) {
+        my ($tmp_fn, $fn) = split(/\t/, $fnfull);
+        my ($file_details) = $repo->command_oneline(qw(ls-files --stage --),
+                                                    $fn);
+        unless (defined($file_details) && $file_details =~ /\A([0-7]{6}) /)
+        {
+                delete_temp_files(@file_list);
+                die("$fn is not in the index");
+        }
+
+        $file_modes{$fn} = $1;
+}
+
+# Edit each file
+foreach my $fnfull (@file_list) {
+        my ($tmp_fn, $fn) = split(/\t/, $fnfull);
+
+        unless (system($editor, $tmp_fn) == 0
+                && check_file_size($tmp_fn)) {
+                # If the editor failed, the file has disappeared or it
+                # has zero size then give up
+                delete_temp_files(@file_list);
+                die("Editor failed or file has zero size");
+        }
+}
+
+# Add each file back to the index
+foreach my $fnfull (@file_list) {
+        my ($tmp_fn, $fn) = split(/\t/, $fnfull);
+
+        my $hash = $repo->command_oneline(qw(hash-object -w --), $tmp_fn);
+
+        unless (defined($hash) && $hash =~ /\A[0-9a-f]{40}\z/) {
+                delete_temp_files(@file_list);
+                die("Failed to add new file");
+        }
+
+        $repo->command(qw(update-index --cacheinfo),
+                       $file_modes{$fn}, $hash, $fn);
+}
+
+# Clean up the temporary files
+delete_temp_files(@file_list);
-- 
1.5.6.3

Re: [PATCH] Add git-edit-index.perl

[Jeff King]

On Wed, Dec 17, 2008 at 08:47:49PM +0000, Neil Roberts wrote:

> This script can be used to edit a file in the index without affecting
> your working tree. It checkouts a copy of the file to a temporary file
> and runs an editor on it. If the editor completes successfully with a
> non-empty file then it updates the index with the new data.

Hmm. Neat idea. I have used add-interactive's "e"dit patch option to do
a similar thing, but it is often unwieldy (e.g., just yesterday I had a
patch that removed about 30 lines and added 1 -- rather than munging the
diff, it would have been simpler to re-add the line in a staged
version).

Thinking out loud: One thing that would make this more useful would be
providing the content of the work tree file in some way. Like seeing the
whole file but with "conflict markers" showing two versions of each
hunk, like:

  a line in both files
  <<<<<<< staged
  a line only in the staged version
  =======
  a line only in the working tree version
  >>>>>>> working tree

and then you can edit around that. Of course, then you _have_ to
edit every hunk since you have just stuck the conflict marker cruft. I
guess a savvy user could just open both versions in their editor and
pull content from one buffer to the other other.

> This is useful to fine tune the results from git add -p. For example
> sometimes your unrelated changes are too close together and
> git-add--interactive will refuse to split them up. Using this script
> you can add both the changes and later edit the index file to
> temporarily remove one of the changes.

Have you tried using the edit-patch option in "git add -p"? I'm curious
if you would like that better for your cases, or if you find this way
more natural.

>  .gitignore          |    1 +
>  Makefile            |    1 +
>  git-edit-index.perl |   98 +++++++++++++++++++++++++++++++++++++++++++++++++++

I have to wonder if this wouldn't be better as part of
git-add--interactive? I guess technically you aren't "adding" from the
work tree since it is purely looking at the staged version. But it seems
to be part of the same workflow, as you are munging content in the
index.

> +sub check_file_size {
> +        my ($fn) = @_;
> +        my ($dev, $ino, $mode, $nlink, $uid, $gid, $rdev, $size,
> +            $atime, $mtime, $ctime, $blksize, $blocks) = stat($fn);
> +
> +        $size;
> +}

FYI, a shorthand for this is:

  (stat $fn))[7];

or you may consider:

  use File::stat;
  stat($fn)->size;

which is nicely readable.

> +        unless (system($editor, $tmp_fn) == 0
> +                && check_file_size($tmp_fn)) {
> +                # If the editor failed, the file has disappeared or it
> +                # has zero size then give up
> +                delete_temp_files(@file_list);
> +                die("Editor failed or file has zero size");

I guess you are aborting on a zero-size file to allow the user a chance
to say "oops, I want to scrap the changes I've saved so far". But you
are disallowing making a file empty by this process. Which I guess is
not all that common, but it still seems restrictive.  I wonder if asking
for confirmation might make more sense.

Also, does it make sense to delete the temp files if the editor failed?
The user may have put work into the file, but we are not successfully
updating the index; so we may be deleting useful work that could be
recovered.

> +        unless (defined($hash) && $hash =~ /\A[0-9a-f]{40}\z/) {
> +                delete_temp_files(@file_list);
> +                die("Failed to add new file");

Again, if we fail to hash for whatever reason, we should not delete the
useful work that the user might have done.

-Peff

Re: [PATCH] Add git-edit-index.perl

[Johannes Schindelin]

Hi,

On Wed, 17 Dec 2008, Jeff King wrote:

> On Wed, Dec 17, 2008 at 08:47:49PM +0000, Neil Roberts wrote:
> 
> > This script can be used to edit a file in the index without affecting 
> > your working tree. It checkouts a copy of the file to a temporary file 
> > and runs an editor on it. If the editor completes successfully with a 
> > non-empty file then it updates the index with the new data.
> 
> Hmm. Neat idea.

Yes, it is a neat idea.  But I always keep in mind what Junio had to say 
about my "add -e" thing (that I use pretty frequently myself): you will 
put something into the index that has _never_ been tested.

Would we really want to bless such a workflow with "official" support?

Ciao,
Dscho

Re: [PATCH] Add git-edit-index.perl

[Jeff King]

On Thu, Dec 18, 2008 at 02:48:39PM +0100, Johannes Schindelin wrote:

> Yes, it is a neat idea.  But I always keep in mind what Junio had to say 
> about my "add -e" thing (that I use pretty frequently myself): you will 
> put something into the index that has _never_ been tested.
> 
> Would we really want to bless such a workflow with "official" support?

That is definitely something to be concerned about. Which is why my
workflow is something like:

  $ hack hack hack
  $ while ! git diff; do
      git add -p
      git commit
    done
  $ for i in `git rev-list origin..`; do
      git checkout $i && make test || barf
    done

That is, it is not inherently a problem to put something untested into
the index as long as you are doing it so that you can go back and test
later.

It _would_ be a nicer workflow to say "I don't want these changes yet"
and selectively put them elsewhere, test what's in the working tree,
commit, and then grab some more changes from your stash. But we don't
have interactive stashing and unstashing yet, which would be required
for that.

-Peff

Re: [PATCH] Add git-edit-index.perl

[Johannes Schindelin]

Hi,

On Thu, 18 Dec 2008, Jeff King wrote:

> It _would_ be a nicer workflow to say "I don't want these changes yet" 
> and selectively put them elsewhere, test what's in the working tree, 
> commit, and then grab some more changes from your stash. But we don't 
> have interactive stashing and unstashing yet, which would be required 
> for that.

git stash -i... Yes, I'd like that!

Ciao,
Dscho

Re: [PATCH] Add git-edit-index.perl

[Miklos Vajna]

[-- Attachment #1: Type: text/plain, Size: 698 bytes --]

On Thu, Dec 18, 2008 at 05:24:00PM +0100, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi,
> 
> On Thu, 18 Dec 2008, Jeff King wrote:
> 
> > It _would_ be a nicer workflow to say "I don't want these changes yet" 
> > and selectively put them elsewhere, test what's in the working tree, 
> > commit, and then grab some more changes from your stash. But we don't 
> > have interactive stashing and unstashing yet, which would be required 
> > for that.
> 
> git stash -i... Yes, I'd like that!

Or git checkout -i?

For the cases when you did two changes in a file, you realize one of
them is not yet necessary, but you don't want to stage/commit the other
change yet.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] Add git-edit-index.perl

[Johannes Schindelin]

Hi,

On Thu, 18 Dec 2008, Miklos Vajna wrote:

> On Thu, Dec 18, 2008 at 05:24:00PM +0100, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> 
> > On Thu, 18 Dec 2008, Jeff King wrote:
> > 
> > > It _would_ be a nicer workflow to say "I don't want these changes 
> > > yet" and selectively put them elsewhere, test what's in the working 
> > > tree, commit, and then grab some more changes from your stash. But 
> > > we don't have interactive stashing and unstashing yet, which would 
> > > be required for that.
> > 
> > git stash -i... Yes, I'd like that!
> 
> Or git checkout -i?

Frankly, I need to move changes away much more often.  Plus, you could 
have what you wished for with a "git checkout -- <path> && git stash -i".  
It's just that you would move out the changes you would not want yet.

Ciao,
Dscho

Re: [PATCH] Add git-edit-index.perl

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Thu, Dec 18, 2008 at 02:48:39PM +0100, Johannes Schindelin wrote:
>
>> Yes, it is a neat idea.  But I always keep in mind what Junio had to say 
>> about my "add -e" thing (that I use pretty frequently myself): you will 
>> put something into the index that has _never_ been tested.
>> 
>> Would we really want to bless such a workflow with "official" support?

Back in stone ages of git, there wasn't usable tool support to make random
unproven commits, later to be tested separately before releasing.  The old
aversion to committing something that has never existed as a whole in the
work tree comes from those days.

The world has changed quite a bit since then, and I do not think the
argument holds anymore when better tool support for "commit first,
validate and fix-up as needed later" workflow is available.

> That is definitely something to be concerned about. Which is why my
> workflow is something like:
>
>   $ hack hack hack
>   $ while ! git diff; do
>       git add -p
>       git commit
>     done
>   $ for i in `git rev-list origin..`; do
>       git checkout $i && make test || barf
>     done
>
> That is, it is not inherently a problem to put something untested into
> the index as long as you are doing it so that you can go back and test
> later.

Yeah, I do not think there is anything inherently wrong about it, either.