[PATCH] git-add: introduce --edit (to edit the diff vs. the index)

[PATCH] git-add: introduce --edit (to edit the diff vs. the index)

[Johannes Schindelin]

With "git add -e [<files>]", Git will fire up an editor with the current
diff relative to the index (i.e. what you would get with "git diff
[<files>]").

Now you can edit the patch as much as you like, including adding/removing
lines, editing the text, whatever.  Make sure, though, that the first
character of the hunk lines is still a space, a plus or a minus.

After you closed the editor, Git will adjust the line counts of the hunks 
if necessary, thanks to the --recount option of apply, and commit the 
patch.  Except if you deleted everything, in which case nothing happens 
(for obvious reasons).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	I actually promised myself not to resend this patch, but a 
	certain guy who has a hat now asked for it.

 Documentation/git-add.txt |   11 ++++-
 builtin-add.c             |   59 +++++++++++++++++++++++-
 t/t3702-add-edit.sh       |  109 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 175 insertions(+), 4 deletions(-)
 create mode 100755 t/t3702-add-edit.sh

diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index ce71838..25e6667 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -9,7 +9,7 @@ SYNOPSIS
 --------
 [verse]
 'git add' [-n] [-v] [--force | -f] [--interactive | -i] [--patch | -p]
-	  [--all | [--update | -u]] [--intent-to-add | -N]
+	  [--edit | -e] [--all | [--update | -u]] [--intent-to-add | -N]
 	  [--refresh] [--ignore-errors] [--] <filepattern>...
 
 DESCRIPTION
@@ -76,6 +76,15 @@ OPTIONS
 	bypassed and the 'patch' subcommand is invoked using each of
 	the specified filepatterns before exiting.
 
+-e, \--edit::
+	Open the diff vs. the index in an editor and let the user
+	edit it.  After the editor was closed, adjust the hunk headers
+	and apply the patch to the index.
++
+*NOTE*: Obviously, if you change anything else than the first character
+on lines beginning with a space or a minus, the patch will no longer
+apply.
+
 -u::
 --update::
 	Update only files that git already knows about, staging modified
diff --git a/builtin-add.c b/builtin-add.c
index cb67d2c..e931974 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -10,12 +10,14 @@
 #include "cache-tree.h"
 #include "run-command.h"
 #include "parse-options.h"
+#include "diff.h"
+#include "revision.h"
 
 static const char * const builtin_add_usage[] = {
 	"git add [options] [--] <filepattern>...",
 	NULL
 };
-static int patch_interactive, add_interactive;
+static int patch_interactive, add_interactive, edit_interactive;
 static int take_worktree_changes;
 
 static void fill_pathspec_matches(const char **pathspec, char *seen, int specs)
@@ -187,6 +189,51 @@ int interactive_add(int argc, const char **argv, const char *prefix)
 	return status;
 }
 
+int edit_patch(int argc, const char **argv, const char *prefix)
+{
+	char *file = xstrdup(git_path("ADD_EDIT.patch"));
+	const char *apply_argv[] = { "apply", "--recount", "--cached",
+		file, NULL };
+	struct child_process child;
+	struct rev_info rev;
+	int out;
+	struct stat st;
+
+	git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
+
+	if (read_cache() < 0)
+		die ("Could not read the index");
+
+	init_revisions(&rev, prefix);
+	rev.diffopt.context = 7;
+
+	argc = setup_revisions(argc, argv, &rev, NULL);
+	rev.diffopt.output_format = DIFF_FORMAT_PATCH;
+	out = open(file, O_CREAT | O_WRONLY, 0644);
+	if (out < 0)
+		die ("Could not open '%s' for writing.", file);
+	rev.diffopt.file = fdopen(out, "w");
+	rev.diffopt.close_file = 1;
+	if (run_diff_files(&rev, 0))
+		die ("Could not write patch");
+
+	launch_editor(file, NULL, NULL);
+
+	if (stat(file, &st))
+		die("Could not stat '%s'", file);
+	if (!st.st_size)
+		die("Empty patch. Aborted.");
+
+	memset(&child, 0, sizeof(child));
+	child.git_cmd = 1;
+	child.argv = apply_argv;
+	if (run_command(&child))
+		die ("Could not apply '%s'", file);
+
+	unlink(file);
+	return 0;
+}
+
 static struct lock_file lock_file;
 
 static const char ignore_error[] =
