threads / patch / 16772

patchAdd git-edit-index.perl

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

## tl;dr

8 messages between Dec 17, 2008 and Dec 18, 2008. Diffs are folded; open one to read it.

replies: 7people: 5as markdown or json

Neil Roberts· Dec 17, 2008, 20:47 UTC · lore

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
Show changes to 3 files +100 −0

.gitignore, Makefile, 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
Jeff King· Dec 18, 2008, 04:37 UTC · re: Neil Roberts · lore

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

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.

Show 5 quoted lines
> 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.

Show 7 quoted lines
> +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.
Show 6 quoted lines
> +        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
Johannes Schindelin· Dec 18, 2008, 13:48 UTC · re: Jeff King · lore

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

Hi,
On Wed, 17 Dec 2008, Jeff King wrote:
Show 8 quoted lines
> 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

Jeff King· Dec 18, 2008, 14:04 UTC · re: Johannes Schindelin · lore

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

On Thu, Dec 18, 2008 at 02:48:39PM +0100, Johannes Schindelin wrote:
Show 5 quoted lines
> 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
Johannes Schindelin· Dec 18, 2008, 16:24 UTC · re: Jeff King · lore

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

Hi,
On Thu, 18 Dec 2008, Jeff King wrote:
Show 5 quoted lines
> 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

Miklos Vajna· Dec 18, 2008, 16:36 UTC · re: Johannes Schindelin · lore

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

On Thu, Dec 18, 2008 at 05:24:00PM +0100, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
Show 11 quoted lines
> 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.

Johannes Schindelin· Dec 18, 2008, 19:47 UTC · re: Miklos Vajna · lore

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

Hi,
On Thu, 18 Dec 2008, Miklos Vajna wrote:
Show 13 quoted lines
> 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

Junio C Hamano· Dec 18, 2008, 21:40 UTC · re: Jeff King · lore

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

Jeff King <peff@peff.net> writes:
Show 7 quoted lines
> 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.

Show 15 quoted lines
> 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.

← back to recent threads