[PATCH] git-add (-a|-u) and -n support

[PATCH] git-add (-a|-u) and -n support

[Michael Witten]

Hello,

The git-add command doesn't handle -n when using -u.

I fixed this and added -a for adding ALL files, not
just those below the current directory (just like
git-commit).

The patch is below, but you can also download it from
http://web.mit.edu/mfwitten/git/0001-git-add-now-understands-two- 
kinds-of-update.patch



 From acc846f5243d26a96aaf0bf1c4f04ecc021385a2 Mon Sep 17 00:00:00 2001
From: Michael Witten <mfwitten@mit.edu>
Date: Sun, 14 Oct 2007 06:13:20 -0400
Subject: [PATCH] git-add now understands two kinds of update:

  	-u: update as before
  	-a: update all as in a true 'git commit -a'

Also, -n works correctly now with the above options.

Signed-off-by: Michael Witten <mfwitten@mit.edu>
---
  builtin-add.c |   69 +++++++++++++++++++++++++++++++++++++ 
+-------------------
  1 files changed, 46 insertions(+), 23 deletions(-)

diff --git a/builtin-add.c b/builtin-add.c
index f9a6580..24887c7 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -13,10 +13,11 @@
  #include "commit.h"
  #include "revision.h"

+enum update_type {NONE, ALL, CURRENT_DIRECTORY};
+
  static const char builtin_add_usage[] =
  "git-add [-n] [-v] [-f] [--interactive | -i] [-u] [--refresh] [--]  
<filepattern>...";

-static int take_worktree_changes;
  static const char *excludes_file;

  static void prune_directory(struct dir_struct *dir, const char  
**pathspec, int prefix)
@@ -83,40 +84,57 @@ static void fill_directory(struct dir_struct  
*dir, const char **pathspec,
  static void update_callback(struct diff_queue_struct *q,
  			    struct diff_options *opt, void *cbdata)
  {
-	int i, verbose;
-
-	verbose = *((int *)cbdata);
+	int i;
+	
+	int* options   = (int*)cbdata;
+	int  verbose   = options[0];
+	int  show_only = options[1];
+	
  	for (i = 0; i < q->nr; i++) {
  		struct diff_filepair *p = q->queue[i];
  		const char *path = p->one->path;
-		switch (p->status) {
-		default:
-			die("unexpected diff status %c", p->status);
-		case DIFF_STATUS_UNMERGED:
-		case DIFF_STATUS_MODIFIED:
-		case DIFF_STATUS_TYPE_CHANGED:
-			add_file_to_cache(path, verbose);
-			break;
-		case DIFF_STATUS_DELETED:
-			remove_file_from_cache(path);
-			if (verbose)
-				printf("remove '%s'\n", path);
-			break;
+		
+		switch (p->status) {			
+			case DIFF_STATUS_UNMERGED:
+			case DIFF_STATUS_MODIFIED:
+			case DIFF_STATUS_TYPE_CHANGED:
+				if (show_only)
+					printf("add '%s'\n", path);
+				else
+					add_file_to_cache(path, verbose);
+				break;
+			
+			case DIFF_STATUS_DELETED:
+				if (show_only)
+					remove_file_from_cache(path);
+				if (verbose)
+					printf("remove '%s'\n", path);
+				break;
+			
+			default:
+				die("unexpected diff status %c", p->status);
  		}
  	}
  }

-static void update(int verbose, const char *prefix, const char **files)
+static void update(enum update_type type, int verbose, int show_only,
+                  const char *prefix, const char **files)
  {
  	struct rev_info rev;
+	int callback_options[] = {verbose, show_only};
+	
  	init_revisions(&rev, prefix);
  	setup_revisions(0, NULL, &rev, NULL);
-	rev.prune_data = get_pathspec(prefix, files);
+	
+	rev.prune_data = type == ALL ? NULL : get_pathspec(prefix, files);
+	
  	rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;
  	rev.diffopt.format_callback = update_callback;
-	rev.diffopt.format_callback_data = &verbose;
+	rev.diffopt.format_callback_data = callback_options;
+	
  	if (read_cache() < 0)
  		die("index file corrupt");
+	
  	run_diff_files(&rev, 0);
  }

@@ -158,6 +176,7 @@ int cmd_add(int argc, const char **argv, const  
char *prefix)
  {
  	int i, newfd;
  	int verbose = 0, show_only = 0, ignored_too = 0, refresh_only = 0;
+	enum update_type update_type = NONE;
  	const char **pathspec;
  	struct dir_struct dir;
  	int add_interactive = 0;
@@ -201,8 +220,12 @@ int cmd_add(int argc, const char **argv, const  
char *prefix)
  			verbose = 1;
  			continue;
  		}
+		if (!strcmp(arg, "-a")) {
+			update_type = ALL;
+			continue;
+		}
  		if (!strcmp(arg, "-u")) {
-			take_worktree_changes = 1;
+			update_type = CURRENT_DIRECTORY;
  			continue;
  		}
  		if (!strcmp(arg, "--refresh")) {
@@ -212,8 +235,8 @@ int cmd_add(int argc, const char **argv, const  
char *prefix)
  		usage(builtin_add_usage);
  	}

-	if (take_worktree_changes) {
-		update(verbose, prefix, argv + i);
+	if (update_type) {
+		update(update_type, verbose, show_only, prefix, argv + i);
  		goto finish;
  	}

-- 
1.5.3.4.206.g58ba4-dirty

Re: [PATCH] [BUG FIXED] git-add (-a|-u) and -n support

[Michael Witten]

On 14 Oct 2007, at 6:26:28 AM, Michael Witten wrote:

> Hello,
>
> The git-add command doesn't handle -n when using -u.
>
> I fixed this and added -a for adding ALL files, not
> just those below the current directory (just like
> git-commit).
>
> The patch is below, but you can also download it from
> http://web.mit.edu/mfwitten/git/0001-git-add-now-understands-two- 
> kinds-of-update.patch
>


Unfortunately, I introduced a bug.
The following:

> +				if (show_only)
> +					remove_file_from_cache(path);
> +				if (verbose)
> +					printf("remove '%s'\n", path);

Should be:

> +				if (!show_only)
> +					remove_file_from_cache(path);
> +				if (verbose)
> +					printf("remove '%s'\n", path);


The new patch is listed below:
(http://web.mit.edu/mfwitten/git/0001-git-add-now-understands-two- 
kinds-of-update.patch)


 From 6d7480062b1e1c513441d4bbc17a9a8b5d9b1c8f Mon Sep 17 00:00:00 2001
From: Michael Witten <mfwitten@mit.edu>
Date: Sun, 14 Oct 2007 06:13:20 -0400
Subject: [PATCH] git-add now understands two kinds of update:

  	-u: update as before
  	-a: update all as in a true 'git commit -a'

Also, -n works correctly now with the above options.

Signed-off-by: Michael Witten <mfwitten@mit.edu>
---
  builtin-add.c |   69 +++++++++++++++++++++++++++++++++++++ 
+-------------------
  1 files changed, 46 insertions(+), 23 deletions(-)

diff --git a/builtin-add.c b/builtin-add.c
index f9a6580..f180afe 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -13,10 +13,11 @@
  #include "commit.h"
  #include "revision.h"

+enum update_type {NONE, ALL, CURRENT_DIRECTORY};
+
  static const char builtin_add_usage[] =
  "git-add [-n] [-v] [-f] [--interactive | -i] [-u] [--refresh] [--]  
<filepattern>...";

-static int take_worktree_changes;
  static const char *excludes_file;

  static void prune_directory(struct dir_struct *dir, const char  
**pathspec, int prefix)
@@ -83,40 +84,57 @@ static void fill_directory(struct dir_struct  
*dir, const char **pathspec,
  static void update_callback(struct diff_queue_struct *q,
  			    struct diff_options *opt, void *cbdata)
  {
-	int i, verbose;
-
-	verbose = *((int *)cbdata);
+	int i;
+	
+	int* options   = (int*)cbdata;
+	int  verbose   = options[0];
+	int  show_only = options[1];
+	
  	for (i = 0; i < q->nr; i++) {
  		struct diff_filepair *p = q->queue[i];
  		const char *path = p->one->path;
-		switch (p->status) {
-		default:
-			die("unexpected diff status %c", p->status);
-		case DIFF_STATUS_UNMERGED:
-		case DIFF_STATUS_MODIFIED:
-		case DIFF_STATUS_TYPE_CHANGED:
-			add_file_to_cache(path, verbose);
-			break;
-		case DIFF_STATUS_DELETED:
-			remove_file_from_cache(path);
-			if (verbose)
-				printf("remove '%s'\n", path);
-			break;
+		
+		switch (p->status) {			
+			case DIFF_STATUS_UNMERGED:
+			case DIFF_STATUS_MODIFIED:
+			case DIFF_STATUS_TYPE_CHANGED:
+				if (show_only)
+					printf("add '%s'\n", path);
+				else
+					add_file_to_cache(path, verbose);
+				break;
+			
+			case DIFF_STATUS_DELETED:
+				if (verbose)
+					printf("remove '%s'\n", path);
+				if (!show_only)
+					remove_file_from_cache(path);
+				break;
+			
+			default:
+				die("unexpected diff status %c", p->status);
  		}
  	}
  }

-static void update(int verbose, const char *prefix, const char **files)
+static void update(enum update_type type, int verbose, int show_only,
+                  const char *prefix, const char **files)
  {
  	struct rev_info rev;
+	int callback_options[] = {verbose, show_only};
+	
  	init_revisions(&rev, prefix);
  	setup_revisions(0, NULL, &rev, NULL);
-	rev.prune_data = get_pathspec(prefix, files);
+	
+	rev.prune_data = type == ALL ? NULL : get_pathspec(prefix, files);
+	
  	rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;
  	rev.diffopt.format_callback = update_callback;
-	rev.diffopt.format_callback_data = &verbose;
+	rev.diffopt.format_callback_data = callback_options;
+	
  	if (read_cache() < 0)
  		die("index file corrupt");
+	
  	run_diff_files(&rev, 0);
  }

@@ -158,6 +176,7 @@ int cmd_add(int argc, const char **argv, const  
char *prefix)
  {
  	int i, newfd;
  	int verbose = 0, show_only = 0, ignored_too = 0, refresh_only = 0;
+	enum update_type update_type = NONE;
  	const char **pathspec;
  	struct dir_struct dir;
  	int add_interactive = 0;
@@ -201,8 +220,12 @@ int cmd_add(int argc, const char **argv, const  
char *prefix)
  			verbose = 1;
  			continue;
  		}
+		if (!strcmp(arg, "-a")) {
+			update_type = ALL;
+			continue;
+		}
  		if (!strcmp(arg, "-u")) {
-			take_worktree_changes = 1;
+			update_type = CURRENT_DIRECTORY;
  			continue;
  		}
  		if (!strcmp(arg, "--refresh")) {
@@ -212,8 +235,8 @@ int cmd_add(int argc, const char **argv, const  
char *prefix)
  		usage(builtin_add_usage);
  	}

-	if (take_worktree_changes) {
-		update(verbose, prefix, argv + i);
+	if (update_type) {
+		update(update_type, verbose, show_only, prefix, argv + i);
  		goto finish;
  	}

-- 
1.5.3.4.206.g58ba4-dirty

Re: [PATCH] [BUG FIXED 2] git-add (-a|-u) and -n support

[Michael Witten]

Sorry for the "spam"!

On 14 Oct 2007, at 6:26:28 AM, Michael Witten wrote:

> Hello,
>
> The git-add command doesn't handle -n when using -u.
>
> I fixed this and added -a for adding ALL files, not
> just those below the current directory (just like
> git-commit).
>
> The patch is below, but you can also download it from
> http://web.mit.edu/mfwitten/git/0001-git-add-now-understands-two- 
> kinds-of-update.patch
>


Unfortunately, I left a second bug in place.
I was too excited about submitting a patch,
and too tired to get it right.

The following:

> +				if (verbose)
> +					printf("remove '%s'\n", path);
> +				if (!show_only)
> +					remove_file_from_cache(path);

Should be:

> +				if (show_only || verbose)
> +					printf("remove '%s'\n", path);
> +				if (!show_only)
> +					remove_file_from_cache(path);


The new patch is listed below:
(http://web.mit.edu/mfwitten/git/0001-git-add-now-understands-two- 
kinds-of-update.patch)


 From c3c2f07f3f94aa75d73fce0dfabc3958532f38c4 Mon Sep 17 00:00:00 2001
From: Michael Witten <mfwitten@mit.edu>
Date: Sun, 14 Oct 2007 06:13:20 -0400
Subject: [PATCH] git-add now understands two kinds of update:

  	-u: update as before
  	-a: update all as in a true 'git commit -a'

Also, -n works correctly now with the above options.

Signed-off-by: Michael Witten <mfwitten@mit.edu>
---
  builtin-add.c |   69 +++++++++++++++++++++++++++++++++++++ 
+-------------------
  1 files changed, 46 insertions(+), 23 deletions(-)

diff --git a/builtin-add.c b/builtin-add.c
index f9a6580..3c79a95 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -13,10 +13,11 @@
  #include "commit.h"
  #include "revision.h"

+enum update_type {NONE, ALL, CURRENT_DIRECTORY};
+
  static const char builtin_add_usage[] =
  "git-add [-n] [-v] [-f] [--interactive | -i] [-u] [--refresh] [--]  
<filepattern>...";

-static int take_worktree_changes;
  static const char *excludes_file;

  static void prune_directory(struct dir_struct *dir, const char  
**pathspec, int prefix)
@@ -83,40 +84,57 @@ static void fill_directory(struct dir_struct  
*dir, const char **pathspec,
  static void update_callback(struct diff_queue_struct *q,
  			    struct diff_options *opt, void *cbdata)
  {
-	int i, verbose;
-
-	verbose = *((int *)cbdata);
+	int i;
+	
+	int* options   = (int*)cbdata;
+	int  verbose   = options[0];
+	int  show_only = options[1];
+	
  	for (i = 0; i < q->nr; i++) {
  		struct diff_filepair *p = q->queue[i];
  		const char *path = p->one->path;
-		switch (p->status) {
-		default:
-			die("unexpected diff status %c", p->status);
-		case DIFF_STATUS_UNMERGED:
-		case DIFF_STATUS_MODIFIED:
-		case DIFF_STATUS_TYPE_CHANGED:
-			add_file_to_cache(path, verbose);
-			break;
-		case DIFF_STATUS_DELETED:
-			remove_file_from_cache(path);
-			if (verbose)
-				printf("remove '%s'\n", path);
-			break;
+		
+		switch (p->status) {			
+			case DIFF_STATUS_UNMERGED:
+			case DIFF_STATUS_MODIFIED:
+			case DIFF_STATUS_TYPE_CHANGED:
+				if (show_only)
+					printf("add '%s'\n", path);
+				else
+					add_file_to_cache(path, verbose);
+				break;
+			
+			case DIFF_STATUS_DELETED:
+				if (show_only || verbose)
+					printf("remove '%s'\n", path);
+				if (!show_only)
+					remove_file_from_cache(path);
+				break;
+			
+			default:
+				die("unexpected diff status %c", p->status);
  		}
  	}
  }

-static void update(int verbose, const char *prefix, const char **files)
+static void update(enum update_type type, int verbose, int show_only,
+                  const char *prefix, const char **files)
  {
  	struct rev_info rev;
+	int callback_options[] = {verbose, show_only};
+	
  	init_revisions(&rev, prefix);
  	setup_revisions(0, NULL, &rev, NULL);
-	rev.prune_data = get_pathspec(prefix, files);
+	
+	rev.prune_data = type == ALL ? NULL : get_pathspec(prefix, files);
+	
  	rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;
  	rev.diffopt.format_callback = update_callback;
-	rev.diffopt.format_callback_data = &verbose;
+	rev.diffopt.format_callback_data = callback_options;
+	
  	if (read_cache() < 0)
  		die("index file corrupt");
+	
  	run_diff_files(&rev, 0);
  }

@@ -158,6 +176,7 @@ int cmd_add(int argc, const char **argv, const  
char *prefix)
  {
  	int i, newfd;
  	int verbose = 0, show_only = 0, ignored_too = 0, refresh_only = 0;
+	enum update_type update_type = NONE;
  	const char **pathspec;
  	struct dir_struct dir;
  	int add_interactive = 0;
@@ -201,8 +220,12 @@ int cmd_add(int argc, const char **argv, const  
char *prefix)
  			verbose = 1;
  			continue;
  		}
+		if (!strcmp(arg, "-a")) {
+			update_type = ALL;
+			continue;
+		}
  		if (!strcmp(arg, "-u")) {
-			take_worktree_changes = 1;
+			update_type = CURRENT_DIRECTORY;
  			continue;
  		}
  		if (!strcmp(arg, "--refresh")) {
@@ -212,8 +235,8 @@ int cmd_add(int argc, const char **argv, const  
char *prefix)
  		usage(builtin_add_usage);
  	}

-	if (take_worktree_changes) {
-		update(verbose, prefix, argv + i);
+	if (update_type) {
+		update(update_type, verbose, show_only, prefix, argv + i);
  		goto finish;
  	}

-- 
1.5.3.4.207.g6d7480-dirty

Re: [PATCH] [BUG FIXED 2] git-add (-a|-u) and -n support

[Matthieu Moy]

Michael Witten <mfwitten@MIT.EDU> writes:

> Subject: [PATCH] git-add now understands two kinds of update:
>
>  	-u: update as before
>  	-a: update all as in a true 'git commit -a'

I don't find the option set very intuitive. I'd prefer

  - git add -u . => update the current directory as before
  - git add -u   => update all files from the root.

But your solution has the advantage of being backward compatible, so,
no strong opinion here.

(side note: also, while you're here, it would be nice to have a single
command to do "git add .; git add -u", i.e add all unknown files,
update all existing files, and actally remove all deleted files. In
one word, synchronize the index with the working tree completely.
Perhaps "-a" would be a good name for that, not sure)


>  builtin-add.c |   69 +++++++++++++++++++++++++++++++++++++
> +-------------------

Your patch is whitespace-damaged. I don't know how to fix that for
Apple Mail, but git-send-email can help.

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

You should document -a here, and in Documentation/git-add.txt if you
introduce it.

-- 
Matthieu

Re: [PATCH] [BUG FIXED 2] git-add (-a|-u) and -n support

[Michael Witten]

Thanks for the reply!

On 14 Oct 2007, at 9:25:03 AM, Matthieu Moy wrote:

> Michael Witten <mfwitten@MIT.EDU> writes:
>
>> Subject: [PATCH] git-add now understands two kinds of update:
>>
>>  	-u: update as before
>>  	-a: update all as in a true 'git commit -a'
>
> I don't find the option set very intuitive. I'd prefer
>
>   - git add -u . => update the current directory as before
>   - git add -u   => update all files from the root.
>
> But your solution has the advantage of being backward compatible, so,
> no strong opinion here.

Here's compromise that is backwards compatible. For both git-add
and git-commit:
	
	-a dir [dir2 ...] => all changes in the given dirs.
	-a                => all changes from the root.

Then we can just leave -u in place for now, and mark it as deprecated.

In any case, the goal is to make the intuition solid between
git-commit and git-add.


> (side note: also, while you're here, it would be nice to have a single
> command to do "git add .; git add -u", i.e add all unknown files,
> update all existing files, and actally remove all deleted files. In
> one word, synchronize the index with the working tree completely.
> Perhaps "-a" would be a good name for that, not sure)

We can couple the above with -f to do this.


>>  builtin-add.c |   69 +++++++++++++++++++++++++++++++++++++
>> +-------------------
>
> Your patch is whitespace-damaged. I don't know how to fix that for
> Apple Mail, but git-send-email can help.

I see that now; Apple's trying to be smart about blank lines, it  
would seem.

>> static const char builtin_add_usage[] =
>>  "git-add [-n] [-v] [-f] [--interactive | -i] [-u] [--refresh] [--]
>> <filepattern>...";
>
> You should document -a here, and in Documentation/git-add.txt if you
> introduce it.

Let's get -a/-u squared away first.

Thanks again,
Michael Witten

Re: [PATCH] git-add (-a|-u) and -n support

[Jeff King]

On Sun, Oct 14, 2007 at 06:26:28AM -0400, Michael Witten wrote:

> The patch is below, but you can also download it from
> http://web.mit.edu/mfwitten/git/0001-git-add-now-understands-two-kinds-of-update.patch

Hi Michael,

Thank you for submitting a patch! However, please make sure you read
SubmittingPatches carefully. Your message should consist of the regular
mail headers (as generated by git-format-patch + whatever else you want
to add), the commit message, a line containing "---", and then the diff.
Any cover letter material should go after the "---".

IOW, the format the git-format-patch generates is actually a mail in the
correct format. If you can convince your mailer to use that as a
template for sending a mail, that is the best bet.

I know these seem like little things, but they make the life of the
maintainer much easier, as it means your patch and commit message can be
applied directly by the git tools.

-Peff "policing the list with an iron fist in Junio's absence" King

Re: [PATCH] git-add (-a|-u) and -n support

[Michael Witten]

On 15 Oct 2007, at 12:20:28 AM, Jeff King wrote:

> Thank you for submitting a patch! However, please make sure you read
> SubmittingPatches carefully.

I apologize, though I got your first email about
Documentation/SubmittingPatches after I had sent
in this patch; I had gone rummaging around the homepage
for some information, but had found nothing special.

I just submitted a patch (properly!) to Petr Baudis
to add a link to that documentation on the main page.

> -Peff "policing the list with an iron fist in Junio's absence" King

Don't worry, I didn't take it personally ;-)

Sincerely,
Michael Witten

Re: [PATCH] [BUG FIXED 2] git-add (-a|-u) and -n support

[Karl Hasselström]

On 2007-10-14 18:00:44 -0400, Michael Witten wrote:

> Here's compromise that is backwards compatible. For both git-add and
> git-commit:
>
>       -a dir [dir2 ...] => all changes in the given dirs.
>       -a                => all changes from the root.
>
> Then we can just leave -u in place for now, and mark it as
> deprecated.
>
> In any case, the goal is to make the intuition solid between
> git-commit and git-add.

As I recall, Junio had some specific reason for calling the flag -u
instead of -a. Search in the list archive for the patch that
introduced the flag, or wait till he gets back and ask him.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

Re: [PATCH] [BUG FIXED 2] git-add (-a|-u) and -n support

[Michael Witten]

On 15 Oct 2007, at 10:31:37 AM, Karl Hasselström wrote:

> On 2007-10-14 18:00:44 -0400, Michael Witten wrote:
>
>> Here's compromise that is backwards compatible. For both git-add and
>> git-commit:
>>
>>       -a dir [dir2 ...] => all changes in the given dirs.
>>       -a                => all changes from the root.
>>
>> Then we can just leave -u in place for now, and mark it as
>> deprecated.
>>
>> In any case, the goal is to make the intuition solid between
>> git-commit and git-add.
>
> As I recall, Junio had some specific reason for calling the flag -u
> instead of -a. Search in the list archive for the patch that
> introduced the flag, or wait till he gets back and ask him.

I have a feeling it was because -u currently just updates modified
files in the current directory, while -a updated all modified files.

Fortunately, the suggestion above does not break anything.

Michael Witten

P.S.
When is Junio coming back? I know I know... Look through the  
archives. :-P

Re: [PATCH] [BUG FIXED 2] git-add (-a|-u) and -n support

[Johannes Schindelin]

Hi,

On Mon, 15 Oct 2007, Michael Witten wrote:

> When is Junio coming back? I know I know... Look through the archives. 
> :-P-

It's not in the archives.  Just send your stuff, and let the list be a 
filter for Junio.  He'll be overwhelmed with the amount of emails already.

If I were to give him some advice, it would be to just delete all git list 
mails in his inbox and ask for people to resend their latest and greatest, 
_after_ he had a look at what Lars collected.

Ciao,
Dscho