Re: [PATCH 2/4] merge-recursive: increase marker length with depth of recursion
On Mon, Oct 15, 2018 at 7:17 PM Junio C Hamano wrote: > > Elijah Newren writes: > > > Would you like me to edit the commit message to include this more > > difficult case? > > Neither. If the "marker length" change is required in a separate > series that will build on top of the current 4-patch series, I think > dropping this step from the current series and make it a part of the > series that deals with rename/rename would make more sense. Okay, I'll resubmit this series without this patch or associated testcase, and include those in the later file collision series.
Re: [PATCH 2/4] merge-recursive: increase marker length with depth of recursion
Elijah Newren writes: > Would you like me to edit the commit message to include this more > difficult case? Neither. If the "marker length" change is required in a separate series that will build on top of the current 4-patch series, I think dropping this step from the current series and make it a part of the series that deals with rename/rename would make more sense.
Re: [PATCH 2/4] merge-recursive: increase marker length with depth of recursion
On Sun, Oct 14, 2018 at 10:12 PM Junio C Hamano wrote: > > Elijah Newren writes: > > > When using merge.conflictstyle=diff3 to have the "base version" be shown > > in conflicts, there is the possibility that the base version itself has > > conflicts in it. This comes about when there are more than one merge > > base, and the merging of those merge bases produces a conflict. > > Since this process applies recursively, it is possible to have conflict > > markers nested at an arbitrary depth; to be able to differentiate the > > conflict markers from different nestings, we make them all of different > > lengths. > > I know it is possible that the common ancestor part that is enclosed > by the outermost makers can have arbitrary conflicts, and they can > be even recursive conflicts. But I fail to see why it is a problem. > Perhaps that is because I am not particularly good at resolving > merge conflicts, but as long as the common part of the outermost > merge is identifyable, would that really matter? What I would do > while looking at common ancestor part with conflicts (not even a > recursive one) is just to ignore it, so... > > Not that I strongly oppose to incrementing the marker length at > every level. I do not think it would hurt, but I just do not see > how it would help. Fair enough. The real motivation for these changes was the modification to rename/rename(2to1) conflicts (and rename/add conflicts) to behave like add/add conflicts -- that change means we can have nested conflict markers even without invoking the recursive part of the recursive machinery. So, I needed a way to increase the length of the conflict markers besides just checking opts->virtual_ancestor. Just using a fixed extra indent seemed problematic, because if I also had to worry about even one virtual_ancestor, then I was already dealing with the possibility of triply nested conflict markers and only one of them from a virtual merge base. See t6036 in https://public-inbox.org/git/20181014020537.17991-3-new...@gmail.com/. However, that series was long enough, so to try to simplify review I split as much as I could out of it. That resulted, among other things, in me submitting this marker nesting change as part of this series using a more limited rationale. Would you like me to edit the commit message to include this more difficult case?
Re: [PATCH 2/4] merge-recursive: increase marker length with depth of recursion
Elijah Newren writes: > When using merge.conflictstyle=diff3 to have the "base version" be shown > in conflicts, there is the possibility that the base version itself has > conflicts in it. This comes about when there are more than one merge > base, and the merging of those merge bases produces a conflict. > Since this process applies recursively, it is possible to have conflict > markers nested at an arbitrary depth; to be able to differentiate the > conflict markers from different nestings, we make them all of different > lengths. I know it is possible that the common ancestor part that is enclosed by the outermost makers can have arbitrary conflicts, and they can be even recursive conflicts. But I fail to see why it is a problem. Perhaps that is because I am not particularly good at resolving merge conflicts, but as long as the common part of the outermost merge is identifyable, would that really matter? What I would do while looking at common ancestor part with conflicts (not even a recursive one) is just to ignore it, so... Not that I strongly oppose to incrementing the marker length at every level. I do not think it would hurt, but I just do not see how it would help.
