This is an archive of the discontinued Mercurial Phabricator instance.

Differential D3967

dispatch: making all hg abortions be output with a specific label
ClosedPublic

Authored by rdamazio on Jul 20 2018, 2:22 AM.

Details

Reviewers
martinvonz
pulkit
Group Reviewers
hg-reviewers
Commits
rHGafc4ad706f9c: dispatch: making all hg abortions be output with a specific label
Summary

This allows abortions to be highlighted specially and separately
from warnings - for instance, red is a reasonable color for when hg
aborts, but is overly dramatic for most warnings produced elsewhere.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

rdamazio created this revision.Jul 20 2018, 2:22 AM
martinvonz added a subscriber: martinvonz.Jul 20 2018, 2:46 AM
martinvonz added inline comments.
mercurial/commandserver.py
356

What do you think about introducing a ui.error() instead?

rdamazio marked an inline comment as done.Jul 20 2018, 4:25 PM
rdamazio updated this revision to Diff 9641.
martinvonz accepted this revision.Jul 20 2018, 4:30 PM

Looks good! I won't queue it now because it's freeze and it feels controversial enough that I shouldn't queue it anyway. I've actually wanted a ui.error() for some time and it's just the fact that we still don't have have it that makes me wonder if it's controversial.

mercurial/scmutil.py
172–174

nit: revert formatting change

This revision is now accepted and ready to land.Jul 20 2018, 4:30 PM
lothiraldan added a subscriber: lothiraldan.Jul 23 2018, 4:38 AM
lothiraldan added inline comments.
mercurial/commandserver.py
356

Huge 👍

pulkit accepted this revision.Jul 23 2018, 3:05 PM
pulkit added a subscriber: pulkit.

Looks good to me too. I don't think this is something we accept on stable so I will hold on pushing this until code-freeze.

martinvonz added a comment.Aug 1 2018, 12:20 PM

Queuing this given Boris's and Pulkit's blessings.

Closed by commit rHGafc4ad706f9c: dispatch: making all hg abortions be output with a specific label (authored by rdamazio). · Explain WhyAug 1 2018, 12:41 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/color.py (1 line)
Mmercurial/commandserver.py (2 lines)
Mmercurial/dispatch.py (6 lines)
Mmercurial/scmutil.py (66 lines)
Mmercurial/ui.py (12 lines)

Diff 9694

mercurial/color.py

'formatvariant.name.uptodate': 'green', 'formatvariant.name.uptodate': 'green',
'formatvariant.repo.mismatchconfig': 'red', 'formatvariant.repo.mismatchconfig': 'red',
'formatvariant.repo.mismatchdefault': 'yellow', 'formatvariant.repo.mismatchdefault': 'yellow',
'formatvariant.repo.uptodate': 'green', 'formatvariant.repo.uptodate': 'green',
'formatvariant.config.special': 'yellow', 'formatvariant.config.special': 'yellow',
'formatvariant.config.default': 'green', 'formatvariant.config.default': 'green',
'formatvariant.default': '', 'formatvariant.default': '',
'histedit.remaining': 'red bold', 'histedit.remaining': 'red bold',
'ui.error': 'red',
'ui.prompt': 'yellow', 'ui.prompt': 'yellow',
'log.changeset': 'yellow', 'log.changeset': 'yellow',
'patchbomb.finalsummary': '', 'patchbomb.finalsummary': '',
'patchbomb.from': 'magenta', 'patchbomb.from': 'magenta',
'patchbomb.to': 'cyan', 'patchbomb.to': 'cyan',
'patchbomb.subject': 'green', 'patchbomb.subject': 'green',
'patchbomb.diffstats': '', 'patchbomb.diffstats': '',
'rebase.rebased': 'blue', 'rebase.rebased': 'blue',

mercurial/commandserver.py

sv = None sv = None
try: try:
sv = createcmdserver(repo, conn, fin, fout) sv = createcmdserver(repo, conn, fin, fout)
try: try:
sv.serve() sv.serve()
# handle exceptions that may be raised by command server. most of # handle exceptions that may be raised by command server. most of
# known exceptions are caught by dispatch. # known exceptions are caught by dispatch.
except error.Abort as inst: except error.Abort as inst:
ui.warn(_('abort: %s\n') % inst) ui.error(_('abort: %s\n') % inst)
martinvonzUnsubmitted
Done

What do you think about introducing a ui.error() instead?

martinvonz: What do you think about introducing a ui.error() instead?
lothiraldanUnsubmitted
Not Done

