D8306: phabricator: add basectx arguments to file related `phabsend` utilities

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

D8306: phabricator: add basectx arguments to file related `phabsend` utilities

martinvonz (Martin von Zweigbergk)
mharbison72 created this revision.
Herald added subscribers: mercurial-devel, Kwan.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  This is in support of a future `--fold` option, that allows rolling up several
  commits into a single review with a diff from the start to the end of the range.
 
  There are no functional changes yet- the original `ctx` is also passed as the
  new `basectx`, which represents the first commit in the review range (similar to
  `qbase` in MQ parlance).  Other functions will need the range of commits, but
  these deal with status or the diffs, so they only need the end points.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  hgext/phabricator.py

CHANGE DETAILS

diff --git a/hgext/phabricator.py b/hgext/phabricator.py
--- a/hgext/phabricator.py
+++ b/hgext/phabricator.py
@@ -550,11 +550,11 @@
     return result
 
 
-def getdiff(ctx, diffopts):
+def getdiff(basectx, ctx, diffopts):
     """plain-text diff without header (user, commit message, etc)"""
     output = util.stringio()
     for chunk, _label in patch.diffui(
-        ctx.repo(), ctx.p1().node(), ctx.node(), None, opts=diffopts
+        ctx.repo(), basectx.p1().node(), ctx.node(), None, opts=diffopts
     ):
         output.write(chunk)
     return output.getvalue()
@@ -661,13 +661,13 @@
         )
 
 
-def maketext(pchange, ctx, fname):
+def maketext(pchange, basectx, ctx, fname):
     """populate the phabchange for a text file"""
     repo = ctx.repo()
     fmatcher = match.exact([fname])
     diffopts = mdiff.diffopts(git=True, context=32767)
     _pfctx, _fctx, header, fhunks = next(
-        patch.diffhunks(repo, ctx.p1(), ctx, fmatcher, opts=diffopts)
+        patch.diffhunks(repo, basectx.p1(), ctx, fmatcher, opts=diffopts)
     )
 
     for fhunk in fhunks:
@@ -813,25 +813,25 @@
         return True
 
 
-def addremoved(pdiff, ctx, removed):
+def addremoved(pdiff, basectx, ctx, removed):
     """add removed files to the phabdiff. Shouldn't include moves"""
     for fname in removed:
         pchange = phabchange(
             currentPath=fname, oldPath=fname, type=DiffChangeType.DELETE
         )
-        oldfctx = ctx.p1()[fname]
+        oldfctx = basectx.p1()[fname]
         pchange.addoldmode(gitmode[oldfctx.flags()])
         if not (oldfctx.isbinary() or notutf8(oldfctx)):
-            maketext(pchange, ctx, fname)
+            maketext(pchange, basectx, ctx, fname)
 
         pdiff.addchange(pchange)
 
 
-def addmodified(pdiff, ctx, modified):
+def addmodified(pdiff, basectx, ctx, modified):
     """add modified files to the phabdiff"""
     for fname in modified:
         fctx = ctx[fname]
-        oldfctx = ctx.p1()[fname]
+        oldfctx = basectx.p1()[fname]
         pchange = phabchange(currentPath=fname, oldPath=fname)
         filemode = gitmode[fctx.flags()]
         originalmode = gitmode[oldfctx.flags()]
@@ -848,12 +848,12 @@
             makebinary(pchange, fctx)
             addoldbinary(pchange, oldfctx, fctx)
         else:
-            maketext(pchange, ctx, fname)
+            maketext(pchange, basectx, ctx, fname)
 
         pdiff.addchange(pchange)
 
 
-def addadded(pdiff, ctx, added, removed):
+def addadded(pdiff, basectx, ctx, added, removed):
     """add file adds to the phabdiff, both new files and copies/moves"""
     # Keep track of files that've been recorded as moved/copied, so if there are
     # additional copies we can mark them (moves get removed from removed)
@@ -869,7 +869,7 @@
 
         if renamed:
             originalfname = renamed[0]
-            oldfctx = ctx.p1()[originalfname]
+            oldfctx = basectx.p1()[originalfname]
             originalmode = gitmode[oldfctx.flags()]
             pchange.oldPath = originalfname
 
@@ -914,7 +914,7 @@
             if renamed:
                 addoldbinary(pchange, oldfctx, fctx)
         else:
-            maketext(pchange, ctx, fname)
+            maketext(pchange, basectx, ctx, fname)
 
         pdiff.addchange(pchange)
 
@@ -924,21 +924,21 @@
         pdiff.addchange(movedchange)
 
 
-def creatediff(ctx):
+def creatediff(basectx, ctx):
     """create a Differential Diff"""
     repo = ctx.repo()
     repophid = getrepophid(repo)
     # Create a "Differential Diff" via "differential.creatediff" API
     pdiff = phabdiff(
-        sourceControlBaseRevision=b'%s' % ctx.p1().hex(),
+        sourceControlBaseRevision=b'%s' % basectx.p1().hex(),
         branch=b'%s' % ctx.branch(),
     )
-    modified, added, removed, _d, _u, _i, _c = ctx.p1().status(ctx)
+    modified, added, removed, _d, _u, _i, _c = basectx.p1().status(ctx)
     # addadded will remove moved files from removed, so addremoved won't get
     # them
-    addadded(pdiff, ctx, added, removed)
-    addmodified(pdiff, ctx, modified)
-    addremoved(pdiff, ctx, removed)
+    addadded(pdiff, basectx, ctx, added, removed)
+    addmodified(pdiff, basectx, ctx, modified)
+    addremoved(pdiff, basectx, ctx, removed)
     if repophid:
         pdiff.repositoryPHID = repophid
     diff = callconduit(
@@ -947,7 +947,11 @@
         pycompat.byteskwargs(attr.asdict(pdiff)),
     )
     if not diff:
-        raise error.Abort(_(b'cannot create diff for %s') % ctx)
+        if basectx != ctx:
+            msg = _(b'cannot create diff for %s::%s') % (basectx, ctx)
+        else:
+            msg = _(b'cannot create diff for %s') % ctx
+        raise error.Abort(msg)
     return diff
 
 
@@ -1008,17 +1012,21 @@
 
     If actions is not None, they will be appended to the transaction.
     """
+    basectx = ctx
     repo = ctx.repo()
     if oldnode:
         diffopts = mdiff.diffopts(git=True, context=32767)
         oldctx = repo.unfiltered()[oldnode]
-        neednewdiff = getdiff(ctx, diffopts) != getdiff(oldctx, diffopts)
+        oldbasectx = oldctx
+        neednewdiff = getdiff(basectx, ctx, diffopts) != getdiff(
+            oldbasectx, oldctx, diffopts
+        )
     else:
         neednewdiff = True
 
     transactions = []
     if neednewdiff:
-        diff = creatediff(ctx)
+        diff = creatediff(basectx, ctx)
         transactions.append({b'type': b'update', b'value': diff[b'phid']})
         if comment:
             transactions.append({b'type': b'comment', b'value': comment})



To: mharbison72, #hg-reviewers
Cc: Kwan, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D8306: phabricator: add basectx arguments to file related `phabsend` utilities

martinvonz (Martin von Zweigbergk)
Alphare added a comment.
Alphare accepted this revision.


  I am very curious as to how the UI will look on a folded series.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8306/new/

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

To: mharbison72, #hg-reviewers, Alphare
Cc: Alphare, Kwan, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D8306: phabricator: add basectx arguments to file related `phabsend` utilities

martinvonz (Martin von Zweigbergk)
In reply to this post by martinvonz (Martin von Zweigbergk)
mharbison72 added a comment.


  In D8306#124380 <https://phab.mercurial-scm.org/D8306#124380>, @Alphare wrote:
 
  > I am very curious as to how the UI will look on a folded series.
 
  Thanks for the reviews!
 
  See D8330 <https://phab.mercurial-scm.org/D8330>.  It basically looks the same, other than the `Commits` tab has multiple revisions.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8306/new/

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

To: mharbison72, #hg-reviewers, Alphare
Cc: Alphare, Kwan, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D8306: phabricator: add basectx arguments to file related `phabsend` utilities

martinvonz (Martin von Zweigbergk)
In reply to this post by martinvonz (Martin von Zweigbergk)
Closed by commit rHG53d75fdeaaaa: phabricator: add basectx arguments to file related `phabsend` utilities (authored by mharbison72).
This revision was automatically updated to reflect the committed changes.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D8306?vs=20846&id=20894

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8306/new/

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

AFFECTED FILES
  hgext/phabricator.py

CHANGE DETAILS

diff --git a/hgext/phabricator.py b/hgext/phabricator.py
--- a/hgext/phabricator.py
+++ b/hgext/phabricator.py
@@ -550,11 +550,11 @@
     return result
 
 
-def getdiff(ctx, diffopts):
+def getdiff(basectx, ctx, diffopts):
     """plain-text diff without header (user, commit message, etc)"""
     output = util.stringio()
     for chunk, _label in patch.diffui(
-        ctx.repo(), ctx.p1().node(), ctx.node(), None, opts=diffopts
+        ctx.repo(), basectx.p1().node(), ctx.node(), None, opts=diffopts
     ):
         output.write(chunk)
     return output.getvalue()
@@ -661,13 +661,13 @@
         )
 
 
-def maketext(pchange, ctx, fname):
+def maketext(pchange, basectx, ctx, fname):
     """populate the phabchange for a text file"""
     repo = ctx.repo()
     fmatcher = match.exact([fname])
     diffopts = mdiff.diffopts(git=True, context=32767)
     _pfctx, _fctx, header, fhunks = next(
-        patch.diffhunks(repo, ctx.p1(), ctx, fmatcher, opts=diffopts)
+        patch.diffhunks(repo, basectx.p1(), ctx, fmatcher, opts=diffopts)
     )
 
     for fhunk in fhunks:
@@ -813,25 +813,25 @@
         return True
 
 
-def addremoved(pdiff, ctx, removed):
+def addremoved(pdiff, basectx, ctx, removed):
     """add removed files to the phabdiff. Shouldn't include moves"""
     for fname in removed:
         pchange = phabchange(
             currentPath=fname, oldPath=fname, type=DiffChangeType.DELETE
         )
