Git development
 help / color / mirror / Atom feed
* [PATCH] git-hash-object should honor config variables
From: Nicolas Pitre @ 2007-11-10 20:00 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

... such as core.compression.

Signed-off-by: Nicolas Pitre <nico@cam.org>
---
diff --git a/hash-object.c b/hash-object.c
index 18f5017..0a58f3f 100644
--- a/hash-object.c
+++ b/hash-object.c
@@ -42,6 +42,8 @@ int main(int argc, char **argv)
 	int prefix_length = -1;
 	int no_more_flags = 0;
 
+	git_config(git_default_config);
+
 	for (i = 1 ; i < argc; i++) {
 		if (!no_more_flags && argv[i][0] == '-') {
 			if (!strcmp(argv[i], "-t")) {

^ permalink raw reply related

* Re: [PATCH] status&commit: Teach them to show commits of modified submodules.
From: Sven Verdoolaege @ 2007-11-10 20:00 UTC (permalink / raw)
  To: Ping Yin; +Cc: git
In-Reply-To: <20071110195509.GI2261MdfPADPa@greensroom.kotnet.org>

On Sat, Nov 10, 2007 at 08:55:09PM +0100, Sven Verdoolaege wrote:
> On Sun, Nov 11, 2007 at 03:27:43AM +0800, Ping Yin wrote:
> > This commit teaches git status/commit to also show commits of user-cared
> 
> Does it?  It looks like you only changed git-commit.

OK.  Never mind about this one.

skimo

^ permalink raw reply

* Re: [PATCH] status&commit: Teach them to show commits of modified submodules.
From: Sven Verdoolaege @ 2007-11-10 19:55 UTC (permalink / raw)
  To: Ping Yin; +Cc: git
In-Reply-To: <1194722863-14741-1-git-send-email-pkufranky@gmail.com>

On Sun, Nov 11, 2007 at 03:27:43AM +0800, Ping Yin wrote:
> This commit teaches git status/commit to also show commits of user-cared

Does it?  It looks like you only changed git-commit.
Shouldn't this be put in wt_status_print, if anywhere?

Also, you have some typos:

> +	test -n "$modules" && echo -e "#\n# submodule modifiled: "$modules"\n#"
[..]
> +				echo "  Warn: $name dosn't contains commit $headone"

skimo

^ permalink raw reply

* [PATCH] status&commit: Teach them to show commits of modified submodules.
From: Ping Yin @ 2007-11-10 19:27 UTC (permalink / raw)
  To: git; +Cc: Ping Yin

git status/commit just treats submodules as ordinary files when reporting status
changes. However, one may also wonder how submodules change (the commits).

This commit teaches git status/commit to also show commits of user-cared
modified submodules since HEAD (or HEAD^ if --amend option is on).
Notes:
	1. Submodules already checked out are considered to be user-cared ones.
	2. For submodules deleted or initially added, commits are not shown.

For example, when commiting, submodule sm1 and sm2 are both changed. sm1 has
commit C in HEAD and commit E in index. The history of sm1 is
	--A-->B-->C (in HEAD:354cd45)
	  \
	   -->D->E (in index:3f751e5)

git status will give the following output (just output commits of submodules
before the original output) to show how to change from commit C (in HEAD) to
commit E (in index) for submodule sm1: backward ('<<<') to commit A, and then
forward ('>>>') to commit E. Similar illustration for output of sm2 is omitted.

	#
	# submodule modifiled: sm1 sm2
	#
	# * sm1 354cd45...3f751e5:
	#   <<<
	#   	one line message for C
	#   	one line message for B
	#   >>>
	#   	one line message for D
	#   	one line message for E
	#
	# * sm2 5c8bfb5...ac46d84:
	#   <<<
	#   	msg
	#
	# On branch master
	# Changes to be committed:
	#   (use "git reset HEAD <file>..." to unstage)
	#
	#	modified:   sm1
	#	modified:   sm2

For sm1, if the commit recorded in HEAD/index (say commit C/E) is not found in
the work tree (say sm1 respository in the work tree), a warning will be issued.

	#
	# submodule modifiled: sm1
	#
	# * sm1 354cd45...3f751e5:
	#   Warn: sm1 doesn't contains commit 354cd45
	#
	# On branch master
	# Changes to be committed:
	#   (use "git reset HEAD <file>..." to unstage)
	#
	#	modified:   sm1

Signed-off-by: Ping Yin <pkufranky@gmail.com>
---
 git-commit.sh |   68 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 68 insertions(+), 0 deletions(-)

diff --git a/git-commit.sh b/git-commit.sh
index fcb8443..29f2ebe 100755
--- a/git-commit.sh
+++ b/git-commit.sh
@@ -33,6 +33,68 @@ save_index () {
 	cp -p "$THIS_INDEX" "$NEXT_INDEX"
 }
 
+# Show log of modified submodule (index modification since HEAD or $1)
+# $1 is the commit to be compared, default 'HEAD'
+show_module_log () {
+	cwd=$(pwd)
+	cd_to_toplevel 
+
+	# get modified modules which have been checked out (i.e. cared by user)
+	modules=$(git diff --cached --name-only $1 |
+	(
+		IFS=''	# handle the module name containing space or tab
+		while read name
+		do
+			git ls-files --stage "$name" | grep '^160000 ' >&/dev/null &&
+			GIT_DIR="$name/.git" git-rev-parse --git-dir >&/dev/null &&
+			echo "$name"
+		done
+	)
+	)
+
+	# TODO: quote module names containing space or tab
+	test -n "$modules" && echo -e "#\n# submodule modifiled: "$modules"\n#"
+	OLDIFS=$IFS
+	IFS=$'\n\r'	# '\r' for mac os 
+	for name in $modules
+	do
+		range=$(git diff --cached -- "$name" | sed -n '/^index.*160000$/ p' | awk '{print $2}')
+		indexone=${range#*..}
+		headone=${range%..*}
+		(
+			echo "* $name $headone...$indexone:"
+			headfail=
+			indexfail=
+			GIT_DIR="$name/.git" git-rev-parse $headone >&/dev/null || headfail='t'
+			GIT_DIR="$name/.git" git-rev-parse $indexone >&/dev/null || indexfail='t'
+			case "$headfail,$indexfail" in
+			t,)
+				echo "  Warn: $name dosn't contains commit $headone"
+				;;
+			,t)
+				echo "  Warn: $name dosn't contains commit $indexone"
+				;;
+			t,t)
+				echo "  Warn: $name dosn't contains commits $headone and $indexone"
+				;;
+			*)
+				left=$(GIT_DIR="$name/.git" git log --pretty=oneline $indexone..$headone 2>&1 |
+				sed 's/^\w\+ /  \t/')
+				right=$(GIT_DIR="$name/.git" git log --pretty=oneline --reverse  $headone..$indexone 2>&1 |
+				sed 's/^\w\+ /  \t/')
+
+				test -n "$left" && echo -e "  <<<\n$left"
+				test -n "$right" && echo -e "  >>>\n$right"
+				;;
+			esac
+			echo
+		) | sed 's/^/# /'
+	done
+	IFS=$OLDIFS
+
+	cd "$cwd"
+}
+
 run_status () {
 	# If TMP_INDEX is defined, that means we are doing
 	# "--only" partial commit, and that index file is used
@@ -55,6 +117,12 @@ run_status () {
 	else
 		color=--nocolor
 	fi
+	if test -z "$amend"
+	then
+		show_module_log
+	else
+		show_module_log "HEAD^"
+	fi
 	git runstatus ${color} \
 		${verbose:+--verbose} \
 		${amend:+--amend} \
-- 
1.5.3.4

^ permalink raw reply related

* [PATCH] gitweb: correct month in date display for atom feeds
From: Vincent Zanotti @ 2007-11-10 18:55 UTC (permalink / raw)
  To: git

Signed-off-by: Vincent Zanotti <vincent.zanotti@m4x.org>

---
 gitweb/gitweb.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 759dff1..e788ef9 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1856,7 +1856,7 @@ sub parse_date {
        $date{'mday-time'} = sprintf "%d %s %02d:%02d",
                             $mday, $months[$mon], $hour ,$min;
        $date{'iso-8601'}  = sprintf "%04d-%02d-%02dT%02d:%02d:%02dZ",
-                            1900+$year, $mon, $mday, $hour ,$min, $sec;
+                            1900+$year, 1+$mon, $mday, $hour ,$min, $sec;

        $tz =~ m/^([+\-][0-9][0-9])([0-9][0-9])$/;
        my $local = $epoch + ((int $1 + ($2/60)) * 3600);
--
1.5.3.4

^ permalink raw reply related

* Re: [PATCH v4] user-manual: Add section "Why bisecting merge commits can be harder ..."
From: Linus Torvalds @ 2007-11-10 19:10 UTC (permalink / raw)
  To: Steffen Prohaska
  Cc: Junio C Hamano, Benoit Sigoure, Andreas Ericsson, Johannes Sixt,
	git
In-Reply-To: <1194702594213-git-send-email-prohaska@zib.de>



On Sat, 10 Nov 2007, Steffen Prohaska wrote:
> +
> +But if you already made a merge C instead of rebasing, all
> +is not lost.  In the illustrated case, you can easily rebase
> +one parent branch on top of the other after the fact, just
> +to understand the history and to make the history more
> +easily to bisect.

I simply don't think this is true. 

You can *not* easily rebase after the fact. Both the parents may have lots 
of merges independently of each other, and rebase won't be able to do 
*anything* with such a case. In fact, the only reason you think you can 
easily rebase after-the-fact is that you made the example history be 
trivial. In real life, if the example history is that trivial (just a 
single merge of two otherwise linear histories), you'd probably generally 
not have this problem in the first place.

The facts are:

 - git bisect can handle merges quite well, and points to the right 
   commit (which is *usually* not a merge).

 - but merges by definition introduce changes from two independent lines 
   of development, and while both lines may work well on their own, it is 
   possible that (a) the merge itself was done incorrectly or (b) the two 
   (or more) features that were introduced simply clash.

 - "git rebase" won't do a thing for this after the fact, except in 
   trivial cases, and even then it will generate a new history that simply 
   isn't compatible with the original history, so while it could in theory 
   help bisect things further in some limited and simple cases, in general 
   it's simply not that useful an approach.

..and I think it's simply wrong to even *try* to imply that "git rebase" 
can somehow change any of this.

			Linus

^ permalink raw reply

* [UPDATE DIFF-CLEANUP 1/2] Make the diff_options bitfields be an unsigned with explicit masks.
From: Pierre Habouzit @ 2007-11-10 19:05 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vy7d95ji1.fsf@gitster.siamese.dyndns.org>

reverse_diff was a bit-value in disguise, it's merged in the flags now.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 builtin-blame.c      |   10 ++--
 builtin-diff-files.c |    4 +-
 builtin-diff-index.c |    4 +-
 builtin-diff-tree.c  |    7 ++-
 builtin-diff.c       |   12 +++---
 builtin-log.c        |   24 ++++------
 combine-diff.c       |   10 ++--
 diff-lib.c           |   24 +++++-----
 diff.c               |  123 ++++++++++++++++++++++++-------------------------
 diff.h               |   40 ++++++++++-------
 log-tree.c           |    6 +--
 merge-recursive.c    |    2 +-
 patch-ids.c          |    2 +-
 revision.c           |   22 +++++-----
 tree-diff.c          |   14 +++---
 15 files changed, 155 insertions(+), 149 deletions(-)

diff --git a/builtin-blame.c b/builtin-blame.c
index ba80bf8..c158d31 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -335,7 +335,7 @@ static struct origin *find_origin(struct scoreboard *sb,
 	 * same and diff-tree is fairly efficient about this.
 	 */
 	diff_setup(&diff_opts);
-	diff_opts.recursive = 1;
+	DIFF_OPT_SET(&diff_opts, RECURSIVE);
 	diff_opts.detect_rename = 0;
 	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
 	paths[0] = origin->path;
@@ -409,7 +409,7 @@ static struct origin *find_rename(struct scoreboard *sb,
 	const char *paths[2];
 
 	diff_setup(&diff_opts);
-	diff_opts.recursive = 1;
+	DIFF_OPT_SET(&diff_opts, RECURSIVE);
 	diff_opts.detect_rename = DIFF_DETECT_RENAME;
 	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
 	diff_opts.single_follow = origin->path;
@@ -1075,7 +1075,7 @@ static int find_copy_in_parent(struct scoreboard *sb,
 		return 1; /* nothing remains for this target */
 
 	diff_setup(&diff_opts);
-	diff_opts.recursive = 1;
+	DIFF_OPT_SET(&diff_opts, RECURSIVE);
 	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
 
 	paths[0] = NULL;
@@ -1093,7 +1093,7 @@ static int find_copy_in_parent(struct scoreboard *sb,
 	if ((opt & PICKAXE_BLAME_COPY_HARDEST)
 	    || ((opt & PICKAXE_BLAME_COPY_HARDER)
 		&& (!porigin || strcmp(target->path, porigin->path))))
-		diff_opts.find_copies_harder = 1;
+		DIFF_OPT_SET(&diff_opts, FIND_COPIES_HARDER);
 
 	if (is_null_sha1(target->commit->object.sha1))
 		do_diff_cache(parent->tree->object.sha1, &diff_opts);
@@ -1102,7 +1102,7 @@ static int find_copy_in_parent(struct scoreboard *sb,
 			       target->commit->tree->object.sha1,
 			       "", &diff_opts);
 
-	if (!diff_opts.find_copies_harder)
+	if (!DIFF_OPT_TST(&diff_opts, FIND_COPIES_HARDER))
 		diffcore_std(&diff_opts);
 
 	retval = 0;
diff --git a/builtin-diff-files.c b/builtin-diff-files.c
index 6cb30c8..046b7e3 100644
--- a/builtin-diff-files.c
+++ b/builtin-diff-files.c
@@ -31,5 +31,7 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)
 	if (!rev.diffopt.output_format)
 		rev.diffopt.output_format = DIFF_FORMAT_RAW;
 	result = run_diff_files_cmd(&rev, argc, argv);
-	return rev.diffopt.exit_with_status ? rev.diffopt.has_changes: result;
+	if (DIFF_OPT_TST(&rev.diffopt, EXIT_WITH_STATUS))
+		return DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES) != 0;
+	return result;
 }
diff --git a/builtin-diff-index.c b/builtin-diff-index.c
index 81e7167..556c506 100644
--- a/builtin-diff-index.c
+++ b/builtin-diff-index.c
@@ -44,5 +44,7 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix)
 		return -1;
 	}
 	result = run_diff_index(&rev, cached);
-	return rev.diffopt.exit_with_status ? rev.diffopt.has_changes: result;
+	if (DIFF_OPT_TST(&rev.diffopt, EXIT_WITH_STATUS))
+		return DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES) != 0;
+	return result;
 }
