Patchwork D1694: debugcommands: replace opts.get('foo') by opts['foo']

login
register
mail settings
Submitter phabricator
Date Dec. 14, 2017, 11:35 p.m.
Message ID <differential-rev-PHID-DREV-st5j4xpzrrfvoenmbk64-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/26289/
State Superseded
Headers show

Comments

phabricator - Dec. 14, 2017, 11:35 p.m.
martinvonz 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/D1694

AFFECTED FILES
  mercurial/debugcommands.py

CHANGE DETAILS




To: martinvonz, #hg-reviewers
Cc: mercurial-devel
phabricator - Dec. 15, 2017, 1:29 p.m.
yuja added a comment.


  Queued the first three patches, but I'm not certain about this. Sometimes we
  do the reverse change for ease of calling command function as a plain function.

REPOSITORY
  rHG Mercurial

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

To: martinvonz, #hg-reviewers
Cc: yuja, mercurial-devel
phabricator - Jan. 10, 2018, 10:40 p.m.
durin42 added a comment.


  In https://phab.mercurial-scm.org/D1694#29072, @yuja wrote:
  
  > Queued the first three patches, but I'm not certain about this. Sometimes we
  >  do the reverse change for ease of calling command function as a plain function.
  
  
  I think it's probably okay for debug commands, those are pretty rare to use as a function aren't they?

REPOSITORY
  rHG Mercurial

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

To: martinvonz, #hg-reviewers
Cc: durin42, yuja, mercurial-devel
phabricator - Jan. 12, 2018, 12:17 p.m.
yuja added a comment.


  In https://phab.mercurial-scm.org/D1694#31044, @durin42 wrote:
  
  > I think it's probably okay for debug commands, those are pretty rare to use as a function aren't they?
  
  
  Yeah, it's okay, but why do we apply a different rule to debug commands?
  
  If we take this, I'd rather replace `.get()` by `[]` everywhere to blame third-party
  tools which don't pass all options.

REPOSITORY
  rHG Mercurial

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

To: martinvonz, #hg-reviewers
Cc: durin42, yuja, mercurial-devel
phabricator - Jan. 18, 2018, 8:09 p.m.
durin42 added a comment.


  I don't feel strongly either. Martin, if you want to pursue this (which seems fine to me) why not make ocmmands.py consistent as well?

REPOSITORY
  rHG Mercurial

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

To: martinvonz, #hg-reviewers
Cc: durin42, yuja, mercurial-devel
phabricator - June 14, 2018, 7:51 p.m.
pulkit added a comment.


  @martinvonz do you want to pursue this or can this be mark as abandoned? (Trying to close differentials which are no longer required)

REPOSITORY
  rHG Mercurial

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

To: martinvonz, #hg-reviewers
Cc: pulkit, durin42, yuja, mercurial-devel

Patch

diff --git a/mercurial/debugcommands.py b/mercurial/debugcommands.py
--- a/mercurial/debugcommands.py
+++ b/mercurial/debugcommands.py
@@ -416,7 +416,7 @@ 
 def debugcolor(ui, repo, **opts):
     """show available color, effects or style"""
     ui.write(('color mode: %s\n') % ui._colormode)
-    if opts.get(r'style'):
+    if opts[r'style']:
         return _debugdisplaystyle(ui)
     else:
         return _debugdisplaycolor(ui)
@@ -484,8 +484,8 @@ 
 
     Otherwise, the changelog DAG of the current repo is emitted.
     """
-    spaces = opts.get(r'spaces')
-    dots = opts.get(r'dots')
+    spaces = opts[r'spaces']
+    dots = opts[r'dots']
     if file_:
         rlog = revlog.revlog(vfsmod.vfs(pycompat.getcwd(), audit=False),
                              file_)
@@ -498,8 +498,8 @@ 
                     yield 'l', (r, "r%i" % r)
     elif repo:
         cl = repo.changelog
-        tags = opts.get(r'tags')
-        branches = opts.get(r'branches')
+        tags = opts[r'tags']
+        branches = opts[r'branches']
         if tags:
             labels = {}
             for l, n in repo.tags().items():
@@ -536,7 +536,7 @@ 
 def debugdata(ui, repo, file_, rev=None, **opts):
     """dump the contents of a data file revision"""
     opts = pycompat.byteskwargs(opts)
-    if opts.get('changelog') or opts.get('manifest') or opts.get('dir'):
+    if opts['changelog'] or opts['manifest'] or opts['dir']:
         if rev is not None:
             raise error.CommandError('debugdata', _('invalid arguments'))
         file_, rev = None, file_
@@ -709,8 +709,8 @@ 
 def debugstate(ui, repo, **opts):
     """show the contents of the current dirstate"""
 
-    nodates = opts.get(r'nodates')
-    datesort = opts.get(r'datesort')
+    nodates = opts[r'nodates']
+    datesort = opts[r'datesort']
 
     timestr = ""
     if datesort:
@@ -752,14 +752,14 @@ 
     random.seed(12323)
 
     def doit(pushedrevs, remoteheads, remote=remote):
-        if opts.get('old'):
+        if opts['old']:
             if not util.safehasattr(remote, 'branches'):
                 # enable in-client legacy support
                 remote = localrepo.locallegacypeer(remote.local())
             common, _in, hds = treediscovery.findcommonincoming(repo, remote,
                                                                 force=True)
             common = set(common)
-            if not opts.get('nonheads'):
+            if not opts['nonheads']:
                 ui.write(("unpruned common: %s\n") %
                          " ".join(sorted(short(n) for n in common)))
                 dag = dagutil.revlogdag(repo.changelog)
@@ -837,7 +837,7 @@ 
     _('[-r REV] FILESPEC'))
 def debugfileset(ui, repo, expr, **opts):
     '''parse and apply a fileset specification'''
-    ctx = scmutil.revsingle(repo, opts.get(r'rev'), None)
+    ctx = scmutil.revsingle(repo, opts[r'rev'], None)
     if ui.verbose:
         tree = fileset.parse(expr)
         ui.note(fileset.prettyformat(tree), "\n")
@@ -1292,21 +1292,21 @@ 
 
     """
 
-    if opts.get(r'force_lock'):
+    if opts[r'force_lock']:
         repo.svfs.unlink('lock')
-    if opts.get(r'force_wlock'):
+    if opts[r'force_wlock']:
         repo.vfs.unlink('wlock')
-    if opts.get(r'force_lock') or opts.get(r'force_wlock'):
+    if opts[r'force_lock'] or opts[r'force_wlock']:
         return 0
 
     locks = []
     try:
-        if opts.get(r'set_wlock'):
+        if opts[r'set_wlock']:
             try:
                 locks.append(repo.wlock(False))
             except error.LockHeld:
                 raise error.Abort(_('wlock is already held'))
-        if opts.get(r'set_lock'):
+        if opts[r'set_lock']:
             try:
                 locks.append(repo.lock(False))
             except error.LockHeld:
@@ -1506,9 +1506,9 @@ 
             raise error.Abort('changeset references must be full hexadecimal '
                              'node identifiers')
 
-    if opts.get('delete'):
+    if opts['delete']:
         indices = []
-        for v in opts.get('delete'):
+        for v in opts['delete']:
             try:
                 indices.append(int(v))
             except ValueError:
@@ -1535,7 +1535,7 @@ 
         try:
             tr = repo.transaction('debugobsolete')
             try:
-                date = opts.get('date')
+                date = opts['date']
                 if date:
                     date = util.parsedate(date)
                 else:
@@ -1570,7 +1570,7 @@ 
 
         markerstoiter = markers
         isrelevant = lambda m: True
-        if opts.get('rev') and opts.get('index'):
+        if opts['rev'] and opts['index']:
             markerstoiter = obsutil.getmarkers(repo)
             markerset = set(markers)
             isrelevant = lambda m: m in markerset
@@ -1587,7 +1587,7 @@ 
                 # are relevant to --rev value
                 continue
             fm.startitem()
-            ind = i if opts.get('index') else None
+            ind = i if opts['index'] else None
             cmdutil.showmarker(fm, m, index=ind)
         fm.end()
 
@@ -1718,7 +1718,7 @@ 
         if uimerge:
             ui.note(('with ui.merge=%r\n') % (uimerge))
 
-        ctx = scmutil.revsingle(repo, opts.get('rev'))
+        ctx = scmutil.revsingle(repo, opts['rev'])
         m = scmutil.match(ctx, pats, opts)
         changedelete = opts['changedelete']
         for path in ctx.walk(m):
@@ -1805,7 +1805,7 @@ 
         dirstate = repo.dirstate
         changedfiles = None
         # See command doc for what minimal does.
-        if opts.get(r'minimal'):
+        if opts[r'minimal']:
             manifestfiles = set(ctx.manifest().keys())
             dirstatefiles = set(dirstate)
             manifestonly = manifestfiles - dirstatefiles
@@ -1827,7 +1827,7 @@ 
     """dump rename information"""
 
     opts = pycompat.byteskwargs(opts)
-    ctx = scmutil.revsingle(repo, opts.get('rev'))
+    ctx = scmutil.revsingle(repo, opts['rev'])
     m = scmutil.match(ctx, (file1,) + pats, opts)
     for abs in ctx.walk(m):
         fctx = ctx[abs]
@@ -1847,7 +1847,7 @@ 
     opts = pycompat.byteskwargs(opts)
     r = cmdutil.openrevlog(repo, 'debugrevlog', file_, opts)
 
-    if opts.get("dump"):
+    if opts["dump"]:
         numrevs = len(r)
         ui.write(("# rev p1rev p2rev start   end deltastart base   p1   p2"
                  " rawsize totalsize compression heads chainlen\n"))