git-commit: select which files to commit while editing the commit message

git-commit: select which files to commit while editing the commit message

[Pazu]

Junio's reworking of the git-commit documentation, and the ensuing discussion
about what is commited, and how do you select that, made me remember how much I
liked SVK -- particularly, how much I liked to remove files from the commit
message template, and have them removed from the commit.

At first, I thought "great opportunity to contribute my first patch to git",
until I realized that git-commit is written in bash, and my brain refuses to
understand that. Yep, I'm that bad. So I'm writing this, and maybe someone
well-versed in bash will find this a good idea and code it :-)

For those not familiar with SVK, if you remove the files mentioned in the commit
template (that "here are the files that you're about to commit" part), SVK won't
commit them. For example, if I modify a couple of files in git, and execute 'git
commit -a', an editor will popup showing something like this:

# Please enter the commit message for your changes.
# (Comment lines starting with '#' will not be included)
# On branch refs/heads/next
# Updated but not checked in:
#   (will commit)
#
#	modified:   perl/Makefile
#	modified:   var.c

Here's where the magic would happen. Removing the line "modified: var.c" would
remove var.c from this commit. Of course, the template message should be
modified to tell the user he can do that.

So, what do you think about this?

-- Pazu

Re: git-commit: select which files to commit while editing the commit message

[Junio C Hamano]

Pazu <pazu@pazu.com.br> writes:

> # Please enter the commit message for your changes.
> # (Comment lines starting with '#' will not be included)
> # On branch refs/heads/next
> # Updated but not checked in:
> #   (will commit)
> #
> #	modified:   perl/Makefile
> #	modified:   var.c
>
> Here's where the magic would happen. Removing the line "modified: var.c" would
> remove var.c from this commit. Of course, the template message should be
> modified to tell the user he can do that.
>
> So, what do you think about this?

Personally, I would refuse to use such a modified git, because
often the first thing I would do in the commit log buffer is
check the listed files and remove the '# ...' lines while
typing.  I do not want that to affect the set of changes I
staged in any way.

But maybe that is just me.

Re: git-commit: select which files to commit while editing the commit message

[Jakub Narebski]

Junio C Hamano wrote:

> Pazu <pazu@pazu.com.br> writes:
> 
>> # Please enter the commit message for your changes.
>> # (Comment lines starting with '#' will not be included)
>> # On branch refs/heads/next
>> # Updated but not checked in:
>> #   (will commit)
>> #
>> #    modified:   perl/Makefile
>> #    modified:   var.c
>>
>> Here's where the magic would happen. Removing the line "modified: var.c" would
>> remove var.c from this commit. Of course, the template message should be
>> modified to tell the user he can do that.
>>
>> So, what do you think about this?
> 
> Personally, I would refuse to use such a modified git, because
> often the first thing I would do in the commit log buffer is
> check the listed files and remove the '# ...' lines while
> typing.  I do not want that to affect the set of changes I
> staged in any way.
> 
> But maybe that is just me.

I was to raise the same objection. 

But this can be solved by using magic _only_ if the template with exception
of "modified:" lines matches, and if there is at least one file
in "modified:" section.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: git-commit: select which files to commit while editing the commit message

[Luben Tuikov]

--- Jakub Narebski <jnareb@gmail.com> wrote:
> Junio C Hamano wrote:
> > Pazu <pazu@pazu.com.br> writes:
> > 
> >> # Please enter the commit message for your changes.
> >> # (Comment lines starting with '#' will not be included)
> >> # On branch refs/heads/next
> >> # Updated but not checked in:
> >> #   (will commit)
> >> #
> >> #    modified:   perl/Makefile
> >> #    modified:   var.c
> >>
> >> Here's where the magic would happen. Removing the line "modified: var.c" would
> >> remove var.c from this commit. Of course, the template message should be
> >> modified to tell the user he can do that.
> >>
> >> So, what do you think about this?
> > 
> > Personally, I would refuse to use such a modified git, because
> > often the first thing I would do in the commit log buffer is
> > check the listed files and remove the '# ...' lines while
> > typing.  I do not want that to affect the set of changes I
> > staged in any way.
> > 
> > But maybe that is just me.
> 
> I was to raise the same objection. 
> 
> But this can be solved by using magic _only_ if the template with exception
> of "modified:" lines matches, and if there is at least one file
> in "modified:" section.

I raise the same objection as Junio.

This is how perforce does it*, and while it is useful, git is NOT perforce,
and I agree with Junio and Jakub.

If you want to commit only few files, update the index for only
the ones you want to commit.  If you did update the index for all
of them, "git-read-tree -m -i HEAD" is your friend.

    Luben

* The reason being is that there is no "index-cache" and the commit
message needs to be scanned to determine which of the edited files
you actually intend to commit to the server at this time.

Re: git-commit: select which files to commit while editing the commit message

[Josef Weidendorfer]

On Friday 08 December 2006 21:45, Luben Tuikov wrote:
> This is how perforce does it*, and while it is useful, git is NOT perforce,
> and I agree with Junio and Jakub.

However, the idea itself is not bad. AFAIK, cogito does it this way.
It could be done as separate command, e.g. "git add --interactive",
and would only update the index.

Re: git-commit: select which files to commit while editing the commit message

[Seth Falcon]

Josef Weidendorfer <Josef.Weidendorfer@gmx.de> writes:

> On Friday 08 December 2006 21:45, Luben Tuikov wrote:
>> This is how perforce does it*, and while it is useful, git is NOT perforce,
>> and I agree with Junio and Jakub.
>
> However, the idea itself is not bad. AFAIK, cogito does it this way.
> It could be done as separate command, e.g. "git add --interactive",
> and would only update the index.

I think such a feature could be quite useful and it would seem to be
an easy thing to provide in an optional or configurable fashion so
that those that don't like it could avoid it.  

Spelling out a bunch of files spread around your tree for update-index
can be annoying.  Some way of marking a list seems natural.  Maybe
that is a separate issue.

+ seth

Re: git-commit: select which files to commit while editing the commit message

[Jakub Narebski]

Seth Falcon wrote:

> Josef Weidendorfer <Josef.Weidendorfer@gmx.de> writes:
> 
>> On Friday 08 December 2006 21:45, Luben Tuikov wrote:
>>> This is how perforce does it*, and while it is useful, git is NOT perforce,
>>> and I agree with Junio and Jakub.
>>
>> However, the idea itself is not bad. AFAIK, cogito does it this way.
>> It could be done as separate command, e.g. "git add --interactive",
>> and would only update the index.
> 
> I think such a feature could be quite useful and it would seem to be
> an easy thing to provide in an optional or configurable fashion so
> that those that don't like it could avoid it.  
> 
> Spelling out a bunch of files spread around your tree for update-index
> can be annoying.  Some way of marking a list seems natural.  Maybe
> that is a separate issue.

Perhaps git-commit should also accept --exclude=<pattern> option?
Would that help?

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: git-commit: select which files to commit while editing the commit message

[Seth Falcon]

Jakub Narebski <jnareb@gmail.com> writes:

> Seth Falcon wrote:
>> Spelling out a bunch of files spread around your tree for update-index
>> can be annoying.  Some way of marking a list seems natural.  Maybe
>> that is a separate issue.
>
> Perhaps git-commit should also accept --exclude=<pattern> option?
> Would that help?

I don't think I understand what an --exclude=<pattern> option would
do, but I'm pretty sure it doesn't help the use case I'm thinking of:

   Editing away, you've made changes in 8 files.

   Reviewing diff, you want to commit 6 of those and continue working
   on the other two.

   It seems that there could be a less manual way than 
   git update-index f1 f2 ... f6


Hmm, maybe I could do:

   git diff --name-only > changed
   ## edit changed
   cat changed|xargs git update-index

I suppose this could be wrapped in a simple way to bring up an editor.

Re: git-commit: select which files to commit while editing the commit message

[Junio C Hamano]

Seth Falcon <sethfalcon@gmail.com> writes:

> I don't think I understand what an --exclude=<pattern> option would
> do, but I'm pretty sure it doesn't help the use case I'm thinking of:
>
>    Editing away, you've made changes in 8 files.
>
>    Reviewing diff, you want to commit 6 of those and continue working
>    on the other two.
>
>    It seems that there could be a less manual way than 
>    git update-index f1 f2 ... f6
>
>
> Hmm, maybe I could do:
>
>    git diff --name-only > changed
>    ## edit changed
>    cat changed|xargs git update-index
>
> I suppose this could be wrapped in a simple way to bring up an editor.

Note that output of cat piped to anything is almost always a bad
programming ;-)

Maybe the "git add --interactive" would give you a transcript
like this:

        $ git add --interactive '*.c' '*.h'
	showing list of modified files...
         1) bozbar.c 2) filfre.c 3) frotz.h 4) nitfol.c 5) rezrov.h
         6) xyzzy.c  7) yomin.h  8) z.c
        choice> 2 3 5 6 7
	showing list of modified files...
         1) bozbar.c 2* filfre.c 3* frotz.h 4) nitfol.c 5* rezrov.h
         6* xyzzy.c  7* yomin.h  8) z.c
        choice> 4+
	showing "git diff -- nitfol.c"...
        diff --git a/nitfol.c b/nitfol.c
        @@ -22,x +22,y @@
         {
        -	int i;
        +	long i;
        +	i = 314;
	 
        stage this hunk [Yes/No/All/Done]? y
        ...
 
The latter half is to come up with a subset of diff and then run
"git-apply --cached" to update only the index with the chosen
hunks.

Re: git-commit: select which files to commit while editing the commit message

[Sean]

On Fri, 08 Dec 2006 11:18:16 -0800
Junio C Hamano <junkio@cox.net> wrote:

> Personally, I would refuse to use such a modified git, because
> often the first thing I would do in the commit log buffer is
> check the listed files and remove the '# ...' lines while
> typing.  I do not want that to affect the set of changes I
> staged in any way.

Your usage should not affected at all by the addition of this
feature.  One of the comment lines could be magic, in that if
it is missing the feature is disabled.  Something like:

#  *** Editable Commit List ***

Or some such at the top of the list, where if it is missing after
commit message editing, the post processing of the file list
will be completely disabled.  Another alternative would be to
just enable the feature with an  -A  or some other commit command
line option, so that those uninterested in such a feature don't
have to even see it.

It would be interesting to hear from Pasky on how this feature
has worked out in practice for Cogito users and whether he thinks
it would be a good addition to Git.

Re: git-commit: select which files to commit while editing the commit message

[Josef Weidendorfer]

On Saturday 09 December 2006 01:59, Junio C Hamano wrote:
> Maybe the "git add --interactive" would give you a transcript
> like this:
> 
>         $ git add --interactive '*.c' '*.h'
> 	showing list of modified files...
>          1) bozbar.c 2) filfre.c 3) frotz.h 4) nitfol.c 5) rezrov.h
>          6) xyzzy.c  7) yomin.h  8) z.c
>         choice> 2 3 5 6 7
> 	showing list of modified files...
>          1) bozbar.c 2* filfre.c 3* frotz.h 4) nitfol.c 5* rezrov.h
>          6* xyzzy.c  7* yomin.h  8) z.c
>         choice> 4+
> 	showing "git diff -- nitfol.c"...
>         diff --git a/nitfol.c b/nitfol.c
>         @@ -22,x +22,y @@
>          {
>         -	int i;
>         +	long i;
>         +	i = 314;
> 	 
>         stage this hunk [Yes/No/All/Done]? y

Wow.
Something like this really would be good.
And please a way to "unstage" accidently added files/hunks, too ;-)