diff --git a/builtin-diff-tree.c b/builtin-diff-tree.c
index 0b591c8..2e13716 100644
--- a/builtin-diff-tree.c
+++ b/builtin-diff-tree.c
@@ -118,8 +118,8 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
 	}
 
 	if (!read_stdin)
-		return opt->diffopt.exit_with_status ?
-		    opt->diffopt.has_changes: 0;
+		return DIFF_OPT_TST(&opt->diffopt, EXIT_WITH_STATUS)
+			&& DIFF_OPT_TST(&opt->diffopt, HAS_CHANGES);
 
 	if (opt->diffopt.detect_rename)
 		opt->diffopt.setup |= (DIFF_SETUP_USE_SIZE_CACHE |
@@ -134,5 +134,6 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
 		else
 			diff_tree_stdin(line);
 	}
-	return opt->diffopt.exit_with_status ? opt->diffopt.has_changes: 0;
+	return DIFF_OPT_TST(&opt->diffopt, EXIT_WITH_STATUS)
+		&& DIFF_OPT_TST(&opt->diffopt, HAS_CHANGES);
 }
diff --git a/builtin-diff.c b/builtin-diff.c
index f557d21..1b61599 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -35,7 +35,7 @@ static void stuff_change(struct diff_options *opt,
 	    !hashcmp(old_sha1, new_sha1) && (old_mode == new_mode))
 		return;
 