Huge 👍

lothiraldan: Huge :+1:
except IOError as inst: except IOError as inst:
if inst.errno != errno.EPIPE: if inst.errno != errno.EPIPE:
raise raise
except KeyboardInterrupt: except KeyboardInterrupt:
pass pass
finally: finally:
sv.cleanup() sv.cleanup()
except: # re-raises except: # re-raises

mercurial/dispatch.py

return -1 return -1
msg = _formatargs(req.args) msg = _formatargs(req.args)
starttime = util.timer() starttime = util.timer()
ret = 1 # default of Python exit code on unhandled exception ret = 1 # default of Python exit code on unhandled exception
try: try:
ret = _runcatch(req) or 0 ret = _runcatch(req) or 0
except error.ProgrammingError as inst: except error.ProgrammingError as inst:
req.ui.warn(_('** ProgrammingError: %s\n') % inst) req.ui.error(_('** ProgrammingError: %s\n') % inst)
if inst.hint: if inst.hint:
req.ui.warn(_('** (%s)\n') % inst.hint) req.ui.error(_('** (%s)\n') % inst.hint)
raise raise
except KeyboardInterrupt as inst: except KeyboardInterrupt as inst:
try: try:
if isinstance(inst, error.SignalInterrupt): if isinstance(inst, error.SignalInterrupt):
msg = _("killed!\n") msg = _("killed!\n")
else: else:
msg = _("interrupted!\n") msg = _("interrupted!\n")
req.ui.warn(msg) req.ui.error(msg)
except error.SignalInterrupt: except error.SignalInterrupt:
# maybe pager would quit without consuming all the output, and # maybe pager would quit without consuming all the output, and
# SIGPIPE was raised. we cannot print anything in this case. # SIGPIPE was raised. we cannot print anything in this case.
pass pass
except IOError as inst: except IOError as inst:
if inst.errno != errno.EPIPE: if inst.errno != errno.EPIPE:
raise raise
ret = -1 ret = -1

mercurial/scmutil.py

raise raise
# Global exception handling, alphabetically # Global exception handling, alphabetically
# Mercurial-specific first, followed by built-in and library exceptions # Mercurial-specific first, followed by built-in and library exceptions
except error.LockHeld as inst: except error.LockHeld as inst:
if inst.errno == errno.ETIMEDOUT: if inst.errno == errno.ETIMEDOUT:
reason = _('timed out waiting for lock held by %r') % inst.locker reason = _('timed out waiting for lock held by %r') % inst.locker
else: else:
reason = _('lock held by %r') % inst.locker reason = _('lock held by %r') % inst.locker
ui.warn(_("abort: %s: %s\n") ui.error(_("abort: %s: %s\n") % (
% (inst.desc or stringutil.forcebytestr(inst.filename), reason)) inst.desc or stringutil.forcebytestr(inst.filename), reason))
if not inst.locker: if not inst.locker:
martinvonzUnsubmitted
Not Done

nit: revert formatting change

