Submitter | Jordi Gutiérrez Hermoso |
---|---|
Date | Aug. 13, 2014, 7 p.m. |
Message ID | <e125a4ec265f9207667e.1407956428@Iris> |
Download | mbox | patch |
Permalink | /patch/5369/ |
State | Changes Requested |
Headers | show |
Comments
On 08/13/2014 12:00 PM, Jordi Gutiérrez Hermoso wrote: > # HG changeset patch > # User Jordi Gutiérrez Hermoso <jordigh@octave.org> > # Date 1407783206 14400 > # Mon Aug 11 14:53:26 2014 -0400 > # Node ID e125a4ec265f9207667e82a62754dfd96f17c0a4 > # Parent fff8e1cec90f64b356c66aa068d3810f7efea0ac > filemerge: add non-interactive internal:merge-local and internal:merge-other > > There are two non-interactive internal merge tools, internal:other and > internal:local, but they don't really merge, they just pick all > changes from the local or other version of the file. In some > situations, it is known that we want a merge and also know that all > merge conflicts should be resolved in one direction. Although external > merge tools can do this, sometimes it can be convenient to do so from > within hg, without invoking a merge tool. These new > internal:merge-local and internal:merge-other tools can do just that. While I kindof like the idea and could see myself use it from time to time, I'm a bit worried about the name of this pair and the proliferation of this kind of variant in general. We already have: - internal:other - internal:local It is not clear how user will be able to distinct between your two new additions - internal:merge-other - internal:merge-local Also, I can see a third type of tool that will take the content of side X even when there was no manifest level conflict. I think we need to think harder about a proper UI to hand all those case. > > diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py > --- a/mercurial/filemerge.py > +++ b/mercurial/filemerge.py > @@ -229,6 +229,40 @@ def _imerge(repo, mynode, orig, fcd, fco > return True, r > return False, 0 > > +def _imergeauto(repo, mynode, orig, fcd, fco, fca, toolconf, files, > + labels=None, localorother=None): > + """ > + Generic driver for _imergelocal and _imergeother > + """ > + assert localorother is not None > + tool, toolpath, binary, symlink = toolconf > + if symlink: > + repo.ui.warn(_('warning: internal:merge-%s cannot merge symlinks ' > + 'for %s\n') % (localorother, fcd.path())) > + return False, 1 > + a, b, c, back = files > + r = simplemerge.simplemerge(repo.ui, a, b, c, label=labels, > + localorother=localorother) > + return True, r > + > +@internaltool('merge-local', True) > +def _imergelocal(*args, **kwargs): > + """ > + Like internal:merge, but resolve all conflicts non-interactively > + in favor of the local changes. > + """ > + success, status = _imergeauto(localorother='local', *args, **kwargs) > + return success, status > + > +@internaltool('merge-other', True) > +def _imergelocal(*args, **kwargs): You forgot to update the function name. > + """ > + Like internal:merge, but resolve all conflicts non-interactively > + in favor of the other changes. > + """ > + success, status = _imergeauto(localorother='other', *args, **kwargs) > + return success, status > + > @internaltool('merge3', True, > _("merging %s incomplete! " > "(edit conflicts, then use 'hg resolve --mark')\n")) > diff --git a/mercurial/simplemerge.py b/mercurial/simplemerge.py > --- a/mercurial/simplemerge.py > +++ b/mercurial/simplemerge.py > @@ -82,7 +82,8 @@ class Merge3Text(object): > start_marker='<<<<<<<', > mid_marker='=======', > end_marker='>>>>>>>', > - base_marker=None): > + base_marker=None, > + localorother=None): > """Return merge in cvs-like form. Slicing the change to simple merge in a dedicated commit would make it simpler to review. > """ > self.conflicts = False > @@ -111,18 +112,25 @@ class Merge3Text(object): > for i in range(t[1], t[2]): > yield self.b[i] > elif what == 'conflict': > - self.conflicts = True > - yield start_marker + newline > - for i in range(t[3], t[4]): > - yield self.a[i] > - if base_marker is not None: > - yield base_marker + newline > - for i in range(t[1], t[2]): > - yield self.base[i] > - yield mid_marker + newline > - for i in range(t[5], t[6]): > - yield self.b[i] > - yield end_marker + newline > + if localorother == 'local': > + for i in range(t[3], t[4]): > + yield self.a[i] > + elif localorother == 'other': > + for i in range(t[5], t[6]): > + yield self.b[i] > + else: > + self.conflicts = True > + yield start_marker + newline > + for i in range(t[3], t[4]): > + yield self.a[i] > + if base_marker is not None: > + yield base_marker + newline > + for i in range(t[1], t[2]): > + yield self.base[i] > + yield mid_marker + newline > + for i in range(t[5], t[6]): > + yield self.b[i] > + yield end_marker + newline > else: > raise ValueError(what) > > @@ -373,7 +381,7 @@ def simplemerge(ui, local, base, other, > out = sys.stdout > > m3 = Merge3Text(basetext, localtext, othertext) > - extrakwargs = {} > + extrakwargs = {"localorother": opts.get("localorother", None)} > if name_base is not None: > extrakwargs['base_marker'] = '|||||||' > extrakwargs['name_base'] = name_base > diff --git a/tests/test-conflict.t b/tests/test-conflict.t > --- a/tests/test-conflict.t > +++ b/tests/test-conflict.t > @@ -232,3 +232,66 @@ internal:merge3 > 5 > >>>>>>> other > Hop we are done. > + > + > +Add some unconflicting changes on each head, to make sure we really > +are merging, unlike internal:local and internal:other > + > + $ hg up -C > + 2 files updated, 0 files merged, 0 files removed, 0 files unresolved > + $ printf "\n\nEnd of file\n" >> a > + $ hg ci -m "Add some stuff at the end" > + $ hg up -r 1 > + 1 files updated, 0 files merged, 1 files removed, 0 files unresolved > + $ printf "Start of file\n\n\n" > tmp > + $ cat a >> tmp > + $ mv tmp a > + $ hg ci -m "Add some stuff at the beginning" > + > +Now test internal:merge-other and internal:merge-local > + > + $ hg merge > + merging a > + warning: conflicts during merge. > + merging a incomplete! (edit conflicts, then use 'hg resolve --mark') > + 1 files updated, 0 files merged, 0 files removed, 1 files unresolved > + use 'hg resolve' to retry unresolved file merges or 'hg update -C .' to abandon > + [1] > + $ hg resolve --tool internal:merge-other a > + merging a > + (no more unresolved files) > + $ cat a > + Start of file > + > + > + Small Mathematical Series. > + 1 > + 2 > + 3 > + 6 > + 8 > + Hop we are done. > + > + > + End of file > + > + $ hg up -C > + 1 files updated, 0 files merged, 1 files removed, 0 files unresolved > + $ hg merge --tool internal:merge-local > + merging a > + 2 files updated, 0 files merged, 0 files removed, 0 files unresolved > + (branch merge, don't forget to commit) > + $ cat a > + Start of file > + > + > + Small Mathematical Series. > + 1 > + 2 > + 300:12 < stantonk> so how would roots ever return anything? > + 4 > + 5 > + Hop we are done. > + > + > + End of file > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > http://selenic.com/mailman/listinfo/mercurial-devel >
On Mon, 2014-08-18 at 15:22 -0700, Pierre-Yves David wrote: > We already have: > > - internal:other > - internal:local > > It is not clear how user will be able to distinct between your two new > additions > > - internal:merge-other > - internal:merge-local Well, they show up with brief explanations in "hg help merge-tools". How could it be made clearer? I think the problem starts that internal:other and internal:local are kind of confusing. A lot of people, myself included, seem to think upon first meeting them that they merge but resolve conflicts one way. > Also, I can see a third type of tool that will take the content of side > X even when there was no manifest level conflict. You mean, when there is no merging to be done? Then no merge tool would get called for it. > I think we need to think harder about a proper UI to hand all those > case. I can't think of such a UI that wouldn't break existing things. What do you have in mind? > > +@internaltool('merge-other', True) > > +def _imergelocal(*args, **kwargs): > > You forgot to update the function name. Oh, wow. I was trying to figure out how this passed tests. I suppose because the internaltool decorator closes over the function it's wrapping when it puts it in the function list. > Slicing the change to simple merge in a dedicated commit would make > it simpler to review. Okay, so two commits, one for simplemerge, one for the rest? I can do that. - Jordi G. H.
Patch
diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py --- a/mercurial/filemerge.py +++ b/mercurial/filemerge.py @@ -229,6 +229,40 @@ def _imerge(repo, mynode, orig, fcd, fco return True, r return False, 0 +def _imergeauto(repo, mynode, orig, fcd, fco, fca, toolconf, files, + labels=None, localorother=None): + """ + Generic driver for _imergelocal and _imergeother + """ + assert localorother is not None + tool, toolpath, binary, symlink = toolconf + if symlink: + repo.ui.warn(_('warning: internal:merge-%s cannot merge symlinks ' + 'for %s\n') % (localorother, fcd.path())) + return False, 1 + a, b, c, back = files + r = simplemerge.simplemerge(repo.ui, a, b, c, label=labels, + localorother=localorother) + return True, r + +@internaltool('merge-local', True) +def _imergelocal(*args, **kwargs): + """ + Like internal:merge, but resolve all conflicts non-interactively + in favor of the local changes. + """ + success, status = _imergeauto(localorother='local', *args, **kwargs) + return success, status + +@internaltool('merge-other', True) +def _imergelocal(*args, **kwargs): + """ + Like internal:merge, but resolve all conflicts non-interactively + in favor of the other changes. + """ + success, status = _imergeauto(localorother='other', *args, **kwargs) + return success, status + @internaltool('merge3', True, _("merging %s incomplete! " "(edit conflicts, then use 'hg resolve --mark')\n")) diff --git a/mercurial/simplemerge.py b/mercurial/simplemerge.py --- a/mercurial/simplemerge.py +++ b/mercurial/simplemerge.py @@ -82,7 +82,8 @@ class Merge3Text(object): start_marker='<<<<<<<', mid_marker='=======', end_marker='>>>>>>>', - base_marker=None): + base_marker=None, + localorother=None): """Return merge in cvs-like form. """ self.conflicts = False @@ -111,18 +112,25 @@ class Merge3Text(object): for i in range(t[1], t[2]): yield self.b[i] elif what == 'conflict': - self.conflicts = True - yield start_marker + newline - for i in range(t[3], t[4]): - yield self.a[i] - if base_marker is not None: - yield base_marker + newline - for i in range(t[1], t[2]): - yield self.base[i] - yield mid_marker + newline - for i in range(t[5], t[6]): - yield self.b[i] - yield end_marker + newline + if localorother == 'local': + for i in range(t[3], t[4]): + yield self.a[i] + elif localorother == 'other': + for i in range(t[5], t[6]): + yield self.b[i] + else: + self.conflicts = True + yield start_marker + newline + for i in range(t[3], t[4]): + yield self.a[i] + if base_marker is not None: + yield base_marker + newline + for i in range(t[1], t[2]): + yield self.base[i] + yield mid_marker + newline + for i in range(t[5], t[6]): + yield self.b[i] + yield end_marker + newline else: raise ValueError(what) @@ -373,7 +381,7 @@ def simplemerge(ui, local, base, other, out = sys.stdout m3 = Merge3Text(basetext, localtext, othertext) - extrakwargs = {} + extrakwargs = {"localorother": opts.get("localorother", None)} if name_base is not None: extrakwargs['base_marker'] = '|||||||' extrakwargs['name_base'] = name_base diff --git a/tests/test-conflict.t b/tests/test-conflict.t --- a/tests/test-conflict.t +++ b/tests/test-conflict.t @@ -232,3 +232,66 @@ internal:merge3 5 >>>>>>> other Hop we are done. + + +Add some unconflicting changes on each head, to make sure we really +are merging, unlike internal:local and internal:other + + $ hg up -C + 2 files updated, 0 files merged, 0 files removed, 0 files unresolved + $ printf "\n\nEnd of file\n" >> a + $ hg ci -m "Add some stuff at the end" + $ hg up -r 1 + 1 files updated, 0 files merged, 1 files removed, 0 files unresolved + $ printf "Start of file\n\n\n" > tmp + $ cat a >> tmp + $ mv tmp a + $ hg ci -m "Add some stuff at the beginning" + +Now test internal:merge-other and internal:merge-local + + $ hg merge + merging a + warning: conflicts during merge. + merging a incomplete! (edit conflicts, then use 'hg resolve --mark') + 1 files updated, 0 files merged, 0 files removed, 1 files unresolved + use 'hg resolve' to retry unresolved file merges or 'hg update -C .' to abandon + [1] + $ hg resolve --tool internal:merge-other a + merging a + (no more unresolved files) + $ cat a + Start of file + + + Small Mathematical Series. + 1 + 2 + 3 + 6 + 8 + Hop we are done. + + + End of file + + $ hg up -C + 1 files updated, 0 files merged, 1 files removed, 0 files unresolved + $ hg merge --tool internal:merge-local + merging a + 2 files updated, 0 files merged, 0 files removed, 0 files unresolved + (branch merge, don't forget to commit) + $ cat a + Start of file + + + Small Mathematical Series. + 1 + 2 + 3 + 4 + 5 + Hop we are done. + + + End of file