This is an archive of the discontinued Mercurial Phabricator instance.

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

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

Download Raw Diff

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

Lint Skipped

Unit

Unit Tests Skipped

Event Timeline

rdamazio created this revision.Jul 20 2018, 2:22 AM

Herald added a reviewer: hg-reviewers. · View Herald TranscriptJul 20 2018, 2:22 AM

Herald added a subscriber: mercurial-devel. · View Herald Transcript

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.

PTAL

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–173	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 👍

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.

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

			Path	Packages
M			mercurial/color.py (1 line)
M			mercurial/commandserver.py (2 lines)
M			mercurial/dispatch.py (6 lines)
M			mercurial/scmutil.py (66 lines)
M			mercurial/ui.py (12 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	9638		Jul 20 2018, 2:22 AM	★	★
Diff 2	9641		Jul 20 2018, 4:25 PM	★	★
Diff 3	9694	rHGafc4ad706f9ccb89c2130d3f6de5dd79430541a8	Jul 20 2018, 2:22 AM	★	★

Diff 9641

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))
				martinvonzUnsubmitted Not Done nit: revert formatting change martinvonz: nit: revert formatting change
	if not inst.locker:			if not inst.locker:
	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)