martinvonz: nit: revert formatting change
ui.warn(_("(lock might be very busy)\n")) ui.error(_("(lock might be very busy)\n"))
except error.LockUnavailable as inst: except error.LockUnavailable as inst:
ui.warn(_("abort: could not lock %s: %s\n") % ui.error(_("abort: could not lock %s: %s\n") %
(inst.desc or stringutil.forcebytestr(inst.filename), (inst.desc or stringutil.forcebytestr(inst.filename),
encoding.strtolocal(inst.strerror))) encoding.strtolocal(inst.strerror)))
except error.OutOfBandError as inst: except error.OutOfBandError as inst:
if inst.args: if inst.args:
msg = _("abort: remote error:\n") msg = _("abort: remote error:\n")
else: else:
msg = _("abort: remote error\n") msg = _("abort: remote error\n")
ui.warn(msg) ui.error(msg)
if inst.args: if inst.args:
ui.warn(''.join(inst.args)) ui.error(''.join(inst.args))
if inst.hint: if inst.hint:
ui.warn('(%s)\n' % inst.hint) ui.error('(%s)\n' % inst.hint)
except error.RepoError as inst: except error.RepoError as inst:
ui.warn(_("abort: %s!\n") % inst) ui.error(_("abort: %s!\n") % inst)
if inst.hint: if inst.hint:
ui.warn(_("(%s)\n") % inst.hint) ui.error(_("(%s)\n") % inst.hint)
except error.ResponseError as inst: except error.ResponseError as inst:
ui.warn(_("abort: %s") % inst.args[0]) ui.error(_("abort: %s") % inst.args[0])
msg = inst.args[1] msg = inst.args[1]
if isinstance(msg, type(u'')): if isinstance(msg, type(u'')):
msg = pycompat.sysbytes(msg) msg = pycompat.sysbytes(msg)
if not isinstance(msg, bytes): if not isinstance(msg, bytes):
ui.warn(" %r\n" % (msg,)) ui.error(" %r\n" % (msg,))
elif not msg: elif not msg:
ui.warn(_(" empty string\n")) ui.error(_(" empty string\n"))
else: else:
ui.warn("\n%r\n" % pycompat.bytestr(stringutil.ellipsis(msg))) ui.error("\n%r\n" % pycompat.bytestr(stringutil.ellipsis(msg)))
except error.CensoredNodeError as inst: except error.CensoredNodeError as inst:
ui.warn(_("abort: file censored %s!\n") % inst) ui.error(_("abort: file censored %s!\n") % inst)
except error.RevlogError as inst: except error.RevlogError as inst:
ui.warn(_("abort: %s!\n") % inst) ui.error(_("abort: %s!\n") % inst)
except error.InterventionRequired as inst: except error.InterventionRequired as inst:
ui.warn("%s\n" % inst) ui.error("%s\n" % inst)
if inst.hint: if inst.hint:
ui.warn(_("(%s)\n") % inst.hint) ui.error(_("(%s)\n") % inst.hint)
return 1 return 1
except error.WdirUnsupported: except error.WdirUnsupported:
ui.warn(_("abort: working directory revision cannot be specified\n")) ui.error(_("abort: working directory revision cannot be specified\n"))
except error.Abort as inst: except error.Abort as inst:
ui.warn(_("abort: %s\n") % inst) ui.error(_("abort: %s\n") % inst)
if inst.hint: if inst.hint:
ui.warn(_("(%s)\n") % inst.hint) ui.error(_("(%s)\n") % inst.hint)
except ImportError as inst: except ImportError as inst:
ui.warn(_("abort: %s!\n") % stringutil.forcebytestr(inst)) ui.error(_("abort: %s!\n") % stringutil.forcebytestr(inst))
m = stringutil.forcebytestr(inst).split()[-1] m = stringutil.forcebytestr(inst).split()[-1]
if m in "mpatch bdiff".split(): if m in "mpatch bdiff".split():
ui.warn(_("(did you forget to compile extensions?)\n")) ui.error(_("(did you forget to compile extensions?)\n"))
elif m in "zlib".split(): elif m in "zlib".split():
ui.warn(_("(is your Python install correct?)\n")) ui.error(_("(is your Python install correct?)\n"))
except IOError as inst: except IOError as inst:
if util.safehasattr(inst, "code"): if util.safehasattr(inst, "code"):
ui.warn(_("abort: %s\n") % stringutil.forcebytestr(inst)) ui.error(_("abort: %s\n") % stringutil.forcebytestr(inst))
elif util.safehasattr(inst, "reason"): elif util.safehasattr(inst, "reason"):
try: # usually it is in the form (errno, strerror) try: # usually it is in the form (errno, strerror)
reason = inst.reason.args[1] reason = inst.reason.args[1]
except (AttributeError, IndexError): except (AttributeError, IndexError):
# it might be anything, for example a string # it might be anything, for example a string
reason = inst.reason reason = inst.reason
if isinstance(reason, pycompat.unicode): if isinstance(reason, pycompat.unicode):
# SSLError of Python 2.7.9 contains a unicode # SSLError of Python 2.7.9 contains a unicode
reason = encoding.unitolocal(reason) reason = encoding.unitolocal(reason)
ui.warn(_("abort: error: %s\n") % reason) ui.error(_("abort: error: %s\n") % reason)
elif (util.safehasattr(inst, "args") elif (util.safehasattr(inst, "args")
and inst.args and inst.args[0] == errno.EPIPE): and inst.args and inst.args[0] == errno.EPIPE):
pass pass
elif getattr(inst, "strerror", None): elif getattr(inst, "strerror", None):
if getattr(inst, "filename", None): if getattr(inst, "filename", None):
ui.warn(_("abort: %s: %s\n") % ( ui.error(_("abort: %s: %s\n") % (
encoding.strtolocal(inst.strerror), encoding.strtolocal(inst.strerror),
stringutil.forcebytestr(inst.filename))) stringutil.forcebytestr(inst.filename)))
else: else:
ui.warn(_("abort: %s\n") % encoding.strtolocal(inst.strerror)) ui.error(_("abort: %s\n") % encoding.strtolocal(inst.strerror))
else: else:
raise raise
except OSError as inst: except OSError as inst:
if getattr(inst, "filename", None) is not None: if getattr(inst, "filename", None) is not None:
ui.warn(_("abort: %s: '%s'\n") % ( ui.error(_("abort: %s: '%s'\n") % (
encoding.strtolocal(inst.strerror), encoding.strtolocal(inst.strerror),
stringutil.forcebytestr(inst.filename))) stringutil.forcebytestr(inst.filename)))
else: else:
ui.warn(_("abort: %s\n") % encoding.strtolocal(inst.strerror)) ui.error(_("abort: %s\n") % encoding.strtolocal(inst.strerror))
except MemoryError: except MemoryError:
ui.warn(_("abort: out of memory\n")) ui.error(_("abort: out of memory\n"))
except SystemExit as inst: except SystemExit as inst:
# Commands shouldn't sys.exit directly, but give a return code. # Commands shouldn't sys.exit directly, but give a return code.
# Just in case catch this and and pass exit code to caller. # Just in case catch this and and pass exit code to caller.
return inst.code return inst.code
except socket.error as inst: except socket.error as inst:
ui.warn(_("abort: %s\n") % stringutil.forcebytestr(inst.args[-1])) ui.error(_("abort: %s\n") % stringutil.forcebytestr(inst.args[-1]))
return -1 return -1
def checknewlabel(repo, lbl, kind): def checknewlabel(repo, lbl, kind):
# Do not use the "kind" parameter in ui output. # Do not use the "kind" parameter in ui output.
# It makes strings difficult to translate. # It makes strings difficult to translate.
if lbl in ['tip', '.', 'null']: if lbl in ['tip', '.', 'null']:
raise error.Abort(_("the name '%s' is reserved") % lbl) raise error.Abort(_("the name '%s' is reserved") % lbl)

