This is an archive of the discontinued Mercurial Phabricator instance.

Can you separate the change from "unresolved conflicts (see hg resolve, then hg rebase --continue)" to "unresolved conflicts (see 'hg resolve', then 'hg rebase --continue')", to make the diff less noisy?

Of course, it feels stupid to change code that’s going to be replaced in the next patch. Still, I think it makes things easier to review (at least that’s my opinion, I don’t know if more experienced reviewers agree).

dploch added a child revision: D8730: error: unify the error message formats for 'rebase' and 'unshelve'.Jul 10 2020, 9:04 PM

dploch removed a child revision: D8714: state: support validated declaration of nested unfinished ops.

dploch updated this revision to Diff 21841.

In D8713#129996, @mjacob wrote:

Can you separate the change from "unresolved conflicts (see hg resolve, then hg rebase --continue)" to "unresolved conflicts (see 'hg resolve', then 'hg rebase --continue')", to make the diff less noisy?

Done. Added a temporary legacy hook which is removed in D8730; all the .t file updates are moved to that revision as well.

Of course, it feels stupid to change code that’s going to be replaced in the next patch. Still, I think it makes things easier to review (at least that’s my opinion, I don’t know if more experienced reviewers agree).

I suppose it's nice in this case because it confirms that it's only the rebase message that's changing (whereas otherwise, you'd need to check every .t file to see this).

mjacob added inline comments.Jul 13 2020, 12:34 AM

mercurial/error.py
117	I think it would have been easier to have the first patch changing the message in `rebase.py` and tests, and the second patch introducing the `ConflictResolutionRequired` class. Generally, patches should not depend on other patches applied later. "Depend" is a bit vague. If the tests fail in between, it’s of course a forbidden dependency. If the tests pass in between, it might be acceptable. Still, in this case, probably both patches should be committed at the same time. If you organize like I proposed above, the text change can be committed independently. Also the total length of both patches would be shorter.

dploch added inline comments.Jul 13 2020, 7:47 PM

mercurial/error.py
117	Done.

martinvonz removed a child revision: D8730: error: unify the error message formats for 'rebase' and 'unshelve'.Jul 13 2020, 11:45 PM

martinvonz edited parent revisions, added: D8730: error: unify the error message formats for 'rebase' and 'unshelve'; removed: D8712: black: format some files in preparation for subsequent changes.

martinvonz added a child revision: D8714: state: support validated declaration of nested unfinished ops.

pulkit added a subscriber: pulkit.Jul 14 2020, 2:50 PM

pulkit added inline comments.

hgext/rebase.py
23	unrelated change
mercurial/error.py
118	Seems like this `if` is not required anymore after D8730.
mercurial/shelve.py
56	unrelated change

dploch updated this revision to Diff 21903.Jul 14 2020, 4:38 PM

dploch added inline comments.Jul 14 2020, 4:42 PM

hgext/rebase.py
23	Removed. Seems like this is a difference between black 19.3b and 19.10b
mercurial/error.py
118	Fixed - I had made the change earlier but due to a bug in Phabricator the upload failed. Martinvonz fixed it so this should be up to date now.
mercurial/shelve.py
56	Removed.

Looks like a step in the right direction.

pulkit accepted this revision.Jul 17 2020, 1:06 PM

This revision is now accepted and ready to land.Jul 17 2020, 1:06 PM

dploch added a commit: rHGe429e7c801b2: error: normalize "unresolved conflicts" error messages with a custom class.Jul 17 2020, 2:04 PM

Closed by commit rHGe429e7c801b2: error: normalize "unresolved conflicts" error messages with a custom class (authored by dploch). · Explain Why

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			hgext/rebase.py (12 lines)
M			mercurial/error.py (27 lines)
M			mercurial/shelve.py (12 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	21810		Jul 8 2020, 6:50 PM	★	★
Diff 2	21841		Jul 10 2020, 9:04 PM	★	★
Diff 3	21903		Jul 14 2020, 4:38 PM	★	★
Diff 4	21957	rHGe429e7c801b2a90e3eb48eee0042f0a4c93b058d	Jul 14 2020, 4:35 PM	★	★

Commit	Parents	Author	Summary	Date
789878bfe1b5	9694895749ad	Daniel Ploch		Jul 9 2020, 6:18 PM

Status	Author	Revision
Closed	dploch	D8714 state: support validated declaration of nested unfinished ops
Closed	dploch	D8713 error: normalize "unresolved conflicts" error messages with a custom class
Closed	dploch	D8730 error: unify the error message formats for 'rebase' and 'unshelve'
Abandoned	dploch	D8712 black: format some files in preparation for subsequent changes

Diff 21841

hgext/rebase.py

	'''			'''

	from __future__ import absolute_import			from __future__ import absolute_import

	import errno			import errno
	import os			import os

	from mercurial.i18n import _			from mercurial.i18n import _
	from mercurial.node import (			from mercurial.node import nullrev, short
				pulkitUnsubmitted Not Done unrelated change pulkit: unrelated change
				dplochAuthorUnsubmitted Done Removed. Seems like this is a difference between black 19.3b and 19.10b dploch: Removed. Seems like this is a difference between black 19.3b and 19.10b
	nullrev,
	short,
	)
	from mercurial.pycompat import open			from mercurial.pycompat import open
	from mercurial import (			from mercurial import (
	bookmarks,			bookmarks,
	cmdutil,			cmdutil,
	commands,			commands,
	copies,			copies,
	destutil,			destutil,
	dirstateguard,			dirstateguard,
	self.collapsef,			self.collapsef,
	dest,			dest,
	wctx=self.wctx,			wctx=self.wctx,
	)			)
	if stats.unresolvedcount > 0:			if stats.unresolvedcount > 0:
	if self.inmemory:			if self.inmemory:
	raise error.InMemoryMergeConflictsError()			raise error.InMemoryMergeConflictsError()
	else:			else:
	raise error.InterventionRequired(			raise error.ConflictResolutionRequired(b'rebase')
	_(
	b'unresolved conflicts (see hg '
	b'resolve, then hg rebase --continue)'
	)
	)
	if not self.collapsef:			if not self.collapsef:
	merging = p2 != nullrev			merging = p2 != nullrev
	editform = cmdutil.mergeeditform(merging, b'rebase')			editform = cmdutil.mergeeditform(merging, b'rebase')
	editor = cmdutil.getcommiteditor(			editor = cmdutil.getcommiteditor(
	editform=editform, **pycompat.strkwargs(opts)			editform=editform, **pycompat.strkwargs(opts)
	)			)
	# We need to set parents again here just in case we're continuing			# We need to set parents again here just in case we're continuing
	# a rebase started with an old hg version (before 9c9cfecd4600),			# a rebase started with an old hg version (before 9c9cfecd4600),

mercurial/error.py



	class InterventionRequired(Hint, Exception):			class InterventionRequired(Hint, Exception):
	"""Exception raised when a command requires human intervention."""			"""Exception raised when a command requires human intervention."""

	__bytes__ = _tobytes			__bytes__ = _tobytes


				class ConflictResolutionRequired(InterventionRequired):
				"""Exception raised when a continuable command required merge conflict resolution."""

				def __init__(self, opname):
				from .i18n import _

				self.opname = opname

				# Temporary hack to minimize test changes: remove in next patch
				mjacobUnsubmitted Not Done I think it would have been easier to have the first patch changing the message in `rebase.py` and tests, and the second patch introducing the `ConflictResolutionRequired` class. Generally, patches should not depend on other patches applied later. "Depend" is a bit vague. If the tests fail in between, it’s of course a forbidden dependency. If the tests pass in between, it might be acceptable. Still, in this case, probably both patches should be committed at the same time. If you organize like I proposed above, the text change can be committed independently. Also the total length of both patches would be shorter. mjacob: I think it would have been easier to have * the first patch changing the message in `rebase.
				dplochAuthorUnsubmitted Done Done. dploch: Done.
				if opname == b'rebase':
				pulkitUnsubmitted Not Done Seems like this `if` is not required anymore after D8730. pulkit: Seems like this `if` is not required anymore after D8730.
				dplochAuthorUnsubmitted Done Fixed - I had made the change earlier but due to a bug in Phabricator the upload failed. Martinvonz fixed it so this should be up to date now. dploch: Fixed - I had made the change earlier but due to a bug in Phabricator the upload failed.
				InterventionRequired.__init__(
				self,
				_(
				b"unresolved conflicts (see hg resolve, then hg %s --continue)"
				)
				% opname,
				)
				else:
				InterventionRequired.__init__(
				self,
				_(
				b"unresolved conflicts (see 'hg resolve', then 'hg %s --continue')"
				)
				% opname,
				)


	class Abort(Hint, Exception):			class Abort(Hint, Exception):
	"""Raised if a command needs to print an error and exit."""			"""Raised if a command needs to print an error and exit."""

	__bytes__ = _tobytes			__bytes__ = _tobytes

	if pycompat.ispy3:			if pycompat.ispy3:

	def __str__(self):			def __str__(self):

mercurial/shelve.py

	phases,			phases,
	pycompat,			pycompat,
	repair,			repair,
	scmutil,			scmutil,
	templatefilters,			templatefilters,
	util,			util,
	vfs as vfsmod,			vfs as vfsmod,
	)			)
	from .utils import (			from .utils import dateutil, stringutil
				pulkitUnsubmitted Not Done unrelated change pulkit: unrelated change
				dplochAuthorUnsubmitted Done Removed. dploch: Removed.
	dateutil,
	stringutil,
	)

	backupdir = b'shelve-backup'			backupdir = b'shelve-backup'
	shelvedir = b'shelved'			shelvedir = b'shelved'
	shelvefileextensions = [b'hg', b'patch', b'shelve']			shelvefileextensions = [b'hg', b'patch', b'shelve']
	# universal extension is present in all types of shelves			# universal extension is present in all types of shelves
	patchextension = b'patch'			patchextension = b'patch'

	# we never need the user, so we use a			# we never need the user, so we use a
	pctx,			pctx,
	tmpwctx,			tmpwctx,
	nodestoremove,			nodestoremove,
	branchtorestore,			branchtorestore,
	opts.get(b'keep'),			opts.get(b'keep'),
	activebookmark,			activebookmark,
	interactive,			interactive,
	)			)
	raise error.InterventionRequired(			raise error.ConflictResolutionRequired(b'unshelve')
	_(
	b"unresolved conflicts (see 'hg resolve', then "
	b"'hg unshelve --continue')"
	)
	)

	with repo.dirstate.parentchange():			with repo.dirstate.parentchange():
	repo.setparents(tmpwctx.node(), nodemod.nullid)			repo.setparents(tmpwctx.node(), nodemod.nullid)
	newnode, ispartialunshelve = _createunshelvectx(			newnode, ispartialunshelve = _createunshelvectx(
	ui, repo, shelvectx, basename, interactive, opts			ui, repo, shelvectx, basename, interactive, opts
	)			)

	if newnode is None:			if newnode is None:

This is an archive of the discontinued Mercurial Phabricator instance.

error: normalize "unresolved conflicts" error messages with a custom classClosedPublic

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 21841

hgext/rebase.py

mercurial/error.py

mercurial/shelve.py

error: normalize "unresolved conflicts" error messages with a custom class
ClosedPublic

Revision Contents
Changeset List