I even miss such a feature in stgit ("specify hunks to break up one
patch into two").

Re: git-commit: select which files to commit while editing the commit message

[Junio C Hamano]

Josef Weidendorfer <Josef.Weidendorfer@gmx.de> writes:

> On Saturday 09 December 2006 01:59, Junio C Hamano wrote:
>> Maybe the "git add --interactive" would give you a transcript
>> like this:
> ...
> Something like this really would be good.

I think I already had the code in jit --- I need to dig it out.

But the above is trivial for anybody who knows how to use
git-diff-index and "git-apply --cached", so...

[PATCH] git-add --interactive (wip)

[Junio C Hamano]

A script to be driven when the user says "git add --interactive"
is introduced.

When it is run, first it runs its internal 'status' command to
show the current status, and then goes into its internactive
command loop.

The command loop shows the list of subcommands available, and
gives a prompt "What now> ".  In general, when the prompt ends
with a single '>', you can pick only one of the choices given
and type return, like this:

    *** Commands ***
      1: status       2: update       3: revert       4: add untracked
      5: patch        6: diff         7: quit         8: help
    What now> 1

You also could say "s" or "sta" or "status" above as long as the
choice is unique.

The main command loop has 6 subcommands (plus help and quit).

 * 'status' shows the change between HEAD and index (i.e. what
   will be committed if you say "git commit"), and between index
   and working tree files (i.e. what you could stage further
   before "git commit" using "git-add") for each path.  A sample
   output looks like this:

              staged     unstaged path
     1:       binary      nothing foo.png
     2:     +403/-35        +1/-1 git-add--interactive.perl

   It shows that foo.png has differences from HEAD (but that is
   binary so line count cannot be shown) and there is no
   difference between indexed copy and the working tree
   version (if the working tree version were also different,
   'binary' would have been shown in place of 'nothing').  The
   other file, git-add--interactive.perl, has 403 lines added
   and 35 lines deleted if you commit what is in the index, but
   working tree file has further modifications (one addition and
   one deletion).

 * 'update' shows the status information and gives prompt
   "Update>>".  When the prompt ends with double '>>', you can
   make more than one selection, concatenated with whitespace or
   comma.  Also you can say ranges.  E.g. "2-5 7,9" to choose
   2,3,4,5,7,9 from the list.  You can say '*' to choose
   everything.

   What you chose are then highlighted with '*', like this:

              staged     unstaged path
     1:       binary      nothing foo.png
   * 2:     +403/-35        +1/-1 git-add--interactive.perl

   To remove selection, prefix the input with - like this:

        Update>> -2

   After making the selection, answer with an empty line to
   stage the contents of working tree files for selected paths
   in the index.

 * 'revert' has a very similar UI to 'update', and the staged
   information for selected paths are reverted to that of the
   HEAD version.  Reverting new paths makes them untracked.

 * 'add untracked' has a very similar UI to 'update' and
   'revert', and lets you add untracked paths to the index.

 * 'patch' lets you choose one path out of 'status' like
   selection.  After choosing the path, it presents diff between
   the index and the working tree file and asks you if you want
   to stage the change of each hunk.  You can say:

        y - add the change from that hunk to index
        n - do not add the change from that hunk to index
        a - add the change from that hunk and all the rest to index
        d - do not the change from that hunk nor any of the rest to index
        j - do not decide on this hunk now, and view the next
            undecided hunk
        J - do not decide on this hunk now, and view the next hunk
        k - do not decide on this hunk now, and view the previous
            undecided hunk
        K - do not decide on this hunk now, and view the previous hunk

   After deciding the fate for all hunks, if there is any hunk
   that was chosen, the index is updated with the selected hunks.

 * 'diff' lets you review what will be committed (i.e. between
   HEAD and index).

This is still rough, but does everything except a few things I
think are needed.

 * 'patch' should be able to allow splitting a hunk into
   multiple hunks.

 * 'patch' does not adjust the line offsets @@ -k,l +m,n @@
   in the hunk header.  This does not have major problem in
   practice, but it _should_ do the adjustment.

 * It does not have any explicit support for a merge in
   progress; it may not work at all.

As is any of my initial WIP, the UI of this presents what it
does in a horribly crappy way.  Improvement patches are
appreciated, but wishlist without code are not really...

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 .gitignore                |    1 +
 Makefile                  |    1 +
 builtin-add.c             |   15 ++
 git-add--interactive.perl |  534 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 551 insertions(+), 0 deletions(-)

diff --git a/.gitignore b/.gitignore
index 7f2cd55..edf93a6 100644
--- a/.gitignore
+++ b/.gitignore
@@ -2,6 +2,7 @@ GIT-CFLAGS
 GIT-VERSION-FILE
 git
 git-add
+git-add--interactive
 git-am
 git-annotate
 git-apply
diff --git a/Makefile b/Makefile
index a1861de..cdd4bf0 100644
--- a/Makefile
+++ b/Makefile
@@ -174,6 +174,7 @@ SCRIPT_SH = \
 	git-lost-found.sh git-quiltimport.sh
 
 SCRIPT_PERL = \
+	git-add--interactive.perl \
 	git-archimport.perl git-cvsimport.perl git-relink.perl \
 	git-rerere.perl \
 	git-cvsserver.perl \
diff --git a/builtin-add.c b/builtin-add.c
index febb75e..37a236e 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -8,6 +8,7 @@
 #include "cache.h"
 #include "builtin.h"
 #include "dir.h"
+#include "exec_cmd.h"
 #include "cache-tree.h"
 
 static const char builtin_add_usage[] =
@@ -89,6 +90,20 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 	int verbose = 0, show_only = 0;
 	const char **pathspec;
 	struct dir_struct dir;
+	int add_interactive = 0;
+
+	for (i = 1; i < argc; i++) {
+		if (!strcmp("--interactive", argv[i]))
+			add_interactive++;
+	}
+	if (add_interactive) {
+		const char *args[] = { "add--interactive", NULL };
+
+		if (add_interactive != 1 || argc != 2)
+			die("add --interactive does not take any parameters");
+		execv_git_cmd(args);
+		exit(1);
+	}
 
 	git_config(git_default_config);
 
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
new file mode 100755
index 0000000..7b42308
--- /dev/null
+++ b/git-add--interactive.perl
@@ -0,0 +1,534 @@
+#!/usr/bin/perl -w
+
+use strict;
+
+sub run_cmd_pipe {
+	my $fh = undef;
+	open($fh, '-|', @_) or die;
+	return <$fh>;
+}
+
+my ($GIT_DIR) = run_cmd_pipe(qw(git rev-parse --git-dir));
+
+if (!defined $GIT_DIR) {
+	exit(1); # rev-parse would have already said "not a git repo"
+}
+chomp($GIT_DIR);
+
+sub refresh {
+	system('git update-index --refresh 2>/dev/null');
+}
+
+sub list_untracked {
+	map {
+		chomp $_;
+		$_;
+	}
+	run_cmd_pipe(qw(git ls-files --others
+			--exclude-per-directory=.gitignore),
+		     "--exclude-from=$GIT_DIR/info/exclude",
+		     '--', @_);
+}
+
+my $status_fmt = '%12s %12s %s';
+my $status_head = sprintf($status_fmt, 'staged', 'unstaged', 'path');
+
+# Returns list of five tuples:
+# 0: print message
+# 1: pathname
+# 2: add/del between HEAD and index
+# 3: add/del between index and working tree
+# 4: binary
+sub list_modified {
+	my ($only) = @_;
+	my (%data, @return);
+	my ($add, $del, $adddel, $file);
+
+	if (!$only || ($only eq 'index-only')) {
+		for (run_cmd_pipe(qw(git diff-index --cached
+				     --numstat --summary HEAD))) {
+			if (($add, $del, $file) =
+			    /^([-\d]+)	([-\d]+)	(.*)/) {
+				my ($change, $bin);
+				if ($add eq '-' && $del eq '-') {
+					$change = 'binary';
+					$bin = 1;
+				}
+				else {
+					$change = "+$add/-$del";
+				}
+				$data{$file} =
+				    [$change, 'nothing', undef, undef, $bin];
+			}
+			elsif (($adddel, $file) =
+			       /^ (create|delete) mode [0-7]+ (.*)$/) {
+				$data{$file}[2] = $adddel;
+			}
+		}
+	}
+
+	if (!$only || ($only eq 'file-only')) {
+		for (run_cmd_pipe(qw(git diff-files --numstat --summary))) {
+			if (($add, $del, $file) =
+			    /^([-\d]+)	([-\d]+)	(.*)/) {
+				if (!exists $data{$file}) {
+					$data{$file} =
+					    ['unchanged', undef, undef, undef];
+				}
+				my ($change, $bin);
+				if ($add eq '-' && $del eq '-') {
+					$change = 'binary';
+					$bin = 1;
+				}
+				else {
+					$change = "+$add/-$del";
+				}
+				$data{$file}[1] = $change;
+				if ($bin) {
+					$data{$file}[4] = 1;
+				}
+			}
+			elsif (($adddel, $file) =
+			       /^ (create|delete) mode [0-7]+ (.*)$/) {
+				$data{$file}[3] = $adddel;
+			}
+		}
+	}
+
+	for (sort keys %data) {
+		my $it = $data{$_};
+		push @return, [(sprintf $status_fmt, @{$it}[0,1], $_), $_,
+			       @{$it}[2,3,4]];
+	}
+	return @return;
+}
+
+
+sub find_unique {
+	my ($string, @stuff) = @_;
+	my $found = undef;
+	for (my $i = 0; $i < @stuff; $i++) {
+		my $it = $stuff[$i];
+		my $hit = undef;
+		if (ref $it) { $it = $it->[0]; }
+		eval {
+			if ($it =~ /^$string/) {
+				$hit = 1;
+			};
+		};
+		if (defined $hit && defined $found) {
+			return undef;
+		}
+		if ($hit) {
+			$found = $i + 1;
+		}
+	}
+	return $found;
+}
+
+sub list_and_choose {
+	my ($opts, @stuff) = @_;
+	my (@chosen, @return);
+	my $i;
+
+      TOPLOOP:
+	while (1) {
+		my $last_lf = 0;
+
+		if ($opts->{HEADER}) {
+			if (!$opts->{LIST_FLAT}) {
+				print "     ";
+			}
+			print "$opts->{HEADER}\n";
+		}
+		for ($i = 0; $i < @stuff; $i++) {
+			my $chosen = $chosen[$i] ? '*' : ' ';
+			my $print = $stuff[$i];
+			if (ref $print) {
+				$print = $print->[0];
+			}
+			printf("%s%2d: %s", $chosen, $i+1, $print);
+			if (($opts->{LIST_FLAT}) &&
+			    (($i + 1) % ($opts->{LIST_FLAT}))) {
+				print "\t";
+				$last_lf = 0;
+			}
+			else {
+				print "\n";
+				$last_lf = 1;
+			}
+		}
+		if (!$last_lf) {
+			print "\n";
+		}
+
+		return if ($opts->{LIST_ONLY});
+
+		print $opts->{PROMPT};
+		if ($opts->{SINGLETON}) {
+			print "> ";
+		}
+		else {
+			print ">> ";
+		}
+		my $line = <STDIN>;
+		last if (!$line);
+		chomp $line;
+		my $donesomething = 0;
+		for my $choice (split(/[\s,]+/, $line)) {
+			my $choose = 1;
+			my ($bottom, $top);
+
+			# Input that begins with '-'; unchoose
+			if ($choice =~ s/^-//) {
+				$choose = 0;
+			}
+			# A range can be specified like 5-7
+			if ($choice =~ /^(\d+)-(\d+)$/) {
+				($bottom, $top) = ($1, $2);
+			}
+			elsif ($choice =~ /^\d+$/) {
+				$bottom = $top = $choice;
+			}
+			elsif ($choice eq '*') {
+				$bottom = 1;
+				$top = 1 + @stuff;
+			}
+			else {
+				$bottom = $top = find_unique($choice, @stuff);
+				if (!defined $bottom) {
+					print "Huh ($choice)?\n";
+					next TOPLOOP;
+				}
+			}
+			if ($opts->{SINGLETON} && $bottom != $top) {
+				print "Huh ($choice)?\n";
+				next TOPLOOP;
+			}
+			for ($i = $bottom-1; $i <= $top-1; $i++) {
+				next if (@stuff <= $i);
+				$chosen[$i] = $choose;
+				$donesomething++;
+			}
+		}
+		last if (!$donesomething || $opts->{IMMEDIATE});
+	}
+	for ($i = 0; $i < @stuff; $i++) {
+		if ($chosen[$i]) {
+			push @return, $stuff[$i];
+		}
+	}
+	return @return;
+}
+
+sub status_cmd {
+	list_and_choose({ LIST_ONLY => 1, HEADER => $status_head },
+			list_modified());
+	print "\n";
+}
+
+sub say_n_paths {
+	my $did = shift @_;
+	my $cnt = scalar @_;
+	print "$did ";
+	if (1 < $cnt) {
+		print "$cnt paths\n";
+	}
+	else {
+		print "one path\n";
+	}
+}
+
+sub update_cmd {
+	my @mods = list_modified('file-only');
+	return if (!@mods);
+
+	my @update = list_and_choose({ PROMPT => 'Update',
+				       HEADER => $status_head, },
+				     @mods);
+	if (@update) {
+		system(qw(git update-index --add --), map { $_->[1] } @update);
+		say_n_paths('updated', @update);
+	}
+	print "\n";
+}
+
+sub revert_cmd {
+	my @update = list_and_choose({ PROMPT => 'Revert',
+				       HEADER => $status_head, },
+				     list_modified());
+	if (@update) {
+		my @lines = run_cmd_pipe(qw(git ls-tree HEAD --),
+					 map { $_->[1] } @update);
+		my $fh;
+		open $fh, '|-', qw(git update-index --index-info)
+		    or die;
+		for (@lines) {
+			print $fh $_;
+		}
+		close($fh);
+		for (@update) {
+			if ($_->[2] && $_->[2] eq 'create') {
+				system(qw(git update-index --force-remove --),
+				       $_->[1]);
+				print "note: $_->[1] is untracked now.\n";
+			}
+		}
+		refresh();
+		say_n_paths('reverted', @update);
+	}
+	print "\n";
+}
+
+sub add_untracked_cmd {
+	my @add = list_and_choose({ PROMPT => 'Add untracked' },
+				  list_untracked());
+	if (@add) {
+		system(qw(git update-index --add --), @add);
+		say_n_paths('added', @add);
+	}
+	print "\n";
+}
+
+sub parse_diff {
+	my ($path) = @_;
+	my @diff = run_cmd_pipe(qw(git diff-files -p --), $path);
+	my (@hunk) = { TEXT => [] };
+
+	for (@diff) {
+		if (/^@@ /) {
+			push @hunk, { TEXT => [] };
+		}
+		push @{$hunk[-1]{TEXT}}, $_;
+	}
+	return @hunk;
+}
+
+sub help_patch_cmd {
+	print <<\EOF ;
+y - stage this hunk
+n - do not stage this hunk
+a - stage this and all the remaining hunks
+d - do not stage this hunk nor any of the remaining hunks
+j - leave this hunk undecided, see next undecided hunk
+J - leave this hunk undecided, see next hunk
+k - leave this hunk undecided, see previous undecided hunk
+K - leave this hunk undecided, see previous hunk
+EOF
+}
+
+sub patch_update_cmd {
+	my @mods = list_modified('file-only');
+	@mods = grep { !($_->[4]) } @mods;
+	return if (!@mods);
+
+	my ($it) = list_and_choose({ PROMPT => 'Patch update',
+				     SINGLETON => 1,
+				     IMMEDIATE => 1,
+				     HEADER => $status_head, },
+				   @mods);
+	return if (!$it);
+
+	my ($ix, $num);
+	my $path = $it->[1];
+	my ($head, @hunk) = parse_diff($path);
+	for (@{$head->{TEXT}}) {
+		print;
+	}
+	$num = scalar @hunk;
+	$ix = 0;
+
+	while (1) {
+		my ($prev, $next, $other, $undecided);
+		$other = '';
+
+		if ($num <= $ix) {
+			$ix = 0;
+		}
+		for (my $i = 0; $i < $ix; $i++) {
+			if (!defined $hunk[$i]{USE}) {
+				$prev = 1;
+				$other .= '/k';
+				last;
+			}
+		}
+		if ($ix) {
+			$other .= '/K';
+		}
+		for (my $i = $ix + 1; $i < $num; $i++) {
+			if (!defined $hunk[$i]{USE}) {
+				$next = 1;
+				$other .= '/j';
+				last;
+			}
+		}
+		if ($ix < $num - 1) {
+			$other .= '/J';
+		}
+		for (my $i = 0; $i < $num; $i++) {
+			if (!defined $hunk[$i]{USE}) {
+				$undecided = 1;
+				last;
+			}
+		}
+		last if (!$undecided);
+
+		for (@{$hunk[$ix]{TEXT}}) {
+			print;
+		}
+		print "Stage this hunk [y/n/a/d$other/?]? ";
+		my $line = <STDIN>;
+		if ($line) {
+			if ($line =~ /^y/i) {
+				$hunk[$ix]{USE} = 1;
+			}
+			elsif ($line =~ /^n/i) {
+				$hunk[$ix]{USE} = 0;
+			}
+			elsif ($line =~ /^a/i) {
+				while ($ix < $num) {
+					if (!defined $hunk[$ix]{USE}) {
+						$hunk[$ix]{USE} = 1;
+					}
+					$ix++;
+				}
+				next;
+			}
+			elsif ($line =~ /^d/i) {
+				while ($ix < $num) {
+					if (!defined $hunk[$ix]{USE}) {
+						$hunk[$ix]{USE} = 0;
+					}
+					$ix++;
+				}
+				next;
+			}
+			elsif ($other =~ /K/ && $line =~ /^K/) {
+				$ix--;
+				next;
+			}
+			elsif ($other =~ /J/ && $line =~ /^J/) {
+				$ix++;
+				next;
+			}
+			elsif ($other =~ /k/ && $line =~ /^k/) {
+				while (1) {
+					$ix--;
+					last if (!$ix ||
+						 !defined $hunk[$ix]{USE});
+				}
+				next;
+			}
+			elsif ($other =~ /j/ && $line =~ /^j/) {
+				while (1) {
+					$ix++;
+					last if ($ix >= $num ||
+						 !defined $hunk[$ix]{USE});
+				}
+				next;
+			}
+			else {
+				help_patch_cmd($other);
+				next;
+			}
+			# soft increment
+			while (1) {
+				$ix++;
+				last if ($ix >= $num ||
+					 !defined $hunk[$ix]{USE});
+			}
+		}
+	}
+
+	my ($o_lno, $n_lno);
+	my @result = ();
+	for (@hunk) {
+		my $text = $_->{TEXT};
+		my ($o_ofs, $o_cnt, $n_ofs, $n_cnt) =
+		    $text->[0] =~ /^@@ -(\d+)(?:,(\d+)) \+(\d+)(?:,(\d+)) @@/;
+		if (!$_->{USE}) {
+			# Adjust offset here.
+			next;
+		}
+		else {
+			for (@$text) {
+				push @result, $_;
+			}
+		}
+	}
+
+	if (@result) {
+		my $fh;
+
+		open $fh, '|-', qw(git apply --cached);
+		for (@{$head->{TEXT}}, @result) {
+			print $fh $_;
+		}
+		close $fh;
+		refresh();
+	}
+
+	print "\n";
+}
+
+sub diff_cmd {
+	my @mods = list_modified('index-only');
+	@mods = grep { !($_->[4]) } @mods;
+	return if (!@mods);
+	my ($it) = list_and_choose({ PROMPT => 'Review diff',
+				     SINGLETON => 1,
+				     IMMEDIATE => 1,
+				     HEADER => $status_head, },
+				   @mods);
+	return if (!$it);
+	system(qw(git diff-index -p --cc --cached HEAD --), $it->[1]);
+}
+
+sub quit_cmd {
+	print "Bye.\n";
+	exit(0);
+}
+
+sub help_cmd {
+	print <<\EOF ;
+status        - show paths with changes
+update        - add working tree state to the staged set of changes
+revert        - revert staged set of changes back to the HEAD version
+patch         - pick hunks and update selectively
+diff	      - view diff between HEAD and index
+add untracked - add contents of untracked files to the staged set of changes
+EOF
+}
+
+sub main_loop {
+	my @cmd = ([ 'status', \&status_cmd, ],
+		   [ 'update', \&update_cmd, ],
+		   [ 'revert', \&revert_cmd, ],
+		   [ 'add untracked', \&add_untracked_cmd, ],
+		   [ 'patch', \&patch_update_cmd, ],
+		   [ 'diff', \&diff_cmd, ],
+		   [ 'quit', \&quit_cmd, ],
+		   [ 'help', \&help_cmd, ],
+	);
+	while (1) {
+		my ($it) = list_and_choose({ PROMPT => 'What now',
+					     SINGLETON => 1,
+					     LIST_FLAT => 4,
+					     HEADER => '*** Commands ***',
+					     IMMEDIATE => 1 }, @cmd);
+		if ($it) {
+			eval {
+				$it->[1]->();
+			};
+			if ($@) {
+				print "$@";
+			}
+		}
+	}
+}
+
+my @z;
+
+refresh();
+status_cmd();
+main_loop();
-- 
1.4.4.2.gdfd96

Re: [PATCH] git-add --interactive (wip)

[Josef Weidendorfer]

On Monday 11 December 2006 10:23, Junio C Hamano wrote:
> A script to be driven when the user says "git add --interactive"
> is introduced.

Cool.
Just allowing to unstage files this way makes me happy, thanks.

I just tried to work with this thing to separate my recent
"branch.*.localmerge" changes to the simple version I posted
afterwards.

I noted that I need to manually change a few dependent hunks,
so this "one hunk at a time" and not being able to edit changes
in a hunk does not fit my workflow.
Just as a sidenote: after deciding to not apply hunks, you
lose them in this WIP, as you will find nothing in "unstaged" mode
afterwards :-(

Despite not having a patch for it right now, it would work out
better for me to open the editor with the patch for one
file, and allow to edit it to *replace* the staged version.

Neverless, thanks for this WIP,

Re: [PATCH] git-add --interactive (wip)

[Junio C Hamano]

Josef Weidendorfer <Josef.Weidendorfer@gmx.de> writes:

> I just tried to work with this thing to separate my recent
> "branch.*.localmerge" changes to the simple version I posted
> afterwards.
>
> I noted that I need to manually change a few dependent hunks,
> so this "one hunk at a time" and not being able to edit changes
> in a hunk does not fit my workflow.

I've updated my "git add --interactive" in 'pu' and it now knows
how to split a hunk into smaller pieces and recounting the diff
offsets before applying (Johannes pointed out that his partial
commit script, entirely written in bash using shell arrays, has
a similar feature.  I guess I should have stolen his code
instead of lifting the logic from my own abandoned porcelain).

Hunk splitting allows you to start with this hunk:

        @@ -9,9 +9,10 @@
         #include "builtin.h"
         #include "dir.h"
         #include "cache-tree.h"
        +#include "exec_cmd.h"

         static const char builtin_add_usage[] =
        -"git-add [-n] [-v] <filepattern>...";
        +"git-add [-n] [-v] <filepattern>... | git-add --interactive";

         static void prune_directory(struct dir_struct *dir, const char **...
         {

and lets you split it into two, and you can pick each individually:

        @@ -9,5 +9,6 @@
         #include "builtin.h"
         #include "dir.h"
         #include "cache-tree.h"
        +#include "exec_cmd.h"

         static const char builtin_add_usage[] =
        @@ -12,6 +13,6 @@

         static const char builtin_add_usage[] =
        -"git-add [-n] [-v] <filepattern>...";
        +"git-add [-n] [-v] <filepattern>... | git-add --interactive";

         static void prune_directory(struct dir_struct *dir, const char **...
         {

The post-context of the first hunk and pre-context of the
second hunk are duplicated to give you better context when
reviewing each of them alone.  If you choose to apply both, they
are merged back again before applied.

Anyway, after I finished doing this, I started thinking...

One of the most beautifully done patch series in git.git history
is this series by Sergey Vlasov:

	http://thread.gmane.org/gmane.comp.version-control.git/9057

The resulting commits are from e8d2a6d4 to 4041483e (inclusive).

What's interesting about this series is that its patches touch
the same area of the code over and over again, refining a bit by
bit as the series progresses.

Sergey might have done it exactly the way the final patches
presented the evolution, but I somehow doubt it [*1*].  I often
have the state that corresponds to [PATCH n/n] in my working
tree after hacking around without making a single commit, and
often would start wanting to commit the state as a clean
multi-step series at that point.  I think this is the same for
everybody.

What I often end up doing is to make a snapshot of that
"proposed final" state, reset to the original, and work toward
the proposed final state:

	# snapshot
	$ git commit -a -m 'commit goal'
        $ git tag -f CG
        $ git reset --hard HEAD^

	# repeat for (i = 1; i <= n; i++)
        $ git diff HEAD CG >P.diff
        $ edit
        # use P.diff as a reference, and edit the working tree
        # files to produce [i/n] state.  I might remove pieces
        # that belong to [j/n] for j > i from  P.diff and run
	# "git apply --index", or hand edit the files directly
	# in the working tree.
	$ make test
        $ git commit -a -m "[PATCH i/n] title..."
        # end repeat

	# when finished the above cycle, this diff should only
	# show the improvements I made during the decomposition.
        $ git diff CG HEAD

For a very simple case where the patch munging in the "repeat"
part of the above workflow only deletes hunks (or sub-hunks),
the patch subcommand of "git add --interactive" with hunk
splitting would work nicely, but in general it would not be
enough.  It certainly is far from usable to produce something
like what Sergey gave us.

To make it easier, one possibility might be to add a subcommand
to "git add --interactive" that lets you edit what is currently
staged in the index by opening a temporary copy in your favorite
editor, and stage the result of your edit in the index.  But I
feel quite uneasy to introduce ways to update the index with
something _wildly_ different from what you ever had in your
working tree as a whole.

I think it is wrong to commit partially, purely from the index,
when you are building a series that has complex changes that
come during the series but go away at the end.  The user should
be able to verify all the steps in the middle in such a complex
series, but it is not easy if you have it only in the index.

You could do

	$ git checkout-index --prefix=testarea/ -f -q -u -a

and run your tests there, but that takes a discipline, and is
cumbersome to do.

So in short, I think per-hunk update-index is a cute hack and
may be useful in a narrow simple cases, but it would not be so
useful in the real life.


> Just as a sidenote: after deciding to not apply hunks, you
> lose them in this WIP, as you will find nothing in "unstaged" mode
> afterwards :-(

I do not understand this part.  You can 'revert' to match the
index to HEAD and run 'patch' to pick what you want again.


[Footnote]

*1* Maybe Sergey indeed did things the way the series was
presented, in which case that would only strengthen my respect
to his ability even more.  The quoted patch series made him one
of three people on the git list whose patches I trust 100%; in
fact, I trust their patches so much that I read them not to find
bugs but to admire and enjoy.

Re: [PATCH] git-add --interactive (wip)

[Johannes Schindelin]

Hi,

On Tue, 12 Dec 2006, Junio C Hamano wrote:

> I've updated my "git add --interactive" in 'pu' and it now knows how to 
> split a hunk into smaller pieces and recounting the diff offsets before 
> applying (Johannes pointed out that his partial commit script, entirely 
> written in bash using shell arrays, has a similar feature.  I guess I 
> should have stolen his code instead of lifting the logic from my own 
> abandoned porcelain).

Well, not completely true. I do not split hunks into subhunks.

Note that you _could_ split hunks at arbitrary lines; if it is a line 
common between the two files, it is easier though.

And you could not have ripped from my script: I make heavy use of bash 
arrays (in fact, I wrote this script partly to learn how to work with bash 
arrays). If I could think of a clean way to spawn processes piped into 
"less -FS" several times from a C program, I would already have rewritten 
it in C.

> So in short, I think per-hunk update-index is a cute hack and may be 
> useful in a narrow simple cases, but it would not be so useful in the 
> real life.

To the contrary! In spite of having written git-hunk-commit to please 
those poor souls coming from the darcs side, I already used it myself 
quite often! I think it is a killer feature.

Ciao,
Dscho

Re: [PATCH] git-add --interactive (wip)

[Josef Weidendorfer]

On Tuesday 12 December 2006 22:51, Junio C Hamano wrote:
> I've updated my "git add --interactive" in 'pu' and it now knows
> how to split a hunk into smaller pieces and recounting the diff
> offsets before applying

Nice.
Yes, this seems to be a missing piece in this hunk-wise staging.

> To make it easier, one possibility might be to add a subcommand
> to "git add --interactive" that lets you edit what is currently
> staged in the index by opening a temporary copy in your favorite
> editor, and stage the result of your edit in the index.

Yes. Sounds interesting.
Another would be to open the editor with the diff of this hunk.

> But I 
> feel quite uneasy to introduce ways to update the index with
> something _wildly_ different from what you ever had in your
> working tree as a whole.

Hmm... yes.
Is this not a general problem with staging, of course to
a lesser degree if you work at file granularity, because
the probability of dependence of changes in multiple files,
which could lead to a wrong commit, is less.

> I think it is wrong to commit partially, purely from the index,
> when you are building a series that has complex changes that
> come during the series but go away at the end.  The user should
> be able to verify all the steps in the middle in such a complex
> series, but it is not easy if you have it only in the index.

What you really want is to test the commit afterwards. If
you did it wrong, you always can do "git reset --mixed HEAD^"
or "git commit --amend".

However, testing a commit with a dirty working tree is not
possible. For this to work, you would want that
"git-checkout --store" can store away a dirty working state when
going to another revision, e.g. store it into a temporary ref
"<current branch>.dirtywork".
You would need a "git-checkout --restore" which would restore
the dirty state of the branch you are switching too.

Then, to check the commit of staged things, it should be enough
to do "git-checkout --store", check the commit, and do a
"git-checkout --restore" afterwards to get the dirty state back.

> You could do
> 
> 	$ git checkout-index --prefix=testarea/ -f -q -u -a
> 
> and run your tests there, but that takes a discipline, and is
> cumbersome to do.

IMHO that is too tricky for the average git user.

> So in short, I think per-hunk update-index is a cute hack and
> may be useful in a narrow simple cases, but it would not be so
> useful in the real life.

No. It currently is starting to get useful. With the ability
to temporarily store away a dirty state of the working directory,
it really could become very good.
 
> > Just as a sidenote: after deciding to not apply hunks, you
> > lose them in this WIP, as you will find nothing in "unstaged" mode
> > afterwards :-(
> 
> I do not understand this part.  You can 'revert' to match the
> index to HEAD and run 'patch' to pick what you want again.

Hmmm...
I lost my changes in the working directory; there was nothing to
pick again any more.
Perhaps I did something wrong.

Re: [PATCH] git-add --interactive (wip)

[Junio C Hamano]

Josef Weidendorfer <Josef.Weidendorfer@gmx.de> writes:

> No. It currently is starting to get useful. With the ability
> to temporarily store away a dirty state of the working directory,
> it really could become very good.

Hmm, a way to easily stash away local changes and restoring
would lead to a system where you can easily stash and unstash
multiple snapshots and switch between them, and such a model
sounds vaguely familiar...

>> > Just as a sidenote: after deciding to not apply hunks, you
>> > lose them in this WIP, as you will find nothing in "unstaged" mode
>> > afterwards :-(
>> 
>> I do not understand this part.  You can 'revert' to match the
>> index to HEAD and run 'patch' to pick what you want again.
>
> I lost my changes in the working directory; there was nothing to
> pick again any more.

That's worrysome.  By design, "git add" should not touch working
tree at all (only read from there), so if you find cases that
violates it that should be fixed; please let me know.

Re: [PATCH] git-add --interactive (wip)

[Andreas Ericsson]

Johannes Schindelin wrote:
> Hi,
> 
> On Tue, 12 Dec 2006, Junio C Hamano wrote:
> 
>> So in short, I think per-hunk update-index is a cute hack and may be 
>> useful in a narrow simple cases, but it would not be so useful in the 
>> real life.
> 
> To the contrary! In spite of having written git-hunk-commit to please 
> those poor souls coming from the darcs side, I already used it myself 
> quite often! I think it is a killer feature.
> 

I've tried it a couple of times as well, and it really is very, very 
nice, especially for young projects where you know you have to make 
loads of changes (often basic and codewise small features).

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se

Re: [PATCH] git-add --interactive (wip)

[Johannes Schindelin]

Hi,

On Tue, 12 Dec 2006, Junio C Hamano wrote:

> Josef Weidendorfer <Josef.Weidendorfer@gmx.de> writes:
> 
> > No. It currently is starting to get useful. With the ability
> > to temporarily store away a dirty state of the working directory,
> > it really could become very good.
> 
> Hmm, a way to easily stash away local changes and restoring
> would lead to a system where you can easily stash and unstash
> multiple snapshots and switch between them, and such a model
> sounds vaguely familiar...

Hmm, what might that be? :-)

A more simple approach than to buy into Python would be to introduce a 
very simple program, which exchanges work directory contents with index 
contents. So,

$ git revolve-stage
$ [test the staged revision]
$ [possibly fix a thing or two]
$ git revolve-stage
$ git commit

Opinions?

Ciao,
Dscho

Re: [PATCH] git-add --interactive (wip)

[Shawn Pearce]

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> On Tue, 12 Dec 2006, Junio C Hamano wrote:
> > Hmm, a way to easily stash away local changes and restoring
> > would lead to a system where you can easily stash and unstash
> > multiple snapshots and switch between them, and such a model
> > sounds vaguely familiar...
> 
> Hmm, what might that be? :-)
> 
> A more simple approach than to buy into Python 

Hmmm... last I heard Git doesn't depend on Python anymore, thanks
largely to you and Alex.  But it does stash and unstash snapshots
of my working directory wicked fast using these things called
branches...  :-)

-- 

Re: [PATCH] git-add --interactive (wip)

[Johannes Schindelin]

Hi,

On Wed, 13 Dec 2006, Shawn Pearce wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > On Tue, 12 Dec 2006, Junio C Hamano wrote:
> > > Hmm, a way to easily stash away local changes and restoring
> > > would lead to a system where you can easily stash and unstash
> > > multiple snapshots and switch between them, and such a model
> > > sounds vaguely familiar...
> > 
> > Hmm, what might that be? :-)
> > 
> > A more simple approach than to buy into Python 
> 
> Hmmm... last I heard Git doesn't depend on Python anymore, thanks
> largely to you and Alex.

I referred to StGit, which is pure Python AFAIK.

> But it does stash and unstash snapshots of my working directory wicked 
> fast using these things called branches...  :-)

The PEBCAK. I cannot hardly type the commands as fast as Git need to stash 
snapshots into branches.

So, git-revolve-stage would help me. Or maybe I create bash aliases

$ alias stash='stash_tree=$(git-write-tree)'
$ alias unstash='git-read-tree $stash_tree'

Ciao,
Dscho

Re: [PATCH] git-add --interactive (wip)

[Josef Weidendorfer]

On Wednesday 13 December 2006 06:34, Junio C Hamano wrote:
> >> > Just as a sidenote: after deciding to not apply hunks, you
> >> > lose them in this WIP, as you will find nothing in "unstaged" mode
> >> > afterwards :-(
> >> 
> >> I do not understand this part.  You can 'revert' to match the
> >> index to HEAD and run 'patch' to pick what you want again.
> >
> > I lost my changes in the working directory; there was nothing to
> > pick again any more.
> 
> That's worrysome.  By design, "git add" should not touch working
> tree at all (only read from there), so if you find cases that
> violates it that should be fixed; please let me know.

I just tried to reproduce the failure with exact the same hunk I used
the first time. I wasn't able to get any wrong result.
So perhaps I did something wrong the first time.

However, if I see something suspect, I will tell you ;-)

Thanks,