-	if (opt->reverse_diff) {
+	if (DIFF_OPT_TST(opt, REVERSE_DIFF)) {
 		unsigned tmp;
 		const unsigned char *tmp_u;
 		const char *tmp_c;
@@ -253,13 +253,13 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 		if (diff_setup_done(&rev.diffopt) < 0)
 			die("diff_setup_done failed");
 	}
-	rev.diffopt.allow_external = 1;
-	rev.diffopt.recursive = 1;
+	DIFF_OPT_SET(&rev.diffopt, ALLOW_EXTERNAL);
+	DIFF_OPT_SET(&rev.diffopt, RECURSIVE);
 
 	/* If the user asked for our exit code then don't start a
 	 * pager or we would end up reporting its exit code instead.
 	 */
-	if (!rev.diffopt.exit_with_status)
+	if (!DIFF_OPT_TST(&rev.diffopt, EXIT_WITH_STATUS))
 		setup_pager();
 
 	/* Do we have --cached and not have a pending object, then
@@ -363,8 +363,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 	else
 		result = builtin_diff_combined(&rev, argc, argv,
 					     ent, ents);
-	if (rev.diffopt.exit_with_status)
-		result = rev.diffopt.has_changes;
+	if (DIFF_OPT_TST(&rev.diffopt, EXIT_WITH_STATUS))
+		result = DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES) != 0;
 
 	if (1 < rev.diffopt.skip_stat_unmatch)
 		refresh_index_quietly();
diff --git a/builtin-log.c b/builtin-log.c
index b6ca04a..72745cd 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -55,13 +55,13 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
 	rev->abbrev = DEFAULT_ABBREV;
 	rev->commit_format = CMIT_FMT_DEFAULT;
 	rev->verbose_header = 1;
-	rev->diffopt.recursive = 1;
+	DIFF_OPT_SET(&rev->diffopt, RECURSIVE);
 	rev->show_root_diff = default_show_root;
 	rev->subject_prefix = fmt_patch_subject_prefix;
 	argc = setup_revisions(argc, argv, rev, "HEAD");
 	if (rev->diffopt.pickaxe || rev->diffopt.filter)
 		rev->always_show_header = 0;
-	if (rev->diffopt.follow_renames) {
+	if (DIFF_OPT_TST(&rev->diffopt, FOLLOW_RENAMES)) {
 		rev->always_show_header = 0;
 		if (rev->diffopt.nr_paths != 1)
 			usage("git logs can only follow renames on one pathname at a time");
@@ -308,11 +308,9 @@ int cmd_show(int argc, const char **argv, const char *prefix)
 			struct tag *t = (struct tag *)o;
 
 			printf("%stag %s%s\n\n",
-					diff_get_color(rev.diffopt.color_diff,
-						DIFF_COMMIT),
+					diff_get_color_opt(&rev.diffopt, DIFF_COMMIT),
 					t->tag,
-					diff_get_color(rev.diffopt.color_diff,
-						DIFF_RESET));
+					diff_get_color_opt(&rev.diffopt, DIFF_RESET));
 			ret = show_object(o->sha1, 1);
 			objects[i].item = (struct object *)t->tagged;
 			i--;
@@ -320,11 +318,9 @@ int cmd_show(int argc, const char **argv, const char *prefix)
 		}
 		case OBJ_TREE:
 			printf("%stree %s%s\n\n",
-					diff_get_color(rev.diffopt.color_diff,
-						DIFF_COMMIT),
+					diff_get_color_opt(&rev.diffopt, DIFF_COMMIT),
 					name,
-					diff_get_color(rev.diffopt.color_diff,
-						DIFF_RESET));
+					diff_get_color_opt(&rev.diffopt, DIFF_RESET));
 			read_tree_recursive((struct tree *)o, "", 0, 0, NULL,
 					show_tree_object);
 			break;
@@ -620,7 +616,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	rev.combine_merges = 0;
 	rev.ignore_merges = 1;
 	rev.diffopt.msg_sep = "";
-	rev.diffopt.recursive = 1;
+	DIFF_OPT_SET(&rev.diffopt, RECURSIVE);
 
 	rev.subject_prefix = fmt_patch_subject_prefix;
 	rev.extra_headers = extra_headers;
@@ -728,8 +724,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	if (!rev.diffopt.output_format)
 		rev.diffopt.output_format = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_SUMMARY | DIFF_FORMAT_PATCH;
 
-	if (!rev.diffopt.text)
-		rev.diffopt.binary = 1;
+	if (!DIFF_OPT_TST(&rev.diffopt, TEXT))
+		DIFF_OPT_SET(&rev.diffopt, BINARY);
 
 	if (!output_directory && !use_stdout)
 		output_directory = prefix;
@@ -887,7 +883,7 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)
 	revs.diff = 1;
 	revs.combine_merges = 0;
 	revs.ignore_merges = 1;
-	revs.diffopt.recursive = 1;
+	DIFF_OPT_SET(&revs.diffopt, RECURSIVE);
 
 	if (add_pending_commit(head, &revs, 0))
 		die("Unknown commit %s", head);
diff --git a/combine-diff.c b/combine-diff.c
index fe5a2a1..5a658dc 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -664,7 +664,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 	int mode_differs = 0;
 	int i, show_hunks;
 	int working_tree_file = is_null_sha1(elem->sha1);
-	int abbrev = opt->full_index ? 40 : DEFAULT_ABBREV;
+	int abbrev = DIFF_OPT_TST(opt, FULL_INDEX) ? 40 : DEFAULT_ABBREV;
 	mmfile_t result_file;
 
 	context = opt->context;
@@ -784,7 +784,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 
 	if (show_hunks || mode_differs || working_tree_file) {
 		const char *abb;
-		int use_color = opt->color_diff;
+		int use_color = DIFF_OPT_TST(opt, COLOR_DIFF);
 		const char *c_meta = diff_get_color(use_color, DIFF_METAINFO);
 		const char *c_reset = diff_get_color(use_color, DIFF_RESET);
 		int added = 0;
@@ -836,7 +836,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 			dump_quoted_path("+++ /dev/", "null", c_meta, c_reset);
 		else
 			dump_quoted_path("+++ b/", elem->path, c_meta, c_reset);
-		dump_sline(sline, cnt, num_parent, opt->color_diff);
+		dump_sline(sline, cnt, num_parent, DIFF_OPT_TST(opt, COLOR_DIFF));
 	}
 	free(result);
 
@@ -929,8 +929,8 @@ void diff_tree_combined(const unsigned char *sha1,
 
 	diffopts = *opt;
 	diffopts.output_format = DIFF_FORMAT_NO_OUTPUT;
-	diffopts.recursive = 1;
-	diffopts.allow_external = 0;
+	DIFF_OPT_SET(&diffopts, RECURSIVE);
+	DIFF_OPT_CLR(&diffopts, ALLOW_EXTERNAL);
 
 	show_log_first = !!rev->loginfo && !rev->no_commit_id;
 	needsep = 0;
diff --git a/diff-lib.c b/diff-lib.c
index da55713..290a170 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -121,7 +121,7 @@ static int queue_diff(struct diff_options *o,
 	} else {
 		struct diff_filespec *d1, *d2;
 
-		if (o->reverse_diff) {
+		if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
 			unsigned tmp;
 			const char *tmp_c;
 			tmp = mode1; mode1 = mode2; mode2 = tmp;
@@ -188,8 +188,8 @@ static int handle_diff_files_args(struct rev_info *revs,
 		else if (!strcmp(argv[1], "-n") ||
 				!strcmp(argv[1], "--no-index")) {
 			revs->max_count = -2;
-			revs->diffopt.exit_with_status = 1;
-			revs->diffopt.no_index = 1;
+			DIFF_OPT_SET(&revs->diffopt, EXIT_WITH_STATUS);
+			DIFF_OPT_SET(&revs->diffopt, NO_INDEX);
 		}
 		else if (!strcmp(argv[1], "-q"))
 			*silent = 1;
@@ -207,7 +207,7 @@ static int handle_diff_files_args(struct rev_info *revs,
 		if (!is_in_index(revs->diffopt.paths[0]) ||
 					!is_in_index(revs->diffopt.paths[1])) {
 			revs->max_count = -2;
-			revs->diffopt.no_index = 1;
+			DIFF_OPT_SET(&revs->diffopt, NO_INDEX);
 		}
 	}
 
@@ -258,7 +258,7 @@ int setup_diff_no_index(struct rev_info *revs,
 			break;
 		} else if (i < argc - 3 && !strcmp(argv[i], "--no-index")) {
 			i = argc - 3;
-			revs->diffopt.exit_with_status = 1;
+			DIFF_OPT_SET(&revs->diffopt, EXIT_WITH_STATUS);
 			break;
 		}
 	if (argc != i + 2 || (!is_outside_repo(argv[i + 1], nongit, prefix) &&
@@ -296,7 +296,7 @@ int setup_diff_no_index(struct rev_info *revs,
 	else
 		revs->diffopt.paths = argv + argc - 2;
 	revs->diffopt.nr_paths = 2;
-	revs->diffopt.no_index = 1;
+	DIFF_OPT_SET(&revs->diffopt, NO_INDEX);
 	revs->max_count = -2;
 	if (diff_setup_done(&revs->diffopt) < 0)
 		die("diff_setup_done failed");
@@ -310,7 +310,7 @@ int run_diff_files_cmd(struct rev_info *revs, int argc, const char **argv)
 	if (handle_diff_files_args(revs, argc, argv, &silent_on_removed))
 		return -1;
 
-	if (revs->diffopt.no_index) {
+	if (DIFF_OPT_TST(&revs->diffopt, NO_INDEX)) {
 		if (revs->diffopt.nr_paths != 2)
 			return error("need two files/directories with --no-index");
 		if (queue_diff(&revs->diffopt, revs->diffopt.paths[0],
@@ -346,7 +346,8 @@ int run_diff_files(struct rev_info *revs, int silent_on_removed)
 		struct cache_entry *ce = active_cache[i];
 		int changed;
 
-		if (revs->diffopt.quiet && revs->diffopt.has_changes)
+		if (DIFF_OPT_TST(&revs->diffopt, QUIET) &&
+			DIFF_OPT_TST(&revs->diffopt, HAS_CHANGES))
 			break;
 
 		if (!ce_path_match(ce, revs->prune_data))
@@ -442,7 +443,7 @@ int run_diff_files(struct rev_info *revs, int silent_on_removed)
 			continue;
 		}
 		changed = ce_match_stat(ce, &st, 0);
-		if (!changed && !revs->diffopt.find_copies_harder)
+		if (!changed && !DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER))
 			continue;
 		oldmode = ntohl(ce->ce_mode);
 		newmode = ntohl(ce_mode_from_stat(ce, st.st_mode));
@@ -561,7 +562,7 @@ static int show_modified(struct rev_info *revs,
 
 	oldmode = old->ce_mode;
 	if (mode == oldmode && !hashcmp(sha1, old->sha1) &&
-	    !revs->diffopt.find_copies_harder)
+	    !DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER))
 		return 0;
 
 	mode = ntohl(mode);
@@ -581,7 +582,8 @@ static int diff_cache(struct rev_info *revs,
 		struct cache_entry *ce = *ac;
 		int same = (entries > 1) && ce_same_name(ce, ac[1]);
 
-		if (revs->diffopt.quiet && revs->diffopt.has_changes)
+		if (DIFF_OPT_TST(&revs->diffopt, QUIET) &&
+			DIFF_OPT_TST(&revs->diffopt, HAS_CHANGES))
 			break;
 
 		if (!ce_path_match(ce, pathspec))
diff --git a/diff.c b/diff.c
index 6bb902f..97c9a59 100644
--- a/diff.c
+++ b/diff.c
@@ -827,10 +827,10 @@ static void show_stats(struct diffstat_t* data, struct diff_options *options)
 	}
 
 	/* Find the longest filename and max number of changes */
-	reset = diff_get_color(options->color_diff, DIFF_RESET);
-	set = diff_get_color(options->color_diff, DIFF_PLAIN);
-	add_c = diff_get_color(options->color_diff, DIFF_FILE_NEW);
-	del_c = diff_get_color(options->color_diff, DIFF_FILE_OLD);
+	reset = diff_get_color_opt(options, DIFF_RESET);
+	set   = diff_get_color_opt(options, DIFF_PLAIN);
+	add_c = diff_get_color_opt(options, DIFF_FILE_NEW);
+	del_c = diff_get_color_opt(options, DIFF_FILE_OLD);
 
 	for (i = 0; i < data->nr; i++) {
 		struct diffstat_file *file = data->files[i];
@@ -1256,8 +1256,8 @@ static void builtin_diff(const char *name_a,
 	mmfile_t mf1, mf2;
 	const char *lbl[2];
 	char *a_one, *b_two;
-	const char *set = diff_get_color(o->color_diff, DIFF_METAINFO);
-	const char *reset = diff_get_color(o->color_diff, DIFF_RESET);
+	const char *set = diff_get_color_opt(o, DIFF_METAINFO);
+	const char *reset = diff_get_color_opt(o, DIFF_RESET);
 
 	a_one = quote_two("a/", name_a + (*name_a == '/'));
 	b_two = quote_two("b/", name_b + (*name_b == '/'));
@@ -1290,7 +1290,7 @@ static void builtin_diff(const char *name_a,
 			goto free_ab_and_return;
 		if (complete_rewrite) {
 			emit_rewrite_diff(name_a, name_b, one, two,
-					o->color_diff);
+					DIFF_OPT_TST(o, COLOR_DIFF));
 			o->found_changes = 1;
 			goto free_ab_and_return;
 		}
@@ -1299,13 +1299,13 @@ static void builtin_diff(const char *name_a,
 	if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
 		die("unable to read files to diff");
 
-	if (!o->text &&
+	if (!DIFF_OPT_TST(o, TEXT) &&
 	    (diff_filespec_is_binary(one) || diff_filespec_is_binary(two))) {
 		/* Quite common confusing case */
 		if (mf1.size == mf2.size &&
 		    !memcmp(mf1.ptr, mf2.ptr, mf1.size))
 			goto free_ab_and_return;
-		if (o->binary)
+		if (DIFF_OPT_TST(o, BINARY))
 			emit_binary_diff(&mf1, &mf2);
 		else
 			printf("Binary files %s and %s differ\n",
@@ -1328,7 +1328,7 @@ static void builtin_diff(const char *name_a,
 		memset(&xecfg, 0, sizeof(xecfg));
 		memset(&ecbdata, 0, sizeof(ecbdata));
 		ecbdata.label_path = lbl;
-		ecbdata.color_diff = o->color_diff;
+		ecbdata.color_diff = DIFF_OPT_TST(o, COLOR_DIFF);
 		ecbdata.found_changesp = &o->found_changes;
 		xpp.flags = XDF_NEED_MINIMAL | o->xdl_opts;
 		xecfg.ctxlen = o->context;
@@ -1344,11 +1344,11 @@ static void builtin_diff(const char *name_a,
 		ecb.outf = xdiff_outf;
 		ecb.priv = &ecbdata;
 		ecbdata.xm.consume = fn_out_consume;
-		if (o->color_diff_words)
+		if (DIFF_OPT_TST(o, COLOR_DIFF_WORDS))
 			ecbdata.diff_words =
 				xcalloc(1, sizeof(struct diff_words_data));
 		xdl_diff(&mf1, &mf2, &xpp, &xecfg, &ecb);
-		if (o->color_diff_words)
+		if (DIFF_OPT_TST(o, COLOR_DIFF_WORDS))
 			free_diff_words_data(&ecbdata);
 	}
 
@@ -1422,7 +1422,7 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,
 	data.xm.consume = checkdiff_consume;
 	data.filename = name_b ? name_b : name_a;
 	data.lineno = 0;
-	data.color_diff = o->color_diff;
+	data.color_diff = DIFF_OPT_TST(o, COLOR_DIFF);
 
 	if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
 		die("unable to read files to diff");
@@ -1866,7 +1866,7 @@ static void run_diff_cmd(const char *pgm,
 			 struct diff_options *o,
 			 int complete_rewrite)
 {
-	if (!o->allow_external)
+	if (!DIFF_OPT_TST(o, ALLOW_EXTERNAL))
 		pgm = NULL;
 	else {
 		const char *cmd = external_diff_attr(name);
@@ -1964,9 +1964,9 @@ static void run_diff(struct diff_filepair *p, struct diff_options *o)
 	}
 
 	if (hashcmp(one->sha1, two->sha1)) {
-		int abbrev = o->full_index ? 40 : DEFAULT_ABBREV;
+		int abbrev = DIFF_OPT_TST(o, FULL_INDEX) ? 40 : DEFAULT_ABBREV;
 
-		if (o->binary) {
+		if (DIFF_OPT_TST(o, BINARY)) {
 			mmfile_t mf;
 			if ((!fill_mmfile(&mf, one) && diff_filespec_is_binary(one)) ||
 			    (!fill_mmfile(&mf, two) && diff_filespec_is_binary(two)))
@@ -2058,7 +2058,10 @@ void diff_setup(struct diff_options *options)
 
 	options->change = diff_change;
 	options->add_remove = diff_addremove;
-	options->color_diff = diff_use_color_default;
+	if (diff_use_color_default)
+		DIFF_OPT_SET(options, COLOR_DIFF);
+	else
+		DIFF_OPT_CLR(options, COLOR_DIFF);
 	options->detect_rename = diff_detect_rename_default;
 }
 
@@ -2077,7 +2080,7 @@ int diff_setup_done(struct diff_options *options)
 	if (count > 1)
 		die("--name-only, --name-status, --check and -s are mutually exclusive");
 
-	if (options->find_copies_harder)
+	if (DIFF_OPT_TST(options, FIND_COPIES_HARDER))
 		options->detect_rename = DIFF_DETECT_COPY;
 
 	if (options->output_format & (DIFF_FORMAT_NAME |
@@ -2101,12 +2104,12 @@ int diff_setup_done(struct diff_options *options)
 				      DIFF_FORMAT_SHORTSTAT |
 				      DIFF_FORMAT_SUMMARY |
 				      DIFF_FORMAT_CHECKDIFF))
-		options->recursive = 1;
+		DIFF_OPT_SET(options, RECURSIVE);
 	/*
 	 * Also pickaxe would not work very well if you do not say recursive
 	 */
 	if (options->pickaxe)
-		options->recursive = 1;
+		DIFF_OPT_SET(options, RECURSIVE);
 
 	if (options->detect_rename && options->rename_limit < 0)
 		options->rename_limit = diff_rename_limit_default;
@@ -2128,9 +2131,9 @@ int diff_setup_done(struct diff_options *options)
 	 * to have found.  It does not make sense not to return with
 	 * exit code in such a case either.
 	 */
-	if (options->quiet) {
+	if (DIFF_OPT_TST(options, QUIET)) {
 		options->output_format = DIFF_FORMAT_NO_OUTPUT;
-		options->exit_with_status = 1;
+		DIFF_OPT_SET(options, EXIT_WITH_STATUS);
 	}
 
 	/*
@@ -2138,7 +2141,7 @@ int diff_setup_done(struct diff_options *options)
 	 * upon the first hit.  We need to run diff as usual.
 	 */
 	if (options->pickaxe || options->filter)
-		options->quiet = 0;
+		DIFF_OPT_CLR(options, QUIET);
 
 	return 0;
 }
@@ -2201,15 +2204,12 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		options->output_format |= DIFF_FORMAT_PATCH;
 	else if (!strcmp(arg, "--raw"))
 		options->output_format |= DIFF_FORMAT_RAW;
-	else if (!strcmp(arg, "--patch-with-raw")) {
+	else if (!strcmp(arg, "--patch-with-raw"))
 		options->output_format |= DIFF_FORMAT_PATCH | DIFF_FORMAT_RAW;
-	}
-	else if (!strcmp(arg, "--numstat")) {
+	else if (!strcmp(arg, "--numstat"))
 		options->output_format |= DIFF_FORMAT_NUMSTAT;
-	}
-	else if (!strcmp(arg, "--shortstat")) {
+	else if (!strcmp(arg, "--shortstat"))
 		options->output_format |= DIFF_FORMAT_SHORTSTAT;
-	}
 	else if (!prefixcmp(arg, "--stat")) {
 		char *end;
 		int width = options->stat_width;
@@ -2241,33 +2241,30 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		options->output_format |= DIFF_FORMAT_CHECKDIFF;
 	else if (!strcmp(arg, "--summary"))
 		options->output_format |= DIFF_FORMAT_SUMMARY;
-	else if (!strcmp(arg, "--patch-with-stat")) {
+	else if (!strcmp(arg, "--patch-with-stat"))
 		options->output_format |= DIFF_FORMAT_PATCH | DIFF_FORMAT_DIFFSTAT;
-	}
 	else if (!strcmp(arg, "-z"))
 		options->line_termination = 0;
 	else if (!prefixcmp(arg, "-l"))
 		options->rename_limit = strtoul(arg+2, NULL, 10);
 	else if (!strcmp(arg, "--full-index"))
-		options->full_index = 1;
+		DIFF_OPT_SET(options, FULL_INDEX);
 	else if (!strcmp(arg, "--binary")) {
 		options->output_format |= DIFF_FORMAT_PATCH;
-		options->binary = 1;
-	}
-	else if (!strcmp(arg, "-a") || !strcmp(arg, "--text")) {
-		options->text = 1;
+		DIFF_OPT_SET(options, BINARY);
 	}
+	else if (!strcmp(arg, "-a") || !strcmp(arg, "--text"))
+		DIFF_OPT_SET(options, TEXT);
 	else if (!strcmp(arg, "--name-only"))
 		options->output_format |= DIFF_FORMAT_NAME;
 	else if (!strcmp(arg, "--name-status"))
 		options->output_format |= DIFF_FORMAT_NAME_STATUS;
 	else if (!strcmp(arg, "-R"))
-		options->reverse_diff = 1;
+		DIFF_OPT_SET(options, REVERSE_DIFF);
 	else if (!prefixcmp(arg, "-S"))
 		options->pickaxe = arg + 2;
-	else if (!strcmp(arg, "-s")) {
+	else if (!strcmp(arg, "-s"))
 		options->output_format |= DIFF_FORMAT_NO_OUTPUT;
-	}
 	else if (!prefixcmp(arg, "-O"))
 		options->orderfile = arg + 2;
 	else if (!prefixcmp(arg, "--diff-filter="))
@@ -2277,28 +2274,25 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 	else if (!strcmp(arg, "--pickaxe-regex"))
 		options->pickaxe_opts = DIFF_PICKAXE_REGEX;
 	else if (!prefixcmp(arg, "-B")) {
-		if ((options->break_opt =
-		     diff_scoreopt_parse(arg)) == -1)
+		if ((options->break_opt = diff_scoreopt_parse(arg)) == -1)
 			return -1;
 	}
 	else if (!prefixcmp(arg, "-M")) {
-		if ((options->rename_score =
-		     diff_scoreopt_parse(arg)) == -1)
+		if ((options->rename_score = diff_scoreopt_parse(arg)) == -1)
 			return -1;
 		options->detect_rename = DIFF_DETECT_RENAME;
 	}
 	else if (!prefixcmp(arg, "-C")) {
 		if (options->detect_rename == DIFF_DETECT_COPY)
-			options->find_copies_harder = 1;
-		if ((options->rename_score =
-		     diff_scoreopt_parse(arg)) == -1)
+			DIFF_OPT_SET(options, FIND_COPIES_HARDER);
+		if ((options->rename_score = diff_scoreopt_parse(arg)) == -1)
 			return -1;
 		options->detect_rename = DIFF_DETECT_COPY;
 	}
 	else if (!strcmp(arg, "--find-copies-harder"))
-		options->find_copies_harder = 1;
+		DIFF_OPT_SET(options, FIND_COPIES_HARDER);
 	else if (!strcmp(arg, "--follow"))
-		options->follow_renames = 1;
+		DIFF_OPT_SET(options, FOLLOW_RENAMES);
 	else if (!strcmp(arg, "--abbrev"))
 		options->abbrev = DEFAULT_ABBREV;
 	else if (!prefixcmp(arg, "--abbrev=")) {
@@ -2309,9 +2303,9 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 			options->abbrev = 40;
 	}
 	else if (!strcmp(arg, "--color"))
-		options->color_diff = 1;
+		DIFF_OPT_SET(options, COLOR_DIFF);
 	else if (!strcmp(arg, "--no-color"))
-		options->color_diff = 0;
+		DIFF_OPT_CLR(options, COLOR_DIFF);
 	else if (!strcmp(arg, "-w") || !strcmp(arg, "--ignore-all-space"))
 		options->xdl_opts |= XDF_IGNORE_WHITESPACE;
 	else if (!strcmp(arg, "-b") || !strcmp(arg, "--ignore-space-change"))
@@ -2319,17 +2313,17 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 	else if (!strcmp(arg, "--ignore-space-at-eol"))
 		options->xdl_opts |= XDF_IGNORE_WHITESPACE_AT_EOL;
 	else if (!strcmp(arg, "--color-words"))
-		options->color_diff = options->color_diff_words = 1;
+		options->flags |= DIFF_OPT_COLOR_DIFF | DIFF_OPT_COLOR_DIFF_WORDS;
 	else if (!strcmp(arg, "--no-renames"))
 		options->detect_rename = 0;
 	else if (!strcmp(arg, "--exit-code"))
-		options->exit_with_status = 1;
+		DIFF_OPT_SET(options, EXIT_WITH_STATUS);
 	else if (!strcmp(arg, "--quiet"))
-		options->quiet = 1;
+		DIFF_OPT_SET(options, QUIET);
 	else if (!strcmp(arg, "--ext-diff"))
-		options->allow_external = 1;
+		DIFF_OPT_SET(options, ALLOW_EXTERNAL);
 	else if (!strcmp(arg, "--no-ext-diff"))
-		options->allow_external = 0;
+		DIFF_OPT_CLR(options, ALLOW_EXTERNAL);
 	else
 		return 0;
 	return 1;
@@ -3084,7 +3078,7 @@ static void diffcore_skip_stat_unmatch(struct diff_options *diffopt)
 			 * to determine how many paths were dirty only
 			 * due to stat info mismatch.
 			 */
-			if (!diffopt->no_index)
+			if (!DIFF_OPT_TST(diffopt, NO_INDEX))
 				diffopt->skip_stat_unmatch++;
 			diff_free_filepair(p);
 		}
@@ -3095,10 +3089,10 @@ static void diffcore_skip_stat_unmatch(struct diff_options *diffopt)
 
 void diffcore_std(struct diff_options *options)
 {
-	if (options->quiet)
+	if (DIFF_OPT_TST(options, QUIET))
 		return;
 
-	if (options->skip_stat_unmatch && !options->find_copies_harder)
+	if (options->skip_stat_unmatch && !DIFF_OPT_TST(options, FIND_COPIES_HARDER))
 		diffcore_skip_stat_unmatch(options);
 	if (options->break_opt != -1)
 		diffcore_break(options->break_opt);
@@ -3113,7 +3107,10 @@ void diffcore_std(struct diff_options *options)
 	diff_resolve_rename_copy();
 	diffcore_apply_filter(options->filter);
 
-	options->has_changes = !!diff_queued_diff.nr;
+	if (diff_queued_diff.nr)
+		DIFF_OPT_SET(options, HAS_CHANGES);
+	else
+		DIFF_OPT_CLR(options, HAS_CHANGES);
 }
 
 
@@ -3137,7 +3134,7 @@ void diff_addremove(struct diff_options *options,
 	 * Before the final output happens, they are pruned after
 	 * merged into rename/copy pairs as appropriate.
 	 */
-	if (options->reverse_diff)
+	if (DIFF_OPT_TST(options, REVERSE_DIFF))
 		addremove = (addremove == '+' ? '-' :
 			     addremove == '-' ? '+' : addremove);
 
@@ -3152,7 +3149,7 @@ void diff_addremove(struct diff_options *options,
 		fill_filespec(two, sha1, mode);
 
 	diff_queue(&diff_queued_diff, one, two);
-	options->has_changes = 1;
+	DIFF_OPT_SET(options, HAS_CHANGES);
 }
 
 void diff_change(struct diff_options *options,
@@ -3164,7 +3161,7 @@ void diff_change(struct diff_options *options,
 	char concatpath[PATH_MAX];
 	struct diff_filespec *one, *two;
 
-	if (options->reverse_diff) {
+	if (DIFF_OPT_TST(options, REVERSE_DIFF)) {
 		unsigned tmp;
 		const unsigned char *tmp_c;
 		tmp = old_mode; old_mode = new_mode; new_mode = tmp;
@@ -3178,7 +3175,7 @@ void diff_change(struct diff_options *options,
 	fill_filespec(two, new_sha1, new_mode);
 
 	diff_queue(&diff_queued_diff, one, two);
-	options->has_changes = 1;
+	DIFF_OPT_SET(options, HAS_CHANGES);
 }
 
 void diff_unmerge(struct diff_options *options,
diff --git a/diff.h b/diff.h
index 4546aad..6ff2b0e 100644
--- a/diff.h
+++ b/diff.h
@@ -43,26 +43,32 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,
 
 #define DIFF_FORMAT_CALLBACK	0x1000
 
+#define DIFF_OPT_RECURSIVE           (1 <<  0)
+#define DIFF_OPT_TREE_IN_RECURSIVE   (1 <<  1)
+#define DIFF_OPT_BINARY              (1 <<  2)
+#define DIFF_OPT_TEXT                (1 <<  3)
+#define DIFF_OPT_FULL_INDEX          (1 <<  4)
+#define DIFF_OPT_SILENT_ON_REMOVE    (1 <<  5)
+#define DIFF_OPT_FIND_COPIES_HARDER  (1 <<  6)
+#define DIFF_OPT_FOLLOW_RENAMES      (1 <<  7)
+#define DIFF_OPT_COLOR_DIFF          (1 <<  8)
+#define DIFF_OPT_COLOR_DIFF_WORDS    (1 <<  9)
+#define DIFF_OPT_HAS_CHANGES         (1 << 10)
+#define DIFF_OPT_QUIET               (1 << 11)
+#define DIFF_OPT_NO_INDEX            (1 << 12)
+#define DIFF_OPT_ALLOW_EXTERNAL      (1 << 13)
+#define DIFF_OPT_EXIT_WITH_STATUS    (1 << 14)
+#define DIFF_OPT_REVERSE_DIFF        (1 << 15)
+#define DIFF_OPT_TST(opts, flag)    ((opts)->flags & DIFF_OPT_##flag)
+#define DIFF_OPT_SET(opts, flag)    ((opts)->flags |= DIFF_OPT_##flag)
+#define DIFF_OPT_CLR(opts, flag)    ((opts)->flags &= ~DIFF_OPT_##flag)
+
 struct diff_options {
 	const char *filter;
 	const char *orderfile;
 	const char *pickaxe;
 	const char *single_follow;
-	unsigned recursive:1,
-		 tree_in_recursive:1,
-		 binary:1,
-		 text:1,
-		 full_index:1,
-		 silent_on_remove:1,
-		 find_copies_harder:1,
-		 follow_renames:1,
-		 color_diff:1,
-		 color_diff_words:1,
-		 has_changes:1,
-		 quiet:1,
-		 no_index:1,
-		 allow_external:1,
-		 exit_with_status:1;
+	unsigned flags;
 	int context;
 	int break_opt;
 	int detect_rename;
@@ -71,7 +77,6 @@ struct diff_options {
 	int output_format;
 	int pickaxe_opts;
 	int rename_score;
-	int reverse_diff;
 	int rename_limit;
 	int setup;
 	int abbrev;
@@ -105,6 +110,9 @@ enum color_diff {
 	DIFF_WHITESPACE = 7,
 };
 const char *diff_get_color(int diff_use_color, enum color_diff ix);
+#define diff_get_color_opt(o, ix) \
+	diff_get_color(DIFF_OPT_TST((o), COLOR_DIFF), ix)
+
 
 extern const char mime_boundary_leader[];
 
diff --git a/log-tree.c b/log-tree.c
index a34beb0..1f3fcf1 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -245,8 +245,7 @@ void show_log(struct rev_info *opt, const char *sep)
 			opt->diffopt.stat_sep = buffer;
 		}
 	} else if (opt->commit_format != CMIT_FMT_USERFORMAT) {
-		fputs(diff_get_color(opt->diffopt.color_diff, DIFF_COMMIT),
-		      stdout);
+		fputs(diff_get_color_opt(&opt->diffopt, DIFF_COMMIT), stdout);
 		if (opt->commit_format != CMIT_FMT_ONELINE)
 			fputs("commit ", stdout);
 		if (commit->object.flags & BOUNDARY)
@@ -266,8 +265,7 @@ void show_log(struct rev_info *opt, const char *sep)
 			       diff_unique_abbrev(parent->object.sha1,
 						  abbrev_commit));
 		show_decorations(commit);
-		printf("%s",
-		       diff_get_color(opt->diffopt.color_diff, DIFF_RESET));
+		printf("%s", diff_get_color_opt(&opt->diffopt, DIFF_RESET));
 		putchar(opt->commit_format == CMIT_FMT_ONELINE ? ' ' : '\n');
 		if (opt->reflog_info) {
 			show_reflog_message(opt->reflog_info,
diff --git a/merge-recursive.c b/merge-recursive.c
index 6c6f595..9a1e2f2 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -366,7 +366,7 @@ static struct path_list *get_renames(struct tree *tree,
 
 	renames = xcalloc(1, sizeof(struct path_list));
 	diff_setup(&opts);
-	opts.recursive = 1;
+	DIFF_OPT_SET(&opts, RECURSIVE);
 	opts.detect_rename = DIFF_DETECT_RENAME;
 	opts.rename_limit = rename_limit;
 	opts.output_format = DIFF_FORMAT_NO_OUTPUT;
diff --git a/patch-ids.c b/patch-ids.c
index a288fac..3be5d31 100644
--- a/patch-ids.c
+++ b/patch-ids.c
@@ -121,7 +121,7 @@ int init_patch_ids(struct patch_ids *ids)
 {
 	memset(ids, 0, sizeof(*ids));
 	diff_setup(&ids->diffopts);
-	ids->diffopts.recursive = 1;
+	DIFF_OPT_SET(&ids->diffopts, RECURSIVE);
 	if (diff_setup_done(&ids->diffopts) < 0)
 		return error("diff_setup_done failed");
 	return 0;
diff --git a/revision.c b/revision.c
index 931f978..040214f 100644
--- a/revision.c
+++ b/revision.c
@@ -252,7 +252,7 @@ static void file_add_remove(struct diff_options *options,
 	}
 	tree_difference = diff;
 	if (tree_difference == REV_TREE_DIFFERENT)
-		options->has_changes = 1;
+		DIFF_OPT_SET(options, HAS_CHANGES);
 }
 
 static void file_change(struct diff_options *options,
@@ -262,7 +262,7 @@ static void file_change(struct diff_options *options,
 		 const char *base, const char *path)
 {
 	tree_difference = REV_TREE_DIFFERENT;
-	options->has_changes = 1;
+	DIFF_OPT_SET(options, HAS_CHANGES);
 }
 
 static int rev_compare_tree(struct rev_info *revs, struct tree *t1, struct tree *t2)
@@ -272,7 +272,7 @@ static int rev_compare_tree(struct rev_info *revs, struct tree *t1, struct tree
 	if (!t2)
 		return REV_TREE_DIFFERENT;
 	tree_difference = REV_TREE_SAME;
-	revs->pruning.has_changes = 0;
+	DIFF_OPT_CLR(&revs->pruning, HAS_CHANGES);
 	if (diff_tree_sha1(t1->object.sha1, t2->object.sha1, "",
 			   &revs->pruning) < 0)
 		return REV_TREE_DIFFERENT;
@@ -296,7 +296,7 @@ static int rev_same_tree_as_empty(struct rev_info *revs, struct tree *t1)
 	init_tree_desc(&empty, "", 0);
 
 	tree_difference = REV_TREE_SAME;
-	revs->pruning.has_changes = 0;
+	DIFF_OPT_CLR(&revs->pruning, HAS_CHANGES);
 	retval = diff_tree(&empty, &real, "", &revs->pruning);
 	free(tree);
 
@@ -688,8 +688,8 @@ void init_revisions(struct rev_info *revs, const char *prefix)
 	revs->abbrev = DEFAULT_ABBREV;
 	revs->ignore_merges = 1;
 	revs->simplify_history = 1;
-	revs->pruning.recursive = 1;
-	revs->pruning.quiet = 1;
+	DIFF_OPT_SET(&revs->pruning, RECURSIVE);
+	DIFF_OPT_SET(&revs->pruning, QUIET);
 	revs->pruning.add_remove = file_add_remove;
 	revs->pruning.change = file_change;
 	revs->lifo = 1;
@@ -1086,13 +1086,13 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 			}
 			if (!strcmp(arg, "-r")) {
 				revs->diff = 1;
-				revs->diffopt.recursive = 1;
+				DIFF_OPT_SET(&revs->diffopt, RECURSIVE);
 				continue;
 			}
 			if (!strcmp(arg, "-t")) {
 				revs->diff = 1;
-				revs->diffopt.recursive = 1;
-				revs->diffopt.tree_in_recursive = 1;
+				DIFF_OPT_SET(&revs->diffopt, RECURSIVE);
+				DIFF_OPT_SET(&revs->diffopt, TREE_IN_RECURSIVE);
 				continue;
 			}
 			if (!strcmp(arg, "-m")) {
@@ -1274,7 +1274,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 		revs->diff = 1;
 
 	/* Pickaxe and rename following needs diffs */
-	if (revs->diffopt.pickaxe || revs->diffopt.follow_renames)
+	if (revs->diffopt.pickaxe || DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES))
 		revs->diff = 1;
 
 	if (revs->topo_order)
@@ -1283,7 +1283,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 	if (revs->prune_data) {
 		diff_tree_setup_paths(revs->prune_data, &revs->pruning);
 		/* Can't prune commits with rename following: the paths change.. */
-		if (!revs->diffopt.follow_renames)
+		if (!DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES))
 			revs->prune = 1;
 		if (!revs->full_diff)
 			diff_tree_setup_paths(revs->prune_data, &revs->diffopt);
diff --git a/tree-diff.c b/tree-diff.c
index 7c261fd..aa0a100 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -39,7 +39,7 @@ static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2, const
 		show_entry(opt, "+", t2, base, baselen);
 		return 1;
 	}
-	if (!opt->find_copies_harder && !hashcmp(sha1, sha2) && mode1 == mode2)
+	if (!DIFF_OPT_TST(opt, FIND_COPIES_HARDER) && !hashcmp(sha1, sha2) && mode1 == mode2)
 		return 0;
 
 	/*
@@ -52,10 +52,10 @@ static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2, const
 		return 0;
 	}
 
-	if (opt->recursive && S_ISDIR(mode1)) {
+	if (DIFF_OPT_TST(opt, RECURSIVE) && S_ISDIR(mode1)) {
 		int retval;
 		char *newbase = malloc_base(base, baselen, path1, pathlen1);
-		if (opt->tree_in_recursive)
+		if (DIFF_OPT_TST(opt, TREE_IN_RECURSIVE))
 			opt->change(opt, mode1, mode2,
 				    sha1, sha2, base, path1);
 		retval = diff_tree_sha1(sha1, sha2, newbase, opt);
@@ -206,7 +206,7 @@ static void show_entry(struct diff_options *opt, const char *prefix, struct tree
 	const char *path;
 	const unsigned char *sha1 = tree_entry_extract(desc, &path, &mode);
 
-	if (opt->recursive && S_ISDIR(mode)) {
+	if (DIFF_OPT_TST(opt, RECURSIVE) && S_ISDIR(mode)) {
 		enum object_type type;
 		int pathlen = tree_entry_len(path, sha1);
 		char *newbase = malloc_base(base, baselen, path, pathlen);
@@ -257,7 +257,7 @@ int diff_tree(struct tree_desc *t1, struct tree_desc *t2, const char *base, stru
 	int baselen = strlen(base);
 
 	for (;;) {
-		if (opt->quiet && opt->has_changes)
+		if (DIFF_OPT_TST(opt, QUIET) && DIFF_OPT_TST(opt, HAS_CHANGES))
 			break;
 		if (opt->nr_paths) {
 			skip_uninteresting(t1, base, baselen, opt);
@@ -315,7 +315,7 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, co
 	q->nr = 0;
 
 	diff_setup(&diff_opts);
-	diff_opts.recursive = 1;
+	DIFF_OPT_SET(&diff_opts, RECURSIVE);
 	diff_opts.detect_rename = DIFF_DETECT_RENAME;
 	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
 	diff_opts.single_follow = opt->paths[0];
@@ -380,7 +380,7 @@ int diff_tree_sha1(const unsigned char *old, const unsigned char *new, const cha
 	init_tree_desc(&t1, tree1, size1);
 	init_tree_desc(&t2, tree2, size2);
 	retval = diff_tree(&t1, &t2, base, opt);
-	if (opt->follow_renames && diff_might_be_rename()) {
+	if (DIFF_OPT_TST(opt, FOLLOW_RENAMES) && diff_might_be_rename()) {
 		init_tree_desc(&t1, tree1, size1);
 		init_tree_desc(&t2, tree2, size2);
 		try_to_follow_renames(&t1, &t2, base, opt);
-- 
1.5.3.5.1630.g4b3b4

^ permalink raw reply related

* Re: [PATCH] for-each-ref: fix setup of option-parsing for --sort
From: Junio C Hamano @ 2007-11-10 18:58 UTC (permalink / raw)
  To: Lars Hjemli; +Cc: Johannes Schindelin, Jakub Narebski, Jon Smirl, git
In-Reply-To: <1194713274-31200-1-git-send-email-hjemli@gmail.com>

Lars Hjemli <hjemli@gmail.com> writes:

> The option value for --sort is already a pointer to a pointer to struct
> ref_sort, so just use it.
>
> Signed-off-by: Lars Hjemli <hjemli@gmail.com>
> ---
>
> On Nov 10, 2007 5:25 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>> Could you add a test for that too, please?
>
> Is this ok?
>

Testing "for that" would be kind of hard and semi pointless,
isn't it?

If it's mismatch of the expected number of times a pointer is
dereferenced between the caller and the callee, I'd imagine that
it will read from and write to random place in memory and would
lead to unpredictable behaviour.  If you are lucky you would not
get expected results but if you are unlucky who knows what would
happen.

But the new test makes sure --sort takes intended effect, which
is a good thing.

Thanks.

>  builtin-for-each-ref.c  |    2 +-
>  t/t6300-for-each-ref.sh |   22 ++++++++++++++++++++++
>  2 files changed, 23 insertions(+), 1 deletions(-)
>
> diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
> index da8c794..e909e66 100644
> --- a/builtin-for-each-ref.c
> +++ b/builtin-for-each-ref.c
> @@ -847,7 +847,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
>  		OPT_GROUP(""),
>  		OPT_INTEGER( 0 , "count", &maxcount, "show only <n> matched refs"),
>  		OPT_STRING(  0 , "format", &format, "format", "format to use for the output"),
> -		OPT_CALLBACK(0 , "sort", &sort_tail, "key",
> +		OPT_CALLBACK(0 , "sort", sort_tail, "key",
>  		            "field name to sort on", &opt_parse_sort),
>  		OPT_END(),
>  	};
> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
> index d0809eb..c722635 100755
> --- a/t/t6300-for-each-ref.sh
> +++ b/t/t6300-for-each-ref.sh
> @@ -148,4 +148,26 @@ test_expect_success 'Check format "rfc2822" date fields output' '
>  	git diff expected actual
>  '
>  
> +cat >expected <<\EOF
> +refs/heads/master
> +refs/tags/testtag
> +EOF
> +
> +test_expect_success 'Verify ascending sort' '
> +	git-for-each-ref --format="%(refname)" --sort=refname >actual &&
> +	git diff expected actual
> +'
> +
> +
> +cat >expected <<\EOF
> +refs/tags/testtag
> +refs/heads/master
> +EOF
> +
> +test_expect_success 'Verify descending sort' '
> +	git-for-each-ref --format="%(refname)" --sort=-refname >actual &&
> +	git diff expected actual
> +'
> +
> +
>  test_done
> -- 
> 1.5.3.5.623.g0a1d

^ permalink raw reply

* Re: [PATCH] Make GIT_INDEX_FILE apply to git-commit
From: Junio C Hamano @ 2007-11-10 18:45 UTC (permalink / raw)
  To: Remi Vanicat; +Cc: git
In-Reply-To: <6b8a91420711101038t3b2ca647v422f81d9365dd05d@mail.gmail.com>

"Remi Vanicat" <vanicat@debian.org> writes:

> 2007/11/10, Junio C Hamano <gitster@pobox.com>:
>> Sounds sensible.  Tests?
>>
>
> all test pass

Sorry for being too terse.  I did not mean existing tests.

Additional tests to make sure your fix will not be broken by
other people who touch the codepath later are requested.

^ permalink raw reply

* Re: git-branch silently ignores --track on local branches
From: Junio C Hamano @ 2007-11-10 18:43 UTC (permalink / raw)
  To: Wayne Davison; +Cc: git, Johannes Schindelin
In-Reply-To: <20071110174557.GC1036@blorf.net>

Wayne Davison <wayne@opencoder.net> writes:

> ...  Is there
> a problem with local branches being supported when explicitly
> requested?

Maybe this one?

commit 6f084a56fcb3543d88d252bb49c1d2bbf2bd0cf3
Author: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Date:   Tue Jul 10 18:50:44 2007 +0100

    branch --track: code cleanup and saner handling of local branches
    
    This patch cleans up some complicated code, and replaces it with a
    cleaner version, using code from remote.[ch], which got extended a
    little in the process.  This also enables us to fix two cases:
    
    The earlier "fix" to setup tracking only when the original ref started
    with "refs/remotes" is wrong.  You are absolutely allowed to use a
    separate layout for your tracking branches.  The correct fix, of course,
    is to set up tracking information only when there is a matching
    remote.<nick>.fetch line containing a colon.
    
    Another corner case was not handled properly.  If two remotes write to
    the original ref, just warn the user and do not set up tracking.
    
    Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>

As a local branch does not have to be "fetched", the restriction
on "remote.<nick>.fetch" is sort of pointless.

Also why remote.<nick>.fetch needs a colon, I begin to wonder.
You can be keep fetching and merging from the same branch of the
same remote without keeping a remote tracking branch for that,
but the above "correct fix" forbids that.

Dscho, what were we smoking when we made this change?

^ permalink raw reply

* Re: [PATCH] Make GIT_INDEX_FILE apply to git-commit
From: Remi Vanicat @ 2007-11-10 18:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vode2mljf.fsf@gitster.siamese.dyndns.org>

2007/11/10, Junio C Hamano <gitster@pobox.com>:
> Sounds sensible.  Tests?
>

all test pass

^ permalink raw reply

* Re: git packs
From: bob @ 2007-11-10 18:01 UTC (permalink / raw)
  To: git
In-Reply-To: <20071110174559.GA2200@old.davidb.org>

It is fairly disappointing as far as indicating the problem.  Here is  
the entire report since it was so short.


============ Begin =================
Process:         git [82703]
Path:            git
Identifier:      git
Version:         ??? (???)
Code Type:       X86-64 (Native)
Parent Process:  bash [261]

Date/Time:       2007-11-10 11:23:33.976 -0500
OS Version:      Mac OS X 10.5 (9A581)
Report Version:  6

Exception Type:  EXC_BAD_ACCESS (SIGBUS)
Exception Codes: KERN_PROTECTION_FAILURE at 0x00007fff5fc00000
Crashed Thread:  Unknown

Error Formulating Crash Report:
*** -[NSCFDictionary setObject:forKey:]: attempt to insert nil value  
(key: VMUSignaturePath)
0x9501626b
0x9586009b
0x9501604b
0x9501608a
0x900d65e8
0x00078308
0x00087eb8
0x0008800e
0x000850a2
0x00002d90
0x000093be
0x0000b57c
0x0000b0c9
0x919f8793
0x0000a7d0
0x919cc075
0x919cbf32

Backtrace not available

Unknown thread crashed with X86 Thread State (64-bit):
   rax: 0x000000000000003b  rbx: 0x00000001191c9338  rcx:  
0x0000000000000392  rdx: 0x00000000aff4bc3b
   rdi: 0x00007fff5fc00000  rsi: 0x0000000040000003  rbp:  
0x00007fff5fbff180  rsp: 0x00007fff5fbff160
    r8: 0x0000000000000000   r9: 0x058487f0858487f0  r10:  
0x000000002cb27436  r11: 0x000000000ee2afe3
   r12: 0x00000000181c84c0  r13: 0x00007fff5fbff1a0  r14:  
0x000000010000000c  r15: 0x0000000101000000
   rip: 0x00007fff80543ca5  rfl: 0x0000000000010202  cr2:  
0x00007fff5fc00000

Binary images description not available
============ end =================

Maybe there was another option to specifiy other than "-m64 -arch  
x86_64".


On Nov 10, 2007, at 12:45 PM, David Brown wrote:

> On Sat, Nov 10, 2007 at 12:40:16PM -0500, bob wrote:
>
>> I am guessing that the "Bus error" is an Apple
>> problem and it did produce a crashreport.  So,
>> I am going to submit it to Apple since it is easily
>> reproducible.
>
> The crash report is probably not all that useful to Apple, since it  
> occurs
> in a program you have built.  Perhaps you should send the stack  
> trace from
> the crash report here and people might have some ideas about what  
> might be
> the problem.
>
> David
> -
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* git-branch silently ignores --track on local branches
From: Wayne Davison @ 2007-11-10 17:45 UTC (permalink / raw)
  To: git

I used to be able to create a branch that tracked master (or another
local branch) by using "git branch --track new-branch" from that
checked-out branch.  However, this functionality was apparently
removed and now the --track option is silently ignored for local
branches.  I'd love to have this functionality restored.  Is there
a problem with local branches being supported when explicitly
requested?

..wayne..

^ permalink raw reply

* Re: git packs
From: David Brown @ 2007-11-10 17:45 UTC (permalink / raw)
  To: bob; +Cc: git
In-Reply-To: <00593593-E943-4DA0-AA9B-FDBB866E7EFB@mac.com>

On Sat, Nov 10, 2007 at 12:40:16PM -0500, bob wrote:

> I am guessing that the "Bus error" is an Apple
> problem and it did produce a crashreport.  So,
> I am going to submit it to Apple since it is easily
> reproducible.

The crash report is probably not all that useful to Apple, since it occurs
in a program you have built.  Perhaps you should send the stack trace from
the crash report here and people might have some ideas about what might be
the problem.

David

^ permalink raw reply

* Re: git packs
From: bob @ 2007-11-10 17:40 UTC (permalink / raw)
  To: git
In-Reply-To: <F6DD8DCD-416B-4DDF-B384-7213C9ED5565@mac.com>

I compiled git under MacOSX 10.4.10 withr:

A) -m64 -arch ppc64 on a dual G5
B) -m64 -arch x86_64 on the dual quad-core

In both case, the link phase failed because there
was no 64-bit version of libz, libssl, libiconv and
libcrypto to link with.

I then installed MacOSX 10.5 (Leopard) which
was just released last month on the dual
quad-core machine with -m64 -arch x86_64.
git compiled and linked successfully.  However,
it failed in the "git add ." which was the
second command after "git init".  The message
was fairly cryptic, "Bus error".

I am guessing that the "Bus error" is an Apple
problem and it did produce a crashreport.  So,
I am going to submit it to Apple since it is easily
reproducible.

Anyway, those are the results.

I am now going to split out the jpg(s), pdf(s)
and (.mov) files from the repository  and just
manage them external to the git repository
which will fix my git problem.

I will report again when I get git running on
Leopard.

^ permalink raw reply

* [PATCH] for-each-ref: fix setup of option-parsing for --sort
From: Lars Hjemli @ 2007-11-10 16:47 UTC (permalink / raw)
  To: Johannes Schindelin, Junio C Hamano; +Cc: Jakub Narebski, Jon Smirl, git

The option value for --sort is already a pointer to a pointer to struct
ref_sort, so just use it.

Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---

On Nov 10, 2007 5:25 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Could you add a test for that too, please?

Is this ok?


 builtin-for-each-ref.c  |    2 +-
 t/t6300-for-each-ref.sh |   22 ++++++++++++++++++++++
 2 files changed, 23 insertions(+), 1 deletions(-)

diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
index da8c794..e909e66 100644
--- a/builtin-for-each-ref.c
+++ b/builtin-for-each-ref.c
@@ -847,7 +847,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 		OPT_GROUP(""),
 		OPT_INTEGER( 0 , "count", &maxcount, "show only <n> matched refs"),
 		OPT_STRING(  0 , "format", &format, "format", "format to use for the output"),
-		OPT_CALLBACK(0 , "sort", &sort_tail, "key",
+		OPT_CALLBACK(0 , "sort", sort_tail, "key",
 		            "field name to sort on", &opt_parse_sort),
 		OPT_END(),
 	};
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index d0809eb..c722635 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -148,4 +148,26 @@ test_expect_success 'Check format "rfc2822" date fields output' '
 	git diff expected actual
 '
 
+cat >expected <<\EOF
+refs/heads/master
+refs/tags/testtag
+EOF
+
+test_expect_success 'Verify ascending sort' '
+	git-for-each-ref --format="%(refname)" --sort=refname >actual &&
+	git diff expected actual
+'
+
+
+cat >expected <<\EOF
+refs/tags/testtag
+refs/heads/master
+EOF
+
+test_expect_success 'Verify descending sort' '
+	git-for-each-ref --format="%(refname)" --sort=-refname >actual &&
+	git diff expected actual
+'
+
+
 test_done
-- 
1.5.3.5.623.g0a1d

^ permalink raw reply related

* Re: [PATCH] for-each-ref: fix setup of option-parsing for --sort
From: Johannes Schindelin @ 2007-11-10 16:25 UTC (permalink / raw)
  To: Lars Hjemli; +Cc: Junio C Hamano, git, Jakub Narebski, Jon Smirl
In-Reply-To: <1194710863-22868-1-git-send-email-hjemli@gmail.com>

Hi,

On Sat, 10 Nov 2007, Lars Hjemli wrote:

> The option value for --sort is already a pointer to a pointer to struct
> ref_sort, so just use it.

Could you add a test for that too, please?

Thanks,
Dscho

^ permalink raw reply

* Re: [PATCH 2/2] --pretty=format: on-demand format expansion
From: René Scharfe @ 2007-11-10 16:24 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Jeff King, Junio C Hamano, Paul Mackerras, Git Mailing List,
	Pierre Habouzit
In-Reply-To: <Pine.LNX.4.64.0711101605560.4362@racer.site>

Johannes Schindelin schrieb:
> Hi,
> 
> On Sat, 10 Nov 2007, René Scharfe wrote:
> 
>> [...] have cooked up a different one on top of a cleaned up version of 
>> mine.  It plays the dirty trick of reading expansions of repeated 
>> placeholders from the strbuf..
> 
> ... which would not work (likely even segfault) if you work with the same 
> private data on different strbufs.
> 
> But I guess it will not matter much in practice.

Only a single strbuf is used, and the function that copies the data
around, strbuf_adddup(), operates on a single strbuf, only.  Copying
data between two strbufs using strbuf_add() etc. would be safe.

What one should *not* do is this:

	strbuf_add(sb, sb->buf + offset, length);

This leads to problems when the buffer is realloc()ated by strbuf_add().

What other things can go wrong?  A segfault would definitely matter..

Thanks,
René

^ permalink raw reply

* Re: [PATCH 2/2] --pretty=format: on-demand format expansion
From: Johannes Schindelin @ 2007-11-10 16:07 UTC (permalink / raw)
  To: René Scharfe
  Cc: Jeff King, Junio C Hamano, Paul Mackerras, Git Mailing List,
	Pierre Habouzit
In-Reply-To: <47359221.7090707@lsrfire.ath.cx>

Hi,

On Sat, 10 Nov 2007, Ren? Scharfe wrote:

> [...] have cooked up a different one on top of a cleaned up version of 
> mine.  It plays the dirty trick of reading expansions of repeated 
> placeholders from the strbuf..

... which would not work (likely even segfault) if you work with the same 
private data on different strbufs.

But I guess it will not matter much in practice.

Ciao,
Dscho

^ permalink raw reply

* [PATCH] for-each-ref: fix setup of option-parsing for --sort
From: Lars Hjemli @ 2007-11-10 16:07 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jakub Narebski, Jon Smirl
In-Reply-To: <9e4733910711100610y478c62cend1d9af84e0ecc08b@mail.gmail.com>

The option value for --sort is already a pointer to a pointer to struct
ref_sort, so just use it.

Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---
 builtin-for-each-ref.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
index da8c794..e909e66 100644
--- a/builtin-for-each-ref.c
+++ b/builtin-for-each-ref.c
@@ -847,7 +847,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 		OPT_GROUP(""),
 		OPT_INTEGER( 0 , "count", &maxcount, "show only <n> matched refs"),
 		OPT_STRING(  0 , "format", &format, "format", "format to use for the output"),
-		OPT_CALLBACK(0 , "sort", &sort_tail, "key",
+		OPT_CALLBACK(0 , "sort", sort_tail, "key",
 		            "field name to sort on", &opt_parse_sort),
 		OPT_END(),
 	};
-- 
1.5.3.5.623.g0a1d

^ permalink raw reply related

* [PATCH] builtin-commit: fix --signoff
From: Johannes Schindelin @ 2007-11-10 15:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, krh
In-Reply-To: <Pine.LNX.4.64.0711101346120.4362@racer.site>


The Signed-off-by: line contained a spurious timestamp.  The reason was
a call to git_committer_info(1), which automatically added the
timestamp.

Instead, fmt_ident() was taught to interpret an empty string for the
date (as opposed to NULL, which still triggers the default behavior)
as "do not bother with the timestamp", and builtin-commit.c uses it.

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

	On Sat, 10 Nov 2007, Johannes Schindelin wrote:

	> Will redo the patch.

	And here it is.

 builtin-commit.c  |   29 ++++++++++++++++++-----------
 ident.c           |   10 +++++++---
 t/t7500-commit.sh |   13 +++++++++++++
 3 files changed, 38 insertions(+), 14 deletions(-)

diff --git a/builtin-commit.c b/builtin-commit.c
index e8bc4c4..a4d69ad 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -181,21 +181,28 @@ static int prepare_log_message(const char *index_file, const char *prefix)
 		die("could not open %s\n", git_path(commit_editmsg));
 
 	stripspace(&sb, 0);
-	if (fwrite(sb.buf, 1, sb.len, fp) < sb.len)
-		die("could not write commit template: %s\n",
-		    strerror(errno));
 
 	if (signoff) {
-		const char *info, *bol;
-
-		info = git_committer_info(1);
-		strbuf_addch(&sb, '\0');
-		bol = strrchr(sb.buf + sb.len - 1, '\n');
-		if (!bol || prefixcmp(bol, sign_off_header))
-			fprintf(fp, "\n");
-		fprintf(fp, "%s%s\n", sign_off_header, git_committer_info(1));
+		struct strbuf sob;
+		int i;
+
+		strbuf_init(&sob, 0);
+		strbuf_addstr(&sob, sign_off_header);
+		strbuf_addstr(&sob, fmt_ident(getenv("GIT_COMMITTER_NAME"),
+                         getenv("GIT_COMMITTER_EMAIL"), "", 1));
+		strbuf_addch(&sob, '\n');
+
+		for (i = sb.len - 1; i > 0 && sb.buf[i - 1] != '\n'; i--)
+			; /* do nothing */
+		if (prefixcmp(sb.buf + i, sob.buf))
+			strbuf_addbuf(&sb, &sob);
+		strbuf_release(&sob);
 	}
 
+	if (fwrite(sb.buf, 1, sb.len, fp) < sb.len)
+		die("could not write commit template: %s\n",
+		    strerror(errno));
+
 	strbuf_release(&sb);
 
 	if (in_merge && !no_edit)
diff --git a/ident.c b/ident.c
index 9b2a852..5be7533 100644
--- a/ident.c
+++ b/ident.c
@@ -224,13 +224,17 @@ const char *fmt_ident(const char *name, const char *email,
 	}
 
 	strcpy(date, git_default_date);
-	if (date_str)
-		parse_date(date_str, date, sizeof(date));
+	if (date_str) {
+		if (*date_str)
+			parse_date(date_str, date, sizeof(date));
+		else
+			date[0] = '\0';
+	}
 
 	i = copy(buffer, sizeof(buffer), 0, name);
 	i = add_raw(buffer, sizeof(buffer), i, " <");
 	i = copy(buffer, sizeof(buffer), i, email);
-	i = add_raw(buffer, sizeof(buffer), i, "> ");
+	i = add_raw(buffer, sizeof(buffer), i, date[0] ? "> " : ">");
 	i = copy(buffer, sizeof(buffer), i, date);
 	if (i >= sizeof(buffer))
 		die("Impossibly long personal identifier");
diff --git a/t/t7500-commit.sh b/t/t7500-commit.sh
index abbf54b..13d5a0c 100755
--- a/t/t7500-commit.sh
+++ b/t/t7500-commit.sh
@@ -93,4 +93,17 @@ test_expect_success 'commit message from file should override template' '
 	commit_msg_is "standard input msg"
 '
 
+cat > expect << EOF
+zort
+Signed-off-by: C O Mitter <committer@example.com>
+EOF
+
+test_expect_success '--signoff' '
+	echo "yet another content *narf*" >> foo &&
+	echo "zort" |
+		GIT_EDITOR=../t7500/add-content git commit -s -F - foo &&
+	git cat-file commit HEAD | sed "1,/^$/d" > output &&
+	git diff expect output
+'
+
 test_done
-- 
1.5.3.5.1674.g6e7f7

^ permalink raw reply related

* [PATCH] test-lib.sh: move error line after error() declaration
From: Michele Ballabio @ 2007-11-10 14:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

This patch removes a spurious "command not found" error
and actually makes the "Test script did not set test_description."
string follow the command line option "--no-color".

Signed-off-by: Michele Ballabio <barra_cuda@katamail.com>
---

This is for the master branch.

 t/test-lib.sh |   15 ++++++++++-----
 1 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 603a8cd..90b6844 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -66,9 +66,6 @@ esac
 	tput sgr0 >/dev/null 2>&1 &&
 	color=t
 
-test "${test_description}" != "" ||
-error "Test script did not set test_description."
-
 while test "$#" -ne 0
 do
 	case "$1" in
@@ -77,8 +74,7 @@ do
 	-i|--i|--im|--imm|--imme|--immed|--immedi|--immedia|--immediat|--immediate)
 		immediate=t; shift ;;
 	-h|--h|--he|--hel|--help)
-		echo "$test_description"
-		exit 0 ;;
+		help=t; shift ;;
 	-v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)
 		verbose=t; shift ;;
 	-q|--q|--qu|--qui|--quie|--quiet)
@@ -124,6 +120,15 @@ say () {
 	say_color info "$*"
 }
 
+test "${test_description}" != "" ||
+error "Test script did not set test_description."
+
+if test "$help" = "t"
+then
+	echo "$test_description"
+	exit 0
+fi
+
 exec 5>&1
 if test "$verbose" = "t"
 then
-- 
1.5.3.5

^ permalink raw reply related

* Re: [PATCH 07/11] git-fetch: Limit automated tag following to only fetched objects
From: Andreas Ericsson @ 2007-11-10 14:10 UTC (permalink / raw)
  To: CJ van den Berg; +Cc: Shawn O. Pearce, Junio C Hamano, git
In-Reply-To: <20071109121228.GA4241@prefect.vdbonline.net>

CJ van den Berg wrote:
> On Fri, Nov 09, 2007 at 06:06:31AM -0500, Shawn O. Pearce wrote:
>> We now redefine the rule to be: "tags are fetched if they refer
>> to an object that was just transferred; that is an object that is
>> new to your repository".  This rule is quite simple to understand,
>> you only get a tag if you just got the object it refers to.
> 
> With this new rule a retrospectively pushed tag will never be fetched,
> right? With our local work flow tags are only ever pushed retrospectively
> because the tagged commit has to first pass regression tests. So this would
> be a major regression for us.
> 

Same for us. Deciding after something has been pushed that "ok, this version
works, let's make that one the release" is, I think, fairly common behaviour.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* Re: [PATCH REPLACEMENT for 2/2] git status: show relative paths when run in a subdirectory
From: Johannes Schindelin @ 2007-11-10 14:10 UTC (permalink / raw)
  To: Michel Marti; +Cc: git
In-Reply-To: <fh46vv$ooj$1@ger.gmane.org>

Hi,

please, please, please do not cull the Cc list.  I consider it rude to 
reply to _me_, but _address_ the mail to me, either the To: (preferred) or 
the Cc: (not so preferred).

On Sat, 10 Nov 2007, Michel Marti wrote:

> Untracked files in the current dir don't include the relative path 
> to the project-root, but changed/updated files do:
> 
> # Changes to be committed:
> #   (use "git reset HEAD <file>..." to unstage)
> #
> #       new file: ../subdir/hello
> #
> # Untracked files:
> #   (use "git add <file>..." to include in what will be committed)
> #
> #       world
> 
> With the patch below (on top of your changes), the output becomes
> 
> # Changes to be committed:
> #   (use "git reset HEAD <file>..." to unstage)
> #
> #       new file: hello
> #
> # Untracked files:
> #   (use "git add <file>..." to include in what will be committed)
> #
> #       world
> 
> Cheers,
> 
> - Michel
> 
> diff --git a/wt-status.c b/wt-status.c
> index 0d25362..2cdc8ce 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -133,8 +133,8 @@ static void wt_status_print_filepair(struct wt_status *s,
>  
>         strbuf_init(&onebuf, 0);
>         strbuf_init(&twobuf, 0);
> -       one = quote_path(p->one->path, -1, &onebuf, s->prefix);
> -       two = quote_path(p->two->path, -1, &twobuf, s->prefix);
> +       one = quote_path(p->one->path, strlen(p->one->path), &onebuf, s->prefix);
> +       two = quote_path(p->two->path, strlen(p->two->path), &twobuf, s->prefix);
>  
>         color_fprintf(s->fp, color(WT_STATUS_HEADER), "#\t");
>         switch (p->status) {
> @@ -233,7 +233,8 @@ static void wt_status_print_initial(struct wt_status *s)
>         for (i = 0; i < active_nr; i++) {
>                 color_fprintf(s->fp, color(WT_STATUS_HEADER), "#\t");
>                 color_fprintf_ln(s->fp, color(WT_STATUS_UPDATED), "new file: %s",
> -                               quote_path(active_cache[i]->name, -1,
> +                               quote_path(active_cache[i]->name,
> +                                       strlen(active_cache[i]->name),
>                                            &buf, s->prefix));
>         }
>         if (active_nr)
> 

This patch is wrong.

If you want to go that way, move the strlen() call _into_ quote_path(), 
like I had it earlier.

But then we will have a double traversal of the strings again.  That's 
what I tried to avoid, but I missed one place:

In line 94, it says "... && off < len && ...".  This should read something 
like "((len < 0 && !in[off]) || off < len)" instead.  Or maybe even "(len 
< 0 || off < len)" and have an "} else if (in[off]) off++; else break;" in 
the loop block.

Besides, you completely ignored the nice examples how other people 
contribute their patches, with mail bodies that double as a commit 
message, a diffstat, and with a test case.

Hth,
Dscho

^ permalink raw reply

* Re: gitweb, updating 'last changed' column on the project page
From: Jon Smirl @ 2007-11-10 14:10 UTC (permalink / raw)
  To: Jakub Narebski, Junio C Hamano; +Cc: git
In-Reply-To: <200711101427.18215.jnareb@gmail.com>

On 11/10/07, Jakub Narebski <jnareb@gmail.com> wrote:
> > It is sorted by committerdate, the sort is ascending. Did you expect
> > it to be descending, pick off the last entry instead of the first?
>
> Excerpts from git-for-each-ref(1):
>
>   git-for-each-ref [--count=<count>]* (...) [--sort=<key>]* (...)
>
>   <count>
>           By  default  the  command  shows all refs that match <pattern>. This
>           option makes it stop after showing that many refs.
>
>    <key>  A field name to sort on. Prefix - to sort in descending order of the
>           value.  When  unspecified,  refname is used. More than one sort keys
>           can be given.
>
> So I expect --sort=-committerdate to sort by date of committing,
> descending, and --count=1 pick first one, which means most recent.

git has a bug, it is not implementing the - prefix. I am using git head.

jonsmirl@terra:~$ cd mpc5200b
jonsmirl@terra:~/mpc5200b$ git for-each-ref
--format="%(refname):%09%(committer)" --sort=-committerdate refs/heads
refs/heads/m24: Jon Smirl <jonsmirl@gmail.com> 1191362799 -0400
refs/heads/m25: Jon Smirl <jonsmirl@gmail.com> 1191472422 -0400
refs/heads/m26: Jon Smirl <jonsmirl@gmail.com> 1194382038 -0500
refs/heads/m28: Jon Smirl <jonsmirl@gmail.com> 1194385071 -0500
refs/heads/m29: Jon Smirl <jonsmirl@gmail.com> 1194674673 -0500
jonsmirl@terra:~/mpc5200b$ git for-each-ref
--format="%(refname):%09%(committer)" --sort=committerdate refs/heads
refs/heads/m24: Jon Smirl <jonsmirl@gmail.com> 1191362799 -0400
refs/heads/m25: Jon Smirl <jonsmirl@gmail.com> 1191472422 -0400
refs/heads/m26: Jon Smirl <jonsmirl@gmail.com> 1194382038 -0500
refs/heads/m28: Jon Smirl <jonsmirl@gmail.com> 1194385071 -0500
refs/heads/m29: Jon Smirl <jonsmirl@gmail.com> 1194674673 -0500
jonsmirl@terra:~/mpc5200b$ git --version
git version 1.5.3.5.1651.g30bf
jonsmirl@terra:~/mpc5200b$


>
> It looks like "your" gitweb sorts ascending instead... strange...
>
>
> How does git_get_last_activity subroutine in your gitweb.cgi looks like?
> Does it have '--sort=-commiterdate'? If it has, then I think it is some
> strange bug in git, if it doesn't it is strange modification of gitweb.
>
> HTH
> --
> Jakub Narebski
> Poland
>


-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox