Patchwork D6107: py3: use r'' instead of b'' in opts.get() in phabricator.py

login
register
mail settings
Submitter phabricator
Date March 9, 2019, 3:01 a.m.
Message ID <differential-rev-PHID-DREV-b5herjemzyfngur65tkt-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/39161/
State Superseded
Headers show

Comments

phabricator - March 9, 2019, 3:01 a.m.
Kwan created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  hgext/phabricator.py

CHANGE DETAILS




To: Kwan, #hg-reviewers
Cc: mercurial-devel
Yuya Nishihara - March 10, 2019, 1:24 a.m.
`pycompat.byteskwargs()` can be used instead.
phabricator - March 10, 2019, 1:29 a.m.
yuja added a comment.


  `pycompat.byteskwargs()` can be used instead.

REPOSITORY
  rHG Mercurial

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

To: Kwan, #hg-reviewers
Cc: yuja, mercurial-devel
phabricator - March 14, 2019, 3:40 p.m.
Kwan added a comment.


  In https://phab.mercurial-scm.org/D6107#88990, @yuja wrote:
  
  > `pycompat.byteskwargs()` can be used instead.
  
  
  I was under the impression that that was more of a hack, mainly useful for when there are lots of existing `opts.get(b'')` uses in a file, and when there are a few it was better to change to `r''`.  Is it preferred to always use that then?

REPOSITORY
  rHG Mercurial

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

To: Kwan, #hg-reviewers
Cc: yuja, mercurial-devel
Yuya Nishihara - March 16, 2019, 2:42 a.m.
>   I was under the impression that that was more of a hack, mainly useful for when there are lots of existing `opts.get(b'')` uses in a file, and when there are a few it was better to change to `r''`.

Correct. And I feel there are lots of `s/b''/r''/`s in this patch.
phabricator - March 16, 2019, 2:44 a.m.
yuja added a comment.


  >   I was under the impression that that was more of a hack, mainly useful for when there are lots of existing `opts.get(b'')` uses in a file, and when there are a few it was better to change to `r''`.
  
  Correct. And I feel there are lots of `s/b''/r''/`s in this patch.

REPOSITORY
  rHG Mercurial

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

To: Kwan, #hg-reviewers
Cc: yuja, mercurial-devel

Patch

diff --git a/hgext/phabricator.py b/hgext/phabricator.py
--- a/hgext/phabricator.py
+++ b/hgext/phabricator.py
@@ -503,26 +503,26 @@ 
     phabsend will check obsstore and the above association to decide whether to
     update an existing Differential Revision, or create a new one.
     """
-    revs = list(revs) + opts.get(b'rev', [])
+    revs = list(revs) + opts.get(r'rev', [])
     revs = scmutil.revrange(repo, revs)
 
     if not revs:
         raise error.Abort(_(b'phabsend requires at least one changeset'))
-    if opts.get(b'amend'):
+    if opts.get(r'amend'):
         cmdutil.checkunfinished(repo)
 
     # {newnode: (oldnode, olddiff, olddrev}
     oldmap = getoldnodedrevmap(repo, [repo[r].node() for r in revs])
 
     confirm = ui.configbool(b'phabsend', b'confirm')
-    confirm |= bool(opts.get(b'confirm'))
+    confirm |= bool(opts.get(r'confirm'))
     if confirm:
         confirmed = _confirmbeforesend(repo, revs, oldmap)
         if not confirmed:
             raise error.Abort(_(b'phabsend cancelled'))
 
     actions = []
-    reviewers = opts.get(b'reviewer', [])
+    reviewers = opts.get(r'reviewer', [])
     if reviewers:
         phids = userphids(repo, reviewers)
         actions.append({b'type': b'reviewers.add', b'value': phids})
@@ -539,7 +539,7 @@ 
 
         # Get Differential Revision ID
         oldnode, olddiff, revid = oldmap.get(ctx.node(), (None, None, None))
-        if oldnode != ctx.node() or opts.get(b'amend'):
+        if oldnode != ctx.node() or opts.get(r'amend'):
             # Create or update Differential Revision
             revision, diff = createdifferentialrevision(
                 ctx, revid, lastrevid, oldnode, olddiff, actions)
@@ -577,7 +577,7 @@ 
         lastrevid = newrevid
 
     # Update commit messages and remove tags
-    if opts.get(b'amend'):
+    if opts.get(r'amend'):
         unfi = repo.unfiltered()
         drevs = callconduit(repo, b'differential.query', {b'ids': drevids})
         with repo.wlock(), repo.lock(), repo.transaction(b'phabsend'):
@@ -955,7 +955,7 @@ 
     If --stack is given, follow dependencies information and read all patches.
     It is equivalent to the ``:`` operator.
     """
-    if opts.get(b'stack'):
+    if opts.get(r'stack'):
         spec = b':(%s)' % spec
     drevs = querydrev(repo, spec)
     readpatch(repo, drevs, ui.write)
@@ -973,7 +973,7 @@ 
 
     DREVSPEC selects revisions. See :hg:`help phabread` for its usage.
     """
-    flags = [n for n in b'accept reject abandon reclaim'.split() if opts.get(n)]
+    flags = [n for n in r'accept reject abandon reclaim'.split() if opts.get(n)]
     if len(flags) > 1:
         raise error.Abort(_(b'%s cannot be used together') % b', '.join(flags))
 
@@ -983,8 +983,8 @@ 
 
     drevs = querydrev(repo, spec)
     for i, drev in enumerate(drevs):
-        if i + 1 == len(drevs) and opts.get(b'comment'):
-            actions.append({b'type': b'comment', b'value': opts[b'comment']})
+        if i + 1 == len(drevs) and opts.get(r'comment'):
+            actions.append({b'type': b'comment', b'value': opts[r'comment']})
         if actions:
             params = {b'objectIdentifier': drev[b'phid'],
                       b'transactions': actions}