[PATCH 5/9] Add "skip_unmerged" option to unpack_trees.

[PATCH 5/9] Add "skip_unmerged" option to unpack_trees.

[Daniel Barkalow]

This option allows the caller to reset everything that isn't unmerged,
leaving the unmerged things to be resolved. If, after a merge of
"working" and "HEAD", this is used with "HEAD" (reset, !update), the
result will be that all of the changes from "local" are in the working
tree but not added to the index (either with the index clean but
unchanged, or with the index unmerged, depending on whether there are
conflicts).

This will be used in checkout -m.

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
 unpack-trees.c |   20 +++++++++++++++++---
 unpack-trees.h |    1 +
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/unpack-trees.c b/unpack-trees.c
index 40d4130..7040fa3 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -85,6 +85,7 @@ static int unpack_trees_rec(struct tree_entry_list **posns, int len,
 		int any_dirs = 0;
 		char *cache_name;
 		int ce_stage;
+		int skip_entry = 0;
 
 		/* Find the first name in the input. */
 
@@ -153,6 +154,8 @@ static int unpack_trees_rec(struct tree_entry_list **posns, int len,
 			any_files = 1;
 			src[0] = active_cache[o->pos];
 			remove = o->pos;
+			if (o->skip_unmerged && ce_stage(src[0]))
+				skip_entry = 1;
 		}
 
 		for (i = 0; i < len; i++) {
@@ -181,6 +184,12 @@ static int unpack_trees_rec(struct tree_entry_list **posns, int len,
 				continue;
 			}
 
+			if (skip_entry) {
+				subposns[i] = df_conflict_list;
+				posns[i] = posns[i]->next;
+				continue;
+			}
+
 			if (!o->merge)
 				ce_stage = 0;
 			else if (i + 1 < o->head_idx)
@@ -205,7 +214,13 @@ static int unpack_trees_rec(struct tree_entry_list **posns, int len,
 			posns[i] = posns[i]->next;
 		}
 		if (any_files) {
-			if (o->merge) {
+			if (skip_entry) {
+				do
+					o->pos++;
+				while (o->pos < active_nr &&
+				       !strcmp(active_cache[o->pos]->name,
+					       src[0]->name));
+			} else if (o->merge) {
 				int ret;
 
 #if DBRT_DEBUG > 1
@@ -730,9 +745,8 @@ int threeway_merge(struct cache_entry **stages,
 	 * If we have an entry in the index cache, then we want to
 	 * make sure that it matches head.
 	 */
-	if (index && !same(index, head)) {
+	if (index && !same(index, head))
 		return o->gently ? -1 : reject_merge(index);
-	}
 
 	if (head) {
 		/* #5ALT, #15 */
diff --git a/unpack-trees.h b/unpack-trees.h
index 83d1229..a2df544 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -16,6 +16,7 @@ struct unpack_trees_options {
 	int trivial_merges_only;
 	int verbose_update;
 	int aggressive;
+	int skip_unmerged;
 	int gently;
 	const char *prefix;
 	int pos;
-- 
1.5.4

Re: [PATCH 5/9] Add "skip_unmerged" option to unpack_trees.

[Johannes Schindelin]

Hi,

On Mon, 4 Feb 2008, Daniel Barkalow wrote:


>  		if (any_files) {
> -			if (o->merge) {
> +			if (skip_entry) {
> +				do
> +					o->pos++;
> +				while (o->pos < active_nr &&
> +				       !strcmp(active_cache[o->pos]->name,
> +					       src[0]->name));
> +			} else if (o->merge) {

Maybe it is just me, but I would have thought

				while (++o->pos < active_nr)
					if (strcmp(active_cache[o->pos]->name,
							src[0]->name))
						break;

more readable.  But that's maybe because I have trouble with do ... while 
constructs logically (I like to see the loop condition first, then the 
loop body).

Ciao,
Dscho

Re: [PATCH 5/9] Add "skip_unmerged" option to unpack_trees.

[Daniel Barkalow]

On Tue, 5 Feb 2008, Johannes Schindelin wrote:

> Hi,
> 
> On Mon, 4 Feb 2008, Daniel Barkalow wrote:
> 
> 
> >  		if (any_files) {
> > -			if (o->merge) {
> > +			if (skip_entry) {
> > +				do
> > +					o->pos++;
> > +				while (o->pos < active_nr &&
> > +				       !strcmp(active_cache[o->pos]->name,
> > +					       src[0]->name));
> > +			} else if (o->merge) {
> 
> Maybe it is just me, but I would have thought
> 
> 				while (++o->pos < active_nr)
> 					if (strcmp(active_cache[o->pos]->name,
> 							src[0]->name))
> 						break;
> 
> more readable.  But that's maybe because I have trouble with do ... while 
> constructs logically (I like to see the loop condition first, then the 
> loop body).

I find yours less readable, because the loop condition is an exceptional 
case (this is the last entry, so we run out of active_cache before finding 
anything else), and you've got the actual effect of the loop in the 
condition instead of the body, and I find using the value of ++x or x++ a 
bit confusing outside of regular idioms. I'd go for:

				o->pos++;
				while (o->pos < active_nr &&
				       !strcmp(active_cache[o->pos]->name,
					       src[0]->name))
					o->pos++;

if you care, though; it's not bad to make it clear we're skipping the 
first of these entries based on a different consideration from the rest 
(the first is the entry we decided to skip, and the rest are ones that 
match it in filename).

	-Daniel
*This .sig left intentionally blank*

Re: [PATCH 5/9] Add "skip_unmerged" option to unpack_trees.

[Johannes Schindelin]

Hi,

On Tue, 5 Feb 2008, Daniel Barkalow wrote:

> On Tue, 5 Feb 2008, Johannes Schindelin wrote:
> 
> > On Mon, 4 Feb 2008, Daniel Barkalow wrote:
> > 
> > 
> > >  		if (any_files) {
> > > -			if (o->merge) {
> > > +			if (skip_entry) {
> > > +				do
> > > +					o->pos++;
> > > +				while (o->pos < active_nr &&
> > > +				       !strcmp(active_cache[o->pos]->name,
> > > +					       src[0]->name));
> > > +			} else if (o->merge) {
> > 
> > Maybe it is just me, but I would have thought
> > 
> > 				while (++o->pos < active_nr)
> > 					if (strcmp(active_cache[o->pos]->name,
> > 							src[0]->name))
> > 						break;
> > 
> > more readable.  But that's maybe because I have trouble with do ... 
> > while constructs logically (I like to see the loop condition first, 
> > then the loop body).
> 
> I find yours less readable, because the loop condition is an exceptional 
> case (this is the last entry, so we run out of active_cache before 
> finding anything else), and you've got the actual effect of the loop in 
> the condition instead of the body, and I find using the value of ++x or 
> x++ a bit confusing outside of regular idioms. I'd go for:
> 
> 				o->pos++;
> 				while (o->pos < active_nr &&
> 				       !strcmp(active_cache[o->pos]->name,
> 					       src[0]->name))
> 					o->pos++;
> 
> if you care, though; it's not bad to make it clear we're skipping the 
> first of these entries based on a different consideration from the rest 
> (the first is the entry we decided to skip, and the rest are ones that 
> match it in filename).

Well, I still find mine more readable (because it reflects how I think, I 
guess), but I do not care deeply enough.  Let's take your original.

Ciao,
Dscho

Re: [PATCH 5/9] Add "skip_unmerged" option to unpack_trees.

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

> On Tue, 5 Feb 2008, Johannes Schindelin wrote:
>
>> Hi,
>> 
>> On Mon, 4 Feb 2008, Daniel Barkalow wrote:
>> 
>> 
>> >  		if (any_files) {
>> > -			if (o->merge) {
>> > +			if (skip_entry) {
>> > +				do
>> > +					o->pos++;
>> > +				while (o->pos < active_nr &&
>> > +				       !strcmp(active_cache[o->pos]->name,
>> > +					       src[0]->name));
>> > +			} else if (o->merge) {
>> 
>> Maybe it is just me, but I would have thought
>> 
>> 				while (++o->pos < active_nr)
>> 					if (strcmp(active_cache[o->pos]->name,
>> 							src[0]->name))
>> 						break;
>> 
>> more readable.  But that's maybe because I have trouble with do ... while 
>> constructs logically (I like to see the loop condition first, then the 
>> loop body).
>
> I find yours less readable, because the loop condition is an exceptional 
> case (this is the last entry, so we run out of active_cache before finding 
> anything else), and you've got the actual effect of the loop in the 
> condition instead of the body, and I find using the value of ++x or x++ a 
> bit confusing outside of regular idioms. I'd go for:
>
> 				o->pos++;
> 				while (o->pos < active_nr &&
> 				       !strcmp(active_cache[o->pos]->name,
> 					       src[0]->name))
> 					o->pos++;
>
> if you care, though; it's not bad to make it clear we're skipping the 
> first of these entries based on a different consideration from the rest 
> (the first is the entry we decided to skip, and the rest are ones that 
> match it in filename).

The last one is vastly more readable than your original, and
moderately easier to follow than Dscho's, at least to me.

Thanks.

Re: [PATCH 5/9] Add "skip_unmerged" option to unpack_trees.

[Daniel Barkalow]

On Tue, 5 Feb 2008, Junio C Hamano wrote:

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > On Tue, 5 Feb 2008, Johannes Schindelin wrote:
> >
> >> Hi,
> >> 
> >> On Mon, 4 Feb 2008, Daniel Barkalow wrote:
> >> 
> >> 
> >> >  		if (any_files) {
> >> > -			if (o->merge) {
> >> > +			if (skip_entry) {
> >> > +				do
> >> > +					o->pos++;
> >> > +				while (o->pos < active_nr &&
> >> > +				       !strcmp(active_cache[o->pos]->name,
> >> > +					       src[0]->name));
> >> > +			} else if (o->merge) {
> >> 
> >> Maybe it is just me, but I would have thought
> >> 
> >> 				while (++o->pos < active_nr)
> >> 					if (strcmp(active_cache[o->pos]->name,
> >> 							src[0]->name))
> >> 						break;
> >> 
> >> more readable.  But that's maybe because I have trouble with do ... while 
> >> constructs logically (I like to see the loop condition first, then the 
> >> loop body).
> >
> > I find yours less readable, because the loop condition is an exceptional 
> > case (this is the last entry, so we run out of active_cache before finding 
> > anything else), and you've got the actual effect of the loop in the 
> > condition instead of the body, and I find using the value of ++x or x++ a 
> > bit confusing outside of regular idioms. I'd go for:
> >
> > 				o->pos++;
> > 				while (o->pos < active_nr &&
> > 				       !strcmp(active_cache[o->pos]->name,
> > 					       src[0]->name))
> > 					o->pos++;
> >
> > if you care, though; it's not bad to make it clear we're skipping the 
> > first of these entries based on a different consideration from the rest 
> > (the first is the entry we decided to skip, and the rest are ones that 
> > match it in filename).
> 
> The last one is vastly more readable than your original, and
> moderately easier to follow than Dscho's, at least to me.

I like this version best, too, and it's what I've got in my revised 
series.

	-Daniel
*This .sig left intentionally blank*

[PATCH 5/9] Add "skip_unmerged" option to unpack_trees.

[Daniel Barkalow]

This option allows the caller to reset everything that isn't unmerged,
leaving the unmerged things to be resolved. If, after a merge of
"working" and "HEAD", this is used with "HEAD" (reset, !update), the
result will be that all of the changes from "local" are in the working
tree but not added to the index (either with the index clean but
unchanged, or with the index unmerged, depending on whether there are
conflicts).

This will be used in checkout -m.

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
 unpack-trees.c |   20 +++++++++++++++++---
 unpack-trees.h |    1 +
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/unpack-trees.c b/unpack-trees.c
index ac919c7..e552167 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -85,6 +85,7 @@ static int unpack_trees_rec(struct tree_entry_list **posns, int len,
 		int any_dirs = 0;
 		char *cache_name;
 		int ce_stage;
+		int skip_entry = 0;
 
 		/* Find the first name in the input. */
 
@@ -153,6 +154,8 @@ static int unpack_trees_rec(struct tree_entry_list **posns, int len,
 			any_files = 1;
 			src[0] = active_cache[o->pos];
 			remove = o->pos;
+			if (o->skip_unmerged && ce_stage(src[0]))
+				skip_entry = 1;
 		}
 
 		for (i = 0; i < len; i++) {
@@ -181,6 +184,12 @@ static int unpack_trees_rec(struct tree_entry_list **posns, int len,
 				continue;
 			}
 
+			if (skip_entry) {
+				subposns[i] = df_conflict_list;
+				posns[i] = posns[i]->next;
+				continue;
+			}
+
 			if (!o->merge)
 				ce_stage = 0;
 			else if (i + 1 < o->head_idx)
@@ -205,7 +214,13 @@ static int unpack_trees_rec(struct tree_entry_list **posns, int len,
 			posns[i] = posns[i]->next;
 		}
 		if (any_files) {
-			if (o->merge) {
+			if (skip_entry) {
+				do
+					o->pos++;
+				while (o->pos < active_nr &&
+				       !strcmp(active_cache[o->pos]->name,
+					       src[0]->name));
+			} else if (o->merge) {
 				int ret;
 
 #if DBRT_DEBUG > 1
@@ -731,9 +746,8 @@ int threeway_merge(struct cache_entry **stages,
 	 * If we have an entry in the index cache, then we want to
 	 * make sure that it matches head.
 	 */
-	if (index && !same(index, head)) {
+	if (index && !same(index, head))
 		return o->gently ? -1 : reject_merge(index);
-	}
 
 	if (head) {
 		/* #5ALT, #15 */
diff --git a/unpack-trees.h b/unpack-trees.h
index 619950d..39612fc 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -16,6 +16,7 @@ struct unpack_trees_options {
 	int trivial_merges_only;
 	int verbose_update;
 	int aggressive;
+	int skip_unmerged;
 	int gently;
 	const char *prefix;
 	int pos;
-- 
1.5.4.rc3.4.g16335