@@ -201,6 +248,7 @@ static struct option builtin_add_options[] = {
 	OPT_GROUP(""),
 	OPT_BOOLEAN('i', "interactive", &add_interactive, "interactive picking"),
 	OPT_BOOLEAN('p', "patch", &patch_interactive, "interactive patching"),
+	OPT_BOOLEAN('e', "edit", &edit_interactive, "super-interactive patching"),
 	OPT_BOOLEAN('f', "force", &ignored_too, "allow adding otherwise ignored files"),
 	OPT_BOOLEAN('u', "update", &take_worktree_changes, "update tracked files"),
 	OPT_BOOLEAN('N', "intent-to-add", &intent_to_add, "record only the fact that the path will be added later"),
@@ -251,14 +299,19 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 	int require_pathspec;
 
 	argc = parse_options(argc, argv, builtin_add_options,
-			  builtin_add_usage, 0);
+			  builtin_add_usage, PARSE_OPT_KEEP_ARGV0);
 	if (patch_interactive)
 		add_interactive = 1;
 	if (add_interactive)
-		exit(interactive_add(argc, argv, prefix));
+		exit(interactive_add(argc - 1, argv + 1, prefix));
 
 	git_config(add_config, NULL);
 
+	if (edit_interactive)
+		return(edit_patch(argc, argv, prefix));
+	argc--;
+	argv++;
+
 	if (addremove && take_worktree_changes)
 		die("-A and -u are mutually incompatible");
 	if ((addremove || take_worktree_changes) && !argc) {
diff --git a/t/t3702-add-edit.sh b/t/t3702-add-edit.sh
new file mode 100755
index 0000000..7262786
--- /dev/null
+++ b/t/t3702-add-edit.sh
@@ -0,0 +1,109 @@
+#!/bin/sh
+#
+# Copyright (c) 2007 Johannes E. Schindelin
+#
+
+test_description='add -e basic tests'
+. ./test-lib.sh
+
+
+cat > file << EOF
+LO, praise of the prowess of people-kings
+of spear-armed Danes, in days long sped,
+we have heard, and what honor the athelings won!
+Oft Scyld the Scefing from squadroned foes,
+from many a tribe, the mead-bench tore,
+awing the earls. Since erst he lay
+friendless, a foundling, fate repaid him:
+for he waxed under welkin, in wealth he throve,
+till before him the folk, both far and near,
+who house by the whale-path, heard his mandate,
+gave him gifts:  a good king he!
+EOF
+
+test_expect_success 'setup' '
+
+	git add file &&
+	test_tick &&
+	git commit -m initial file
+
+'
+
+cat > expected-patch << EOF
+diff --git a/file b/file
+index b9834b5..0b8f197 100644
+--- a/file
++++ b/file
+@@ -1,11 +1,3 @@
+-LO, praise of the prowess of people-kings
+-of spear-armed Danes, in days long sped,
+-we have heard, and what honor the athelings won!
+-Oft Scyld the Scefing from squadroned foes,
+-from many a tribe, the mead-bench tore,
+-awing the earls. Since erst he lay
+-friendless, a foundling, fate repaid him:
+-for he waxed under welkin, in wealth he throve,
+-till before him the folk, both far and near,
+-who house by the whale-path, heard his mandate,
+-gave him gifts:  a good king he!
++#!$SHELL_PATH
++mv -f "\$1" orig-patch &&
++mv -f patch "\$1"
+EOF
+
+cat > patch << EOF
+diff --git a/file b/file
+index b9834b5..ef6e94c 100644
+--- a/file
++++ b/file
+@@ -3,1 +3,333 @@ of spear-armed Danes, in days long sped,
+ we have heard, and what honor the athelings won!
++
+ Oft Scyld the Scefing from squadroned foes,
+@@ -2,7 +1,5 @@ awing the earls. Since erst he lay
+ friendless, a foundling, fate repaid him:
++
+ for he waxed under welkin, in wealth he throve,
+EOF
+
+cat > expected << EOF
+diff --git a/file b/file
+index b9834b5..ef6e94c 100644
+--- a/file
++++ b/file
+@@ -1,10 +1,12 @@
+ LO, praise of the prowess of people-kings
+ of spear-armed Danes, in days long sped,
+ we have heard, and what honor the athelings won!
++
+ Oft Scyld the Scefing from squadroned foes,
+ from many a tribe, the mead-bench tore,
+ awing the earls. Since erst he lay
+ friendless, a foundling, fate repaid him:
++
+ for he waxed under welkin, in wealth he throve,
+ till before him the folk, both far and near,
+ who house by the whale-path, heard his mandate,
+EOF
+
+echo "#!$SHELL_PATH" >fake-editor.sh
+cat >> fake-editor.sh <<\EOF
+mv -f "$1" orig-patch &&
+mv -f patch "$1"
+EOF
+
+test_set_editor "$(pwd)/fake-editor.sh"
+chmod a+x fake-editor.sh
+
+test_expect_success 'add -e' '
+
+	cp fake-editor.sh file &&
+	git add -e &&
+	test_cmp fake-editor.sh file &&
+	test_cmp orig-patch expected-patch &&
+	git diff --cached > out &&
+	test_cmp out expected
+
+'
+
+test_done
-- 
1.6.2.1.613.g25746

Re: [PATCH] git-add: introduce --edit (to edit the diff vs. the index)

[Sverre Rabbelier]

Heya,

On Wed, Apr 8, 2009 at 23:30, Johannes Schindelin <johannes.schindelin@gmx.de> >
>        I actually promised myself not to resend this patch, but a
>        certain guy who has a hat now asked for it.

Whoever it may be that asked you for it, I am glad they did! It's
really nice to help massage a patch series into shape. I'm a big fan
of 'git add -p', this would seem like the logical compliment to it imo
:).

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH] git-add: introduce --edit (to edit the diff vs. the index)

[Ping Yin]

On Thu, Apr 9, 2009 at 5:30 AM, Johannes Schindelin
<johannes.schindelin@gmx.de> wrote:
> With "git add -e [<files>]", Git will fire up an editor with the current
> diff relative to the index (i.e. what you would get with "git diff
> [<files>]").
>
> Now you can edit the patch as much as you like, including adding/removing
> lines, editing the text, whatever.  Make sure, though, that the first
> character of the hunk lines is still a space, a plus or a minus.
>
> After you closed the editor, Git will adjust the line counts of the hunks
> if necessary, thanks to the --recount option of apply, and commit the
> patch.  Except if you deleted everything, in which case nothing happens
> (for obvious reasons).
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>
>        I actually promised myself not to resend this patch, but a
>        certain guy who has a hat now asked for it.
>

I am that guy :-). Thanks. Sometimes "add -e" is more useful than "add
-p" since i can see the full context when editing the patch. However,
sometimes the ability to edit the index directly is even more useful.
For example, if it's a big change (or rewritten), it is hard to edit
the patch instead of the index (in diff mode with the worktree file
side by side). I even encounter a case that i can't beat the patch
into a shape i want when using 'add -p'  ( it will fail to apply)

Re: [PATCH] git-add: introduce --edit (to edit the diff vs. the index)

[Ping Yin]

