{"thread":{"id":"11880","subject":"[PATCH 5/9] Add \"skip_unmerged\" option to unpack_trees.","startedAt":"2008-02-04T18:35:54Z","lastAt":"2008-02-06T01:05:58Z","messageCount":6,"participants":["Daniel Barkalow","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":9},"messages":[{"id":"67405","messageId":"alpine.LNX.1.00.0802041335530.13593@iabervon.org","threadId":"11880","inReplyTo":null,"subject":"[PATCH 5/9] Add \"skip_unmerged\" option to unpack_trees.","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-02-04T18:35:54Z","receivedAt":"2008-02-04T18:35:54Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"This option allows the caller to reset everything that isn't unmerged,\nleaving the unmerged things to be resolved. If, after a merge of\n\"working\" and \"HEAD\", this is used with \"HEAD\" (reset, !update), the\nresult will be that all of the changes from \"local\" are in the working\ntree but not added to the index (either with the index clean but\nunchanged, or with the index unmerged, depending on whether there are\nconflicts).\n\nThis will be used in checkout -m.\n\nSigned-off-by: Daniel Barkalow <barkalow@iabervon.org>\n---\n unpack-trees.c |   20 +++++++++++++++++---\n unpack-trees.h |    1 +\n 2 files changed, 18 insertions(+), 3 deletions(-)\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 40d4130..7040fa3 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -85,6 +85,7 @@ static int unpack_trees_rec(struct tree_entry_list **posns, int len,\n \t\tint any_dirs = 0;\n \t\tchar *cache_name;\n \t\tint ce_stage;\n+\t\tint skip_entry = 0;\n \n \t\t/* Find the first name in the input. */\n \n@@ -153,6 +154,8 @@ static int unpack_trees_rec(struct tree_entry_list **posns, int len,\n \t\t\tany_files = 1;\n \t\t\tsrc[0] = active_cache[o->pos];\n \t\t\tremove = o->pos;\n+\t\t\tif (o->skip_unmerged && ce_stage(src[0]))\n+\t\t\t\tskip_entry = 1;\n \t\t}\n \n \t\tfor (i = 0; i < len; i++) {\n@@ -181,6 +184,12 @@ static int unpack_trees_rec(struct tree_entry_list **posns, int len,\n \t\t\t\tcontinue;\n \t\t\t}\n \n+\t\t\tif (skip_entry) {\n+\t\t\t\tsubposns[i] = df_conflict_list;\n+\t\t\t\tposns[i] = posns[i]->next;\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\n \t\t\tif (!o->merge)\n \t\t\t\tce_stage = 0;\n \t\t\telse if (i + 1 < o->head_idx)\n@@ -205,7 +214,13 @@ static int unpack_trees_rec(struct tree_entry_list **posns, int len,\n \t\t\tposns[i] = posns[i]->next;\n \t\t}\n \t\tif (any_files) {\n-\t\t\tif (o->merge) {\n+\t\t\tif (skip_entry) {\n+\t\t\t\tdo\n+\t\t\t\t\to->pos++;\n+\t\t\t\twhile (o->pos < active_nr &&\n+\t\t\t\t       !strcmp(active_cache[o->pos]->name,\n+\t\t\t\t\t       src[0]->name));\n+\t\t\t} else if (o->merge) {\n \t\t\t\tint ret;\n \n #if DBRT_DEBUG > 1\n@@ -730,9 +745,8 @@ int threeway_merge(struct cache_entry **stages,\n \t * If we have an entry in the index cache, then we want to\n \t * make sure that it matches head.\n \t */\n-\tif (index && !same(index, head)) {\n+\tif (index && !same(index, head))\n \t\treturn o->gently ? -1 : reject_merge(index);\n-\t}\n \n \tif (head) {\n \t\t/* #5ALT, #15 */\ndiff --git a/unpack-trees.h b/unpack-trees.h\nindex 83d1229..a2df544 100644\n--- a/unpack-trees.h\n+++ b/unpack-trees.h\n@@ -16,6 +16,7 @@ struct unpack_trees_options {\n \tint trivial_merges_only;\n \tint verbose_update;\n \tint aggressive;\n+\tint skip_unmerged;\n \tint gently;\n \tconst char *prefix;\n \tint pos;\n-- \n1.5.4\n"},{"id":"67466","messageId":"alpine.LSU.1.00.0802050129570.8543@racer.site","threadId":"11880","inReplyTo":"alpine.LNX.1.00.0802041335530.13593@iabervon.org","subject":"Re: [PATCH 5/9] Add \"skip_unmerged\" option to unpack_trees.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-02-05T01:32:08Z","receivedAt":"2008-02-05T01:32:08Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 4 Feb 2008, Daniel Barkalow wrote:\n\n\n>  \t\tif (any_files) {\n> -\t\t\tif (o->merge) {\n> +\t\t\tif (skip_entry) {\n> +\t\t\t\tdo\n> +\t\t\t\t\to->pos++;\n> +\t\t\t\twhile (o->pos < active_nr &&\n> +\t\t\t\t       !strcmp(active_cache[o->pos]->name,\n> +\t\t\t\t\t       src[0]->name));\n> +\t\t\t} else if (o->merge) {\n\nMaybe it is just me, but I would have thought\n\n\t\t\t\twhile (++o->pos < active_nr)\n\t\t\t\t\tif (strcmp(active_cache[o->pos]->name,\n\t\t\t\t\t\t\tsrc[0]->name))\n\t\t\t\t\t\tbreak;\n\nmore readable.  But that's maybe because I have trouble with do ... while \nconstructs logically (I like to see the loop condition first, then the \nloop body).\n\nCiao,\nDscho\n"},{"id":"67552","messageId":"alpine.LNX.1.00.0802042127210.13593@iabervon.org","threadId":"11880","inReplyTo":"alpine.LSU.1.00.0802050129570.8543@racer.site","subject":"Re: [PATCH 5/9] Add \"skip_unmerged\" option to unpack_trees.","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-02-05T20:38:52Z","receivedAt":"2008-02-05T20:38:52Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Tue, 5 Feb 2008, Johannes Schindelin wrote:\n\n> Hi,\n> \n> On Mon, 4 Feb 2008, Daniel Barkalow wrote:\n> \n> \n> >  \t\tif (any_files) {\n> > -\t\t\tif (o->merge) {\n> > +\t\t\tif (skip_entry) {\n> > +\t\t\t\tdo\n> > +\t\t\t\t\to->pos++;\n> > +\t\t\t\twhile (o->pos < active_nr &&\n> > +\t\t\t\t       !strcmp(active_cache[o->pos]->name,\n> > +\t\t\t\t\t       src[0]->name));\n> > +\t\t\t} else if (o->merge) {\n> \n> Maybe it is just me, but I would have thought\n> \n> \t\t\t\twhile (++o->pos < active_nr)\n> \t\t\t\t\tif (strcmp(active_cache[o->pos]->name,\n> \t\t\t\t\t\t\tsrc[0]->name))\n> \t\t\t\t\t\tbreak;\n> \n> more readable.  But that's maybe because I have trouble with do ... while \n> constructs logically (I like to see the loop condition first, then the \n> loop body).\n\nI find yours less readable, because the loop condition is an exceptional \ncase (this is the last entry, so we run out of active_cache before finding \nanything else), and you've got the actual effect of the loop in the \ncondition instead of the body, and I find using the value of ++x or x++ a \nbit confusing outside of regular idioms. I'd go for:\n\n\t\t\t\to->pos++;\n\t\t\t\twhile (o->pos < active_nr &&\n\t\t\t\t       !strcmp(active_cache[o->pos]->name,\n\t\t\t\t\t       src[0]->name))\n\t\t\t\t\to->pos++;\n\nif you care, though; it's not bad to make it clear we're skipping the \nfirst of these entries based on a different consideration from the rest \n(the first is the entry we decided to skip, and the rest are ones that \nmatch it in filename).\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"67572","messageId":"alpine.LSU.1.00.0802052249270.8543@racer.site","threadId":"11880","inReplyTo":"alpine.LNX.1.00.0802042127210.13593@iabervon.org","subject":"Re: [PATCH 5/9] Add \"skip_unmerged\" option to unpack_trees.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-02-05T22:50:17Z","receivedAt":"2008-02-05T22:50:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 5 Feb 2008, Daniel Barkalow wrote:\n\n> On Tue, 5 Feb 2008, Johannes Schindelin wrote:\n> \n> > On Mon, 4 Feb 2008, Daniel Barkalow wrote:\n> > \n> > \n> > >  \t\tif (any_files) {\n> > > -\t\t\tif (o->merge) {\n> > > +\t\t\tif (skip_entry) {\n> > > +\t\t\t\tdo\n> > > +\t\t\t\t\to->pos++;\n> > > +\t\t\t\twhile (o->pos < active_nr &&\n> > > +\t\t\t\t       !strcmp(active_cache[o->pos]->name,\n> > > +\t\t\t\t\t       src[0]->name));\n> > > +\t\t\t} else if (o->merge) {\n> > \n> > Maybe it is just me, but I would have thought\n> > \n> > \t\t\t\twhile (++o->pos < active_nr)\n> > \t\t\t\t\tif (strcmp(active_cache[o->pos]->name,\n> > \t\t\t\t\t\t\tsrc[0]->name))\n> > \t\t\t\t\t\tbreak;\n> > \n> > more readable.  But that's maybe because I have trouble with do ... \n> > while constructs logically (I like to see the loop condition first, \n> > then the loop body).\n> \n> I find yours less readable, because the loop condition is an exceptional \n> case (this is the last entry, so we run out of active_cache before \n> finding anything else), and you've got the actual effect of the loop in \n> the condition instead of the body, and I find using the value of ++x or \n> x++ a bit confusing outside of regular idioms. I'd go for:\n> \n> \t\t\t\to->pos++;\n> \t\t\t\twhile (o->pos < active_nr &&\n> \t\t\t\t       !strcmp(active_cache[o->pos]->name,\n> \t\t\t\t\t       src[0]->name))\n> \t\t\t\t\to->pos++;\n> \n> if you care, though; it's not bad to make it clear we're skipping the \n> first of these entries based on a different consideration from the rest \n> (the first is the entry we decided to skip, and the rest are ones that \n> match it in filename).\n\nWell, I still find mine more readable (because it reflects how I think, I \nguess), but I do not care deeply enough.  Let's take your original.\n\nCiao,\nDscho\n"},{"id":"67584","messageId":"7vve536kac.fsf@gitster.siamese.dyndns.org","threadId":"11880","inReplyTo":"alpine.LNX.1.00.0802042127210.13593@iabervon.org","subject":"Re: [PATCH 5/9] Add \"skip_unmerged\" option to unpack_trees.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-05T23:44:11Z","receivedAt":"2008-02-05T23:44:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow <barkalow@iabervon.org> writes:\n\n> On Tue, 5 Feb 2008, Johannes Schindelin wrote:\n>\n>> Hi,\n>> \n>> On Mon, 4 Feb 2008, Daniel Barkalow wrote:\n>> \n>> \n>> >  \t\tif (any_files) {\n>> > -\t\t\tif (o->merge) {\n>> > +\t\t\tif (skip_entry) {\n>> > +\t\t\t\tdo\n>> > +\t\t\t\t\to->pos++;\n>> > +\t\t\t\twhile (o->pos < active_nr &&\n>> > +\t\t\t\t       !strcmp(active_cache[o->pos]->name,\n>> > +\t\t\t\t\t       src[0]->name));\n>> > +\t\t\t} else if (o->merge) {\n>> \n>> Maybe it is just me, but I would have thought\n>> \n>> \t\t\t\twhile (++o->pos < active_nr)\n>> \t\t\t\t\tif (strcmp(active_cache[o->pos]->name,\n>> \t\t\t\t\t\t\tsrc[0]->name))\n>> \t\t\t\t\t\tbreak;\n>> \n>> more readable.  But that's maybe because I have trouble with do ... while \n>> constructs logically (I like to see the loop condition first, then the \n>> loop body).\n>\n> I find yours less readable, because the loop condition is an exceptional \n> case (this is the last entry, so we run out of active_cache before finding \n> anything else), and you've got the actual effect of the loop in the \n> condition instead of the body, and I find using the value of ++x or x++ a \n> bit confusing outside of regular idioms. I'd go for:\n>\n> \t\t\t\to->pos++;\n> \t\t\t\twhile (o->pos < active_nr &&\n> \t\t\t\t       !strcmp(active_cache[o->pos]->name,\n> \t\t\t\t\t       src[0]->name))\n> \t\t\t\t\to->pos++;\n>\n> if you care, though; it's not bad to make it clear we're skipping the \n> first of these entries based on a different consideration from the rest \n> (the first is the entry we decided to skip, and the rest are ones that \n> match it in filename).\n\nThe last one is vastly more readable than your original, and\nmoderately easier to follow than Dscho's, at least to me.\n\nThanks.\n"},{"id":"67597","messageId":"alpine.LNX.1.00.0802052004530.13593@iabervon.org","threadId":"11880","inReplyTo":"7vve536kac.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 5/9] Add \"skip_unmerged\" option to unpack_trees.","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-02-06T01:05:58Z","receivedAt":"2008-02-06T01:05:58Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Tue, 5 Feb 2008, Junio C Hamano wrote:\n\n> Daniel Barkalow <barkalow@iabervon.org> writes:\n> \n> > On Tue, 5 Feb 2008, Johannes Schindelin wrote:\n> >\n> >> Hi,\n> >> \n> >> On Mon, 4 Feb 2008, Daniel Barkalow wrote:\n> >> \n> >> \n> >> >  \t\tif (any_files) {\n> >> > -\t\t\tif (o->merge) {\n> >> > +\t\t\tif (skip_entry) {\n> >> > +\t\t\t\tdo\n> >> > +\t\t\t\t\to->pos++;\n> >> > +\t\t\t\twhile (o->pos < active_nr &&\n> >> > +\t\t\t\t       !strcmp(active_cache[o->pos]->name,\n> >> > +\t\t\t\t\t       src[0]->name));\n> >> > +\t\t\t} else if (o->merge) {\n> >> \n> >> Maybe it is just me, but I would have thought\n> >> \n> >> \t\t\t\twhile (++o->pos < active_nr)\n> >> \t\t\t\t\tif (strcmp(active_cache[o->pos]->name,\n> >> \t\t\t\t\t\t\tsrc[0]->name))\n> >> \t\t\t\t\t\tbreak;\n> >> \n> >> more readable.  But that's maybe because I have trouble with do ... while \n> >> constructs logically (I like to see the loop condition first, then the \n> >> loop body).\n> >\n> > I find yours less readable, because the loop condition is an exceptional \n> > case (this is the last entry, so we run out of active_cache before finding \n> > anything else), and you've got the actual effect of the loop in the \n> > condition instead of the body, and I find using the value of ++x or x++ a \n> > bit confusing outside of regular idioms. I'd go for:\n> >\n> > \t\t\t\to->pos++;\n> > \t\t\t\twhile (o->pos < active_nr &&\n> > \t\t\t\t       !strcmp(active_cache[o->pos]->name,\n> > \t\t\t\t\t       src[0]->name))\n> > \t\t\t\t\to->pos++;\n> >\n> > if you care, though; it's not bad to make it clear we're skipping the \n> > first of these entries based on a different consideration from the rest \n> > (the first is the entry we decided to skip, and the rest are ones that \n> > match it in filename).\n> \n> The last one is vastly more readable than your original, and\n> moderately easier to follow than Dscho's, at least to me.\n\nI like this version best, too, and it's what I've got in my revised \nseries.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"}]}