# [PATCH 5/9] Add "skip_unmerged" option to unpack_trees. 6 messages from 2008-02-04 to 2008-02-06. Participants: Daniel Barkalow, Johannes Schindelin, Junio C Hamano. Thread: https://gitlist.dev/t/11880 ## Daniel Barkalow, 2008-02-04 18:35 Subject: [PATCH 5/9] Add "skip_unmerged" option to unpack_trees. Message-ID: URL: https://gitlist.dev/e/alpine.LNX.1.00.0802041335530.13593%40iabervon.org ``` 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 --- 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 ``` ## Johannes Schindelin, 2008-02-05 01:32 Subject: Re: [PATCH 5/9] Add "skip_unmerged" option to unpack_trees. Message-ID: URL: https://gitlist.dev/e/alpine.LSU.1.00.0802050129570.8543%40racer.site In-Reply-To: ``` 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 ``` ## Daniel Barkalow, 2008-02-05 20:38 Subject: Re: [PATCH 5/9] Add "skip_unmerged" option to unpack_trees. Message-ID: URL: https://gitlist.dev/e/alpine.LNX.1.00.0802042127210.13593%40iabervon.org In-Reply-To: ``` 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* ``` ## Johannes Schindelin, 2008-02-05 22:50 Subject: Re: [PATCH 5/9] Add "skip_unmerged" option to unpack_trees. Message-ID: URL: https://gitlist.dev/e/alpine.LSU.1.00.0802052249270.8543%40racer.site In-Reply-To: ``` 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 ``` ## Junio C Hamano, 2008-02-05 23:44 Subject: Re: [PATCH 5/9] Add "skip_unmerged" option to unpack_trees. Message-ID: <7vve536kac.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vve536kac.fsf%40gitster.siamese.dyndns.org In-Reply-To: ``` Daniel Barkalow 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. ``` ## Daniel Barkalow, 2008-02-06 01:05 Subject: Re: [PATCH 5/9] Add "skip_unmerged" option to unpack_trees. Message-ID: URL: https://gitlist.dev/e/alpine.LNX.1.00.0802052004530.13593%40iabervon.org In-Reply-To: <7vve536kac.fsf@gitster.siamese.dyndns.org> ``` On Tue, 5 Feb 2008, Junio C Hamano wrote: > Daniel Barkalow 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* ```