On Thu, Apr 9, 2009 at 10:00 AM, Ping Yin <pkufranky@gmail.com> wrote:
> On Thu, Apr 9, 2009 at 5:30 AM, Johannes Schindelin
> <johannes.schindelin@gmx.de> wrote:
>> With "git add -e [<files>]", Git will fire up an editor with the current
>> diff relative to the index (i.e. what you would get with "git diff
>> [<files>]").
>>
>> Now you can edit the patch as much as you like, including adding/removing
>> lines, editing the text, whatever.  Make sure, though, that the first
>> character of the hunk lines is still a space, a plus or a minus.
>>
>> After you closed the editor, Git will adjust the line counts of the hunks
>> if necessary, thanks to the --recount option of apply, and commit the
>> patch.  Except if you deleted everything, in which case nothing happens
>> (for obvious reasons).
>>
>> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>> ---
>>
>>        I actually promised myself not to resend this patch, but a
>>        certain guy who has a hat now asked for it.
>>
>
> I am that guy :-). Thanks. Sometimes "add -e" is more useful than "add
> -p" since i can see the full context when editing the patch. However,
> sometimes the ability to edit the index directly is even more useful.
> For example, if it's a big change (or rewritten), it is hard to edit
> the patch instead of the index (in diff mode with the worktree file
> side by side). I even encounter a case that i can't beat the patch
> into a shape i want when using 'add -p'  ( it will fail to apply)
>

How about this?
 'add --edit=patch' to edit the patch and "add --edit=index" to edit the index

Re: [PATCH] git-add: introduce --edit (to edit the diff vs. the index)

[Ping Yin]

On Fri, Apr 10, 2009 at 9:43 AM, Ping Yin <pkufranky@gmail.com> wrote:

> How about this?
>  'add --edit=patch' to edit the patch and "add --edit=index" to edit the index
>

One usecase for edit index

After a big change in foo.c in the worktree, you find a typo in foo.c.
You want to fix the typo first before commit the big change. But the
change is so big that you can't just fix the typo in worktree foo.c
and then use "add -p" to pick the typo fix first.

So you can

git stash
fix typo and commit
git stash apply and resolve the conflict

With 'add --edit=patch', you can
git add --edit=patch foo.c

an editor is fired up with the foo.c in index and worktree open side
by side. And you can fix the typo in both index and worktree file.
When exiting the editor, the index file is modified, and no conflict
to resolve.

Re: [PATCH] git-add: introduce --edit (to edit the diff vs. the index)

[Johannes Schindelin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1906 bytes --]

Hi,

On Fri, 10 Apr 2009, Ping Yin wrote:

> On Thu, Apr 9, 2009 at 10:00 AM, Ping Yin <pkufranky@gmail.com> wrote:
> > On Thu, Apr 9, 2009 at 5:30 AM, Johannes Schindelin
> > <johannes.schindelin@gmx.de> wrote:
> >> With "git add -e [<files>]", Git will fire up an editor with the current
> >> diff relative to the index (i.e. what you would get with "git diff
> >> [<files>]").
> >>
> >> Now you can edit the patch as much as you like, including adding/removing
> >> lines, editing the text, whatever.  Make sure, though, that the first
> >> character of the hunk lines is still a space, a plus or a minus.
> >>
> >> After you closed the editor, Git will adjust the line counts of the hunks
> >> if necessary, thanks to the --recount option of apply, and commit the
> >> patch.  Except if you deleted everything, in which case nothing happens
> >> (for obvious reasons).
> >>
> >> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> >> ---
> >>
> >>        I actually promised myself not to resend this patch, but a
> >>        certain guy who has a hat now asked for it.
> >>
> >
> > I am that guy :-). Thanks. Sometimes "add -e" is more useful than "add
> > -p" since i can see the full context when editing the patch. However,
> > sometimes the ability to edit the index directly is even more useful.
> > For example, if it's a big change (or rewritten), it is hard to edit
> > the patch instead of the index (in diff mode with the worktree file
> > side by side). I even encounter a case that i can't beat the patch
> > into a shape i want when using 'add -p'  ( it will fail to apply)
> >
> 
> How about this?
>  'add --edit=patch' to edit the patch and "add --edit=index" to edit the index

As others have mentioned, there _is_ a reason we have a working directory.  
Please understand this as a "I do not like the idea of editing the index 
directly at all".

Ciao,
Dscho

Re: [PATCH] git-add: introduce --edit (to edit the diff vs. the index)

[Sverre Rabbelier]

Heya,

On Fri, Apr 10, 2009 at 20:10, Johannes Schindelin
<Johannes.Schindelin@gmx.de> > As others have mentioned, there _is_ a
reason we have a working directory.
> Please understand this as a "I do not like the idea of editing the index
> directly at all".

I do not intend use it as a replacement of the working directory, but
mostly as a way to split up patches easier. Mainly to _remove_ lines
that I staged that I would like to be in a different patch in the
series (possibly after doing 'git reset --soft'). I always go through
my a longer patch series multiple times until I am satisfied with the
result, this addition would make that a lot easier.

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH] git-add: introduce --edit (to edit the diff vs. the index)

[Johannes Schindelin]

Hi,

On Fri, 10 Apr 2009, Sverre Rabbelier wrote:

> On Fri, Apr 10, 2009 at 20:10, Johannes Schindelin 
> <Johannes.Schindelin@gmx.de> > As others have mentioned, there _is_ a 
> reason we have a working directory.
> > Please understand this as a "I do not like the idea of editing the 
> > index directly at all".
> 
> I do not intend use it as a replacement of the working directory, but 
> mostly as a way to split up patches easier. Mainly to _remove_ lines 
> that I staged that I would like to be in a different patch in the series 
> (possibly after doing 'git reset --soft'). I always go through my a 
> longer patch series multiple times until I am satisfied with the result, 
> this addition would make that a lot easier.

Would not a much saner way be

	$ git reset HEAD^
	$ git add -p
	$ git stash save --keep-index
	# test it
	$ git commit
	$ git stash apply
	# test again
	$ git commit

Hmm?

BTW I do not like the "longness" of "git stash save --keep-index"; of 
course I could install an alias for that, but it feels wrong to have only 
a cumbersome to call something that should be part of a _lot_ of 
workflows.

Ciao,
Dscho

Re: [PATCH] git-add: introduce --edit (to edit the diff vs. the index)

[Sverre Rabbelier]

Heya,

On Mon, Apr 13, 2009 at 18:19, Johannes Schindelin
<Johannes.Schindelin@gmx.de> > Would not a much saner way be
>
>        $ git reset HEAD^
>        $ git add -p
>        $ git stash save --keep-index
>        # test it
>        $ git commit
>        $ git stash apply
>        # test again
>        $ git commit

That's what I do already, but now imagine that at the # test it step I
notice that I staged too much, or need to tweak some value only in
this commit (e.g., change test_expect_success to test_expect_failure
because this commit is the test that indicates the failure, and the
next one will fix it and flip from expect_failure to expect_success).
At the moment I have to make the change in my working directory (which
potentially involved deleting a lot of hunks), stage that and _then_
make my commit. With 'git add -e' I can make the fix and happily
continue.

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH] git-add: introduce --edit (to edit the diff vs. the index)

[Ping Yin]

On Sat, Apr 11, 2009 at 2:10 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Fri, 10 Apr 2009, Ping Yin wrote:
>
>> On Thu, Apr 9, 2009 at 10:00 AM, Ping Yin <pkufranky@gmail.com> wrote:
>> > On Thu, Apr 9, 2009 at 5:30 AM, Johannes Schindelin
>> > <johannes.schindelin@gmx.de> wrote:
>> >> With "git add -e [<files>]", Git will fire up an editor with the current
>> >> diff relative to the index (i.e. what you would get with "git diff
>> >> [<files>]").
>> >>
>> >> Now you can edit the patch as much as you like, including adding/removing
>> >> lines, editing the text, whatever.  Make sure, though, that the first
>> >> character of the hunk lines is still a space, a plus or a minus.
>> >>
>> >> After you closed the editor, Git will adjust the line counts of the hunks
>> >> if necessary, thanks to the --recount option of apply, and commit the
>> >> patch.  Except if you deleted everything, in which case nothing happens
>> >> (for obvious reasons).
>> >>
>> >> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>> >> ---
>> >>
>> >>        I actually promised myself not to resend this patch, but a
>> >>        certain guy who has a hat now asked for it.
>> >>
>> >
>> > I am that guy :-). Thanks. Sometimes "add -e" is more useful than "add
>> > -p" since i can see the full context when editing the patch. However,
>> > sometimes the ability to edit the index directly is even more useful.
>> > For example, if it's a big change (or rewritten), it is hard to edit
>> > the patch instead of the index (in diff mode with the worktree file
>> > side by side). I even encounter a case that i can't beat the patch
>> > into a shape i want when using 'add -p'  ( it will fail to apply)
>> >
>>
>> How about this?
>>  'add --edit=patch' to edit the patch and "add --edit=index" to edit the index
>
> As others have mentioned, there _is_ a reason we have a working directory.
> Please understand this as a "I do not like the idea of editing the index
> directly at all".
>

When doing "add --edit" to edit the patch, do you work on the working
directory? I think they are just different ways to change the index?

Re: [PATCH] git-add: introduce --edit (to edit the diff vs. the index)

[Johannes Schindelin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2420 bytes --]

Hi,

On Sat, 11 Apr 2009, Ping Yin wrote:

> On Sat, Apr 11, 2009 at 2:10 AM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> > Hi,
> >
> > On Fri, 10 Apr 2009, Ping Yin wrote:
> >
> >> On Thu, Apr 9, 2009 at 10:00 AM, Ping Yin <pkufranky@gmail.com> wrote:
> >> > On Thu, Apr 9, 2009 at 5:30 AM, Johannes Schindelin
> >> > <johannes.schindelin@gmx.de> wrote:
> >> >> With "git add -e [<files>]", Git will fire up an editor with the 
> >> >> current diff relative to the index (i.e. what you would get with 
> >> >> "git diff [<files>]").
> >> >>
> >> >> Now you can edit the patch as much as you like, including 
> >> >> adding/removing lines, editing the text, whatever.  Make sure, 
> >> >> though, that the first character of the hunk lines is still a 
> >> >> space, a plus or a minus.
> >> >>
> >> >> After you closed the editor, Git will adjust the line counts of 
> >> >> the hunks if necessary, thanks to the --recount option of apply, 
> >> >> and commit the patch.  Except if you deleted everything, in which 
> >> >> case nothing happens (for obvious reasons).
> >> >>
> >> >> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 
> >> >> ---
> >> >>
> >> >>     I actually promised myself not to resend this patch, but a   
> >> >>   certain guy who has a hat now asked for it.
> >> >>
> >> >
> >> > I am that guy :-). Thanks. Sometimes "add -e" is more useful than 
> >> > "add -p" since i can see the full context when editing the patch. 
> >> > However, sometimes the ability to edit the index directly is even 
> >> > more useful. For example, if it's a big change (or rewritten), it 
> >> > is hard to edit the patch instead of the index (in diff mode with 
> >> > the worktree file side by side). I even encounter a case that i 
> >> > can't beat the patch into a shape i want when using 'add -p'  ( it 
> >> > will fail to apply)
> >> >
> >>
> >> How about this?
> >>  'add --edit=patch' to edit the patch and "add --edit=index" to edit the index
> >
> > As others have mentioned, there _is_ a reason we have a working 
> > directory. Please understand this as a "I do not like the idea of 
> > editing the index directly at all".
> 
> When doing "add --edit" to edit the patch, do you work on the working 
> directory? I think they are just different ways to change the index?

Of course I do "git stash save --keep-index" after staging those changes.

Ciao,
Dscho