Patchwork D377: simplemerge: add `filtereddata=False` to simplemerge()

login
register
mail settings
Submitter phabricator
Date Aug. 14, 2017, 6:15 a.m.
Message ID <differential-rev-PHID-DREV-rk4sxmf7o3mqp2vu36p4-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/22959/
State Superseded
Headers show

Comments

phabricator - Aug. 14, 2017, 6:15 a.m.
phillco created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  In the next patch we'll make this function only take contexts, and also make
  contrib/simplemerge work by having it pass fake context-like objects.
  
  In order for those fake objects to work, simplemerge needs to know not to run
  the repo decoding filters on them -- the data will already be decoded, coming
  off the filesystem, and we won't have the repo object -- hence this flag.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D377

AFFECTED FILES
  mercurial/simplemerge.py

CHANGE DETAILS




To: phillco, #hg-reviewers
Cc: mercurial-devel
phabricator - Aug. 15, 2017, 9:39 p.m.
durin42 added a comment.


  (from an IRC conversation)
  
  I'm not crazy about the shape this makes the API, as I'd rather we didn't start growing awareness of filtering in layers this high.
  
  I think we can probably kill contrib/simplemerge entirely - it's had nothing but cleanup changes and a feature removal since May of 2007, and I can't fathom a use for it today. Let's see if we can get consensus around that on the mailing lists and then hopefully simplify this a TON.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D377

To: phillco, #hg-reviewers
Cc: durin42, mercurial-devel
phabricator - Aug. 15, 2017, 9:40 p.m.
durin42 added a comment.


  Also, timeless noticed https://phab.mercurial-scm.org/rHGabd66eb0889e3efaca4f960e0eacd719c18880dd, which reinforces my belief that we don't need contrib/simplemerge anymore.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D377

To: phillco, #hg-reviewers
Cc: durin42, mercurial-devel
phabricator - Aug. 17, 2017, 6:38 p.m.
phillco abandoned this revision.
phillco added a comment.


  Abandoned in favor of https://phab.mercurial-scm.org/D434's approach.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D377

To: phillco, #hg-reviewers
Cc: durin42, mercurial-devel

Patch

diff --git a/mercurial/simplemerge.py b/mercurial/simplemerge.py
--- a/mercurial/simplemerge.py
+++ b/mercurial/simplemerge.py
@@ -423,12 +423,17 @@ 
     return [name_a, name_b, name_base]
 
 def simplemerge(ui, localfile, basefile, otherfile,
-                localctx=None, basectx=None, otherctx=None, repo=None, **opts):
+                localctx=None, basectx=None, otherctx=None, repo=None,
+                filtereddata=False, **opts):
     """Performs the simplemerge algorithm.
 
     {local|base|other}ctx are optional. If passed, they (local/base/other) will
     be read from and the merge result written to (local). You should pass
-    explicit labels in this mode since the default is to use the file paths."""
+    explicit labels in this mode since the default is to use the file paths.
+
+    filtereddata should only be True if the data() in your context returns
+    decoded data.
+    """
     def readfile(filename):
         f = open(filename, "rb")
         text = f.read()
@@ -438,15 +443,19 @@ 
     def readctx(ctx):
         if not ctx:
             return None
-        if not repo:
-            raise error.ProgrammingError('simplemerge: repo must be passed if '
-                                         'using contexts')
-        # `wwritedata` is used to get the post-filter data from `ctx` (i.e.,
-        # what would have been in the working copy). Since merges were run in
-        # the working copy, and thus used post-filter data, we do the same to
-        # maintain behavior.
-        return repo.wwritedata(ctx.path(),
-                               _verifytext(ctx.data(), ctx.path(), ui, opts))
+        if filtereddata:
+            return _verifytext(ctx.data(), ctx.path(), ui, opts)
+        else:
+            if not repo:
+                raise error.ProgrammingError('simplemerge: repo must be passed '
+                                             'if using contexts and '
+                                             'filtereddata is False.')
+            # `wwritedata` is used to get the post-filter data from `ctx` (i.e.,
+            # what would have been in the working copy). Since merges were run
+            # in the working copy, and thus used post-filter data, we do the
+            # same to maintain behavior.
+            text = _verifytext(ctx.data(), ctx.path(), ui, opts)
+            return repo.wwritedata(ctx.path(), text)
 
     class ctxwriter(object):
         def __init__(self, ctx):