mercurial/ui.py

l = self.fin.readline() l = self.fin.readline()
if not l: if not l:
raise EOFError raise EOFError
return l.rstrip('\n') return l.rstrip('\n')
else: else:
return getpass.getpass('') return getpass.getpass('')
except EOFError: except EOFError:
raise error.ResponseExpected() raise error.ResponseExpected()
def status(self, *msg, **opts): def status(self, *msg, **opts):
'''write status message to output (if ui.quiet is False) '''write status message to output (if ui.quiet is False)
This adds an output label of "ui.status". This adds an output label of "ui.status".
''' '''
if not self.quiet: if not self.quiet:
opts[r'label'] = opts.get(r'label', '') + ' ui.status' opts[r'label'] = opts.get(r'label', '') + ' ui.status'
self.write(*msg, **opts) self.write(*msg, **opts)
def warn(self, *msg, **opts): def warn(self, *msg, **opts):
'''write warning message to output (stderr) '''write warning message to output (stderr)
This adds an output label of "ui.warning". This adds an output label of "ui.warning".
''' '''
opts[r'label'] = opts.get(r'label', '') + ' ui.warning' opts[r'label'] = opts.get(r'label', '') + ' ui.warning'
self.write_err(*msg, **opts) self.write_err(*msg, **opts)
def error(self, *msg, **opts):
'''write error message to output (stderr)
This adds an output label of "ui.error".
'''
opts[r'label'] = opts.get(r'label', '') + ' ui.error'
self.write_err(*msg, **opts)
def note(self, *msg, **opts): def note(self, *msg, **opts):
'''write note to output (if ui.verbose is True) '''write note to output (if ui.verbose is True)
This adds an output label of "ui.note". This adds an output label of "ui.note".
''' '''
if self.verbose: if self.verbose:
opts[r'label'] = opts.get(r'label', '') + ' ui.note' opts[r'label'] = opts.get(r'label', '') + ' ui.note'
self.write(*msg, **opts) self.write(*msg, **opts)
def debug(self, *msg, **opts): def debug(self, *msg, **opts):
'''write debug message to output (if ui.debugflag is True) '''write debug message to output (if ui.debugflag is True)
This adds an output label of "ui.debug". This adds an output label of "ui.debug".
''' '''
if self.debugflag: if self.debugflag:
opts[r'label'] = opts.get(r'label', '') + ' ui.debug' opts[r'label'] = opts.get(r'label', '') + ' ui.debug'
self.write(*msg, **opts) self.write(*msg, **opts)