-        oldfctx = ctx.p1()[fname]
+        oldfctx = basectx.p1()[fname]
         pchange.addoldmode(gitmode[oldfctx.flags()])
         if not (oldfctx.isbinary() or notutf8(oldfctx)):
-            maketext(pchange, ctx, fname)
+            maketext(pchange, basectx, ctx, fname)
 
         pdiff.addchange(pchange)
 
 
-def addmodified(pdiff, ctx, modified):
+def addmodified(pdiff, basectx, ctx, modified):
     """add modified files to the phabdiff"""
     for fname in modified:
         fctx = ctx[fname]
-        oldfctx = ctx.p1()[fname]
+        oldfctx = basectx.p1()[fname]
         pchange = phabchange(currentPath=fname, oldPath=fname)
         filemode = gitmode[fctx.flags()]
         originalmode = gitmode[oldfctx.flags()]
@@ -848,12 +848,12 @@
             makebinary(pchange, fctx)
             addoldbinary(pchange, oldfctx, fctx)
         else:
-            maketext(pchange, ctx, fname)
+            maketext(pchange, basectx, ctx, fname)
 
         pdiff.addchange(pchange)
 
 
-def addadded(pdiff, ctx, added, removed):
+def addadded(pdiff, basectx, ctx, added, removed):
     """add file adds to the phabdiff, both new files and copies/moves"""
     # Keep track of files that've been recorded as moved/copied, so if there are
     # additional copies we can mark them (moves get removed from removed)
@@ -869,7 +869,7 @@
 
         if renamed:
             originalfname = renamed[0]
-            oldfctx = ctx.p1()[originalfname]
+            oldfctx = basectx.p1()[originalfname]
             originalmode = gitmode[oldfctx.flags()]
             pchange.oldPath = originalfname
 
@@ -914,7 +914,7 @@
             if renamed:
                 addoldbinary(pchange, oldfctx, fctx)
         else:
-            maketext(pchange, ctx, fname)
+            maketext(pchange, basectx, ctx, fname)
 
         pdiff.addchange(pchange)
 
@@ -924,21 +924,21 @@
         pdiff.addchange(movedchange)
 
 
-def creatediff(ctx):
+def creatediff(basectx, ctx):
     """create a Differential Diff"""
     repo = ctx.repo()
     repophid = getrepophid(repo)
     # Create a "Differential Diff" via "differential.creatediff" API
     pdiff = phabdiff(
-        sourceControlBaseRevision=b'%s' % ctx.p1().hex(),
+        sourceControlBaseRevision=b'%s' % basectx.p1().hex(),
         branch=b'%s' % ctx.branch(),
     )
-    modified, added, removed, _d, _u, _i, _c = ctx.p1().status(ctx)
+    modified, added, removed, _d, _u, _i, _c = basectx.p1().status(ctx)
     # addadded will remove moved files from removed, so addremoved won't get
     # them
-    addadded(pdiff, ctx, added, removed)
-    addmodified(pdiff, ctx, modified)
-    addremoved(pdiff, ctx, removed)
+    addadded(pdiff, basectx, ctx, added, removed)
+    addmodified(pdiff, basectx, ctx, modified)
+    addremoved(pdiff, basectx, ctx, removed)
     if repophid:
         pdiff.repositoryPHID = repophid
     diff = callconduit(
@@ -947,7 +947,11 @@
         pycompat.byteskwargs(attr.asdict(pdiff)),
     )
     if not diff:
-        raise error.Abort(_(b'cannot create diff for %s') % ctx)
+        if basectx != ctx:
+            msg = _(b'cannot create diff for %s::%s') % (basectx, ctx)
+        else:
+            msg = _(b'cannot create diff for %s') % ctx
+        raise error.Abort(msg)
     return diff
 
 
@@ -1008,17 +1012,21 @@
 
     If actions is not None, they will be appended to the transaction.
     """
+    basectx = ctx
     repo = ctx.repo()
     if oldnode:
         diffopts = mdiff.diffopts(git=True, context=32767)
         oldctx = repo.unfiltered()[oldnode]
-        neednewdiff = getdiff(ctx, diffopts) != getdiff(oldctx, diffopts)
+        oldbasectx = oldctx
+        neednewdiff = getdiff(basectx, ctx, diffopts) != getdiff(
+            oldbasectx, oldctx, diffopts
+        )
     else:
         neednewdiff = True
 
     transactions = []
     if neednewdiff:
-        diff = creatediff(ctx)
+        diff = creatediff(basectx, ctx)
         transactions.append({b'type': b'update', b'value': diff[b'phid']})
         if comment:
             transactions.append({b'type': b'comment', b'value': comment})



To: mharbison72, #hg-reviewers, Alphare, pulkit
Cc: Alphare, Kwan, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel