D1694: debugcommands: replace opts.get('foo') by opts['foo']
lothiraldan added a comment. In https://phab.mercurial-scm.org/D1694#79416, @pulkit wrote: > In https://phab.mercurial-scm.org/D1694#29072, @yuja wrote: > > > Sometimes we do the reverse change for ease of calling command function as a plain function. > > > Just for record, today I hit the problem where some command is using `opts['']` instead of `opts.get()` and I was calling command function as plain function leading me to KeyError. It would be nice to have some official API to call a command as a function. Such function would do the same processing as the dispatch logic to validate input and install default value. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1694 To: martinvonz, #hg-reviewers Cc: lothiraldan, pulkit, durin42, yuja, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1694: debugcommands: replace opts.get('foo') by opts['foo']
pulkit added a comment. In https://phab.mercurial-scm.org/D1694#29072, @yuja wrote: > Sometimes we do the reverse change for ease of calling command function as a plain function. Just for record, today I hit the problem where some command is using `opts['']` instead of `opts.get()` and I was trying to command function as plain function leading me to KeyError. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1694 To: martinvonz, #hg-reviewers Cc: pulkit, durin42, yuja, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1694: debugcommands: replace opts.get('foo') by opts['foo']
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 ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1694: debugcommands: replace opts.get('foo') by opts['foo']
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 ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1694: debugcommands: replace opts.get('foo') by opts['foo']
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 ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1694: debugcommands: replace opts.get('foo') by opts['foo']
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 ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1694: debugcommands: replace opts.get('foo') by opts['foo']
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 ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1694: debugcommands: replace opts.get('foo') by opts['foo']
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
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 =
