This is an archive of the discontinued Mercurial Phabricator instance.

Differential D12019

simplemerge: take over formatting of label from `filemerge`
ClosedPublic

Authored by martinvonz on Jan 21 2022, 6:31 PM.

Download Raw Diff

Details

Reviewers

Alphare

Group Reviewers

hg-reviewers

Commits

rHG3c8cc987672e: simplemerge: take over formatting of label from `filemerge`

Summary

The padding we do of conflict labels depends on which conflict marker
style is used. For two-way conflict markers (the default), the length
of the base label shouldn't matter. It does before this patch,
however. This patch moves the formatting from filemerge to
simplemerge. The latter knows which conflict marker style to use, so
it can easily decide about the padding.

This change will allow us to use more descriptive "base" labels
without causing illogical padding in 2-way markers. I'll do that next.

One wrinkle is that we pass the same labels to external merge tools. I
decided to change that in this patch to be simpler: no padding, and no
ellipsis to fit within 80 columns. My reasoning is that the typical
external, 3-or-4-panel merge tool doesn't show the labels on top of
each others, so the padding doesn't make sense there. The ellipsis is
probably not necessary because the external tools probably have their
own way of dealing with long labels. Also, we limit them to "80 - 8"
to fit the "<<<<<<< " before, which is almost definitely not what an
external tool would put there.

Diff Detail

Repository

rHG Mercurial

Branch

default

Lint

No Linters Available

Unit

No Unit Test Coverage

Event Timeline

martinvonz created this revision.Jan 21 2022, 6:31 PM

Herald added a reviewer: hg-reviewers. · View Herald TranscriptJan 21 2022, 6:31 PM

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

martinvonz added a child revision: D12020: rebase: set custom conflict label for base commit.Jan 21 2022, 6:31 PM

I'm not requesting changes in this patch per se, but it seems like the right place to ask for another patch to update the relnotes for backwards compat reasons. I don't think we're going to break much, but it's good to be explicit and will save me some time in the 6.1 release work.

This revision is now accepted and ready to land.Jan 24 2022, 5:29 AM

In D12019#184276, @Alphare wrote:

I'm not requesting changes in this patch per se, but it seems like the right place to ask for another patch to update the relnotes for backwards compat reasons. I don't think we're going to break much, but it's good to be explicit and will save me some time in the 6.1 release work.

Ah, of course! I've added a note in this patch about the changed formatting. I've also added a separate patch with a note about changed label texts.

martinvonz updated this revision to Diff 31821.Jan 24 2022, 11:41 AM

Alphare accepted this revision.Jan 24 2022, 11:49 AM

martinvonz added a commit: rHG3c8cc987672e: simplemerge: take over formatting of label from `filemerge`.Jan 24 2022, 12:30 PM

Closed by commit rHG3c8cc987672e: simplemerge: take over formatting of label from `filemerge` (authored by martinvonz). · Explain Why

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			mercurial/filemerge.py (51 lines)
M			mercurial/simplemerge.py (16 lines)
M			tests/test-merge-tools.t (4 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	31719		Jan 21 2022, 6:31 PM	★	★
Diff 2	31821		Jan 24 2022, 11:41 AM	★	★
Diff 3	31832	rHG3c8cc987672e05abe7b7b4a69e9da69e18b7cac3	Jan 20 2022, 2:06 PM	★	★

Commit	Parents	Author	Summary	Date
1a0069e1010d	4312f91ddfdb	Martin von Zweigbergk		Jan 20 2022, 2:06 PM

Status	Author	Revision
Closed	martinvonz	D12085 relnotes: add note about changed labels texts
Closed	martinvonz	D12025 graft: attempt to make merge labels more helpful
Closed	martinvonz	D12024 shelve: attempt to make merge labels more helpful
Closed	martinvonz	D12023 histedit: attempt to make merge labels more helpful
Closed	martinvonz	D12022 update: set custom conflict label for base commit
Closed	martinvonz	D12021 merge: set custom conflict label for base commit
Closed	martinvonz	D12020 rebase: set custom conflict label for base commit
Closed	martinvonz	D12019 simplemerge: take over formatting of label from `filemerge`
Closed	martinvonz	D12018 filemerge: pass `simplemerge.MergeInput` to tool functions
Closed	martinvonz	D12017 filemerge: work with `simplemerge.MergeInput` in `filemerge()`
Closed	martinvonz	D12016 filemerge: always define a "base" label
Closed	martinvonz	D12015 filemerge: set default labels a little earlier
Closed	martinvonz	D12014 filemerge: remove unused arguments from `_merge()`
Closed	martinvonz	D12013 simplemerge: take arguments as annotated context objects
Closed	martinvonz	D12012 filemerge: make `labels` a required arg for `_premerge()`
Closed	martinvonz	D12011 filemerge: remove unnecessary check for empty string
Closed	martinvonz	D12010 tests: also test spaces in conflict marker labels

Diff 31719

mercurial/filemerge.py

	templatekw,			templatekw,
	templater,			templater,
	templateutil,			templateutil,
	util,			util,
	)			)

	from .utils import (			from .utils import (
	procutil,			procutil,
	stringutil,
	)			)


	def _toolstr(ui, tool, part, *args):			def _toolstr(ui, tool, part, *args):
	return ui.config(b"merge-tools", tool + b"." + part, *args)			return ui.config(b"merge-tools", tool + b"." + part, *args)


	def _toolbool(ui, tool, part, *args):			def _toolbool(ui, tool, part, *args):
	localpath = _workingpath(repo, fcd)			localpath = _workingpath(repo, fcd)
	args = _toolstr(repo.ui, tool, b"args")			args = _toolstr(repo.ui, tool, b"args")

	with _maketempfiles(			with _maketempfiles(
	repo, fco, fca, repo.wvfs.join(backup.path()), b"$output" in args			repo, fco, fca, repo.wvfs.join(backup.path()), b"$output" in args
	) as temppaths:			) as temppaths:
	basepath, otherpath, localoutputpath = temppaths			basepath, otherpath, localoutputpath = temppaths
	outpath = b""			outpath = b""

				def format_label(input):
				if input.label_detail:
				return b'%s: %s' % (input.label, input.label_detail)
				else:
				return input.label

	env = {			env = {
	b'HG_FILE': fcd.path(),			b'HG_FILE': fcd.path(),
	b'HG_MY_NODE': short(mynode),			b'HG_MY_NODE': short(mynode),
	b'HG_OTHER_NODE': short(fco.changectx().node()),			b'HG_OTHER_NODE': short(fco.changectx().node()),
	b'HG_BASE_NODE': short(fca.changectx().node()),			b'HG_BASE_NODE': short(fca.changectx().node()),
	b'HG_MY_ISLINK': b'l' in fcd.flags(),			b'HG_MY_ISLINK': b'l' in fcd.flags(),
	b'HG_OTHER_ISLINK': b'l' in fco.flags(),			b'HG_OTHER_ISLINK': b'l' in fco.flags(),
	b'HG_BASE_ISLINK': b'l' in fca.flags(),			b'HG_BASE_ISLINK': b'l' in fca.flags(),
	b'HG_MY_LABEL': local.label,			b'HG_MY_LABEL': format_label(local),
	b'HG_OTHER_LABEL': other.label,			b'HG_OTHER_LABEL': format_label(other),
	b'HG_BASE_LABEL': base.label,			b'HG_BASE_LABEL': format_label(base),
	}			}
	ui = repo.ui			ui = repo.ui

	if b"$output" in args:			if b"$output" in args:
	# read input from backup, write to original			# read input from backup, write to original
	outpath = localpath			outpath = localpath
	localpath = localoutputpath			localpath = localoutputpath
	replace = {			replace = {
	b'local': localpath,			b'local': localpath,
	b'base': basepath,			b'base': basepath,
	b'other': otherpath,			b'other': otherpath,
	b'output': outpath,			b'output': outpath,
	b'labellocal': local.label,			b'labellocal': format_label(local),
	b'labelother': other.label,			b'labelother': format_label(other),
	b'labelbase': base.label,			b'labelbase': format_label(base),
	}			}
	args = util.interpolate(			args = util.interpolate(
	br'\$',			br'\$',
	replace,			replace,
	args,			args,
	lambda s: procutil.shellquote(util.localpath(s)),			lambda s: procutil.shellquote(util.localpath(s)),
	)			)
	if _toolbool(ui, tool, b"gui"):			if _toolbool(ui, tool, b"gui"):
	ui, repo, b"merge", toolpath, mergefn, {b'args': argslist}, True			ui, repo, b"merge", toolpath, mergefn, {b'args': argslist}, True
	)			)
	if raised:			if raised:
	r = 1			r = 1
	repo.ui.debug(b'merge tool returned: %d\n' % r)			repo.ui.debug(b'merge tool returned: %d\n' % r)
	return True, r, False			return True, r, False


	def _populate_label_detail(input, template, pad):			def _populate_label_detail(input, template):
	"""Applies the given template to the ctx, prefixed by the label.			"""Applies the given template to the ctx and stores it in the input."""

	Pad is the minimum width of the label prefix, so that multiple markers
	can have aligned templated parts.
	"""
	ctx = input.fctx.changectx()			ctx = input.fctx.changectx()
	if ctx.node() is None:			if ctx.node() is None:
	ctx = ctx.p1()			ctx = ctx.p1()

	props = {b'ctx': ctx}			props = {b'ctx': ctx}
	templateresult = template.renderdefault(props)			templateresult = template.renderdefault(props)
				input.label_detail = templateresult.splitlines()[0] # split for safety
	label = (b'%s:' % input.label).ljust(pad + 1)
	mark = b'%s %s' % (label, templateresult)
	mark = mark.splitlines()[0] # split for safety

	# 8 for the prefix of conflict marker lines (e.g. '<<<<<<< ')
	input.label = stringutil.ellipsis(mark, 80 - 8)


	def _populate_label_details(repo, inputs, tool=None):			def _populate_label_details(repo, inputs, tool=None):
	"""Formats the given labels using the conflict marker template.			"""Populates the label details using the conflict marker template."""

	Returns a list of formatted labels.
	"""
	ui = repo.ui			ui = repo.ui
	template = ui.config(b'command-templates', b'mergemarker')			template = ui.config(b'command-templates', b'mergemarker')
	if tool is not None:			if tool is not None:
	template = _toolstr(ui, tool, b'mergemarkertemplate', template)			template = _toolstr(ui, tool, b'mergemarkertemplate', template)
	template = templater.unquotestring(template)			template = templater.unquotestring(template)
	tres = formatter.templateresources(ui, repo)			tres = formatter.templateresources(ui, repo)
	tmpl = formatter.maketemplater(			tmpl = formatter.maketemplater(
	ui, template, defaults=templatekw.keywords, resources=tres			ui, template, defaults=templatekw.keywords, resources=tres
	)			)

	pad = max(len(input.label) for input in inputs)

	for input in inputs:			for input in inputs:
	_populate_label_detail(input, tmpl, pad)			_populate_label_detail(input, tmpl)


	def partextras(labels):			def partextras(labels):
	"""Return a dictionary of extra labels for use in prompts to the user			"""Return a dictionary of extra labels for use in prompts to the user

	Intended use is in strings of the form "(l)ocal%(l)s".			Intended use is in strings of the form "(l)ocal%(l)s".
	"""			"""
	if labels is None:			if labels is None:
	toolconf,			toolconf,
	backup,			backup,
	)			)
	# we're done if premerge was successful (r is 0)			# we're done if premerge was successful (r is 0)
	if not r:			if not r:
	return r, False			return r, False

	# Reset to basic labels			# Reset to basic labels
	local.label = labels[0]			local.label_detail = None
	other.label = labels[1]			other.label_detail = None
	base.label = labels[2]			base.label_detail = None

	if markerstyle != b'basic':			if markerstyle != b'basic':
	_populate_label_details(repo, [local, other, base], tool=tool)			_populate_label_details(repo, [local, other, base], tool=tool)

	needcheck, r, deleted = func(			needcheck, r, deleted = func(
	repo,			repo,
	mynode,			mynode,
	local,			local,

mercurial/simplemerge.py

	if not opts.get('quiet'):			if not opts.get('quiet'):
	ui.warn(_(b'warning: %s\n') % msg)			ui.warn(_(b'warning: %s\n') % msg)
	if not opts.get('text'):			if not opts.get('text'):
	raise error.Abort(msg)			raise error.Abort(msg)
	return text			return text


	def _format_labels(*inputs):			def _format_labels(*inputs):
				pad = max(len(input.label) if input.label else 0 for input in inputs)
	labels = []			labels = []
	for input in inputs:			for input in inputs:
	if input.label:			if input.label:
	labels.append(input.label)			if input.label_detail:
				label = (
				(input.label + b':').ljust(pad + 1)
				+ b' '
				+ input.label_detail
				)
				else:
				label = input.label
				# 8 for the prefix of conflict marker lines (e.g. '<<<<<<< ')
				labels.append(stringutil.ellipsis(label, 80 - 8))
	else:			else:
	labels.append(None)			labels.append(None)
	return labels			return labels


	def _detect_newline(m3):			def _detect_newline(m3):
	if len(m3.a) > 0:			if len(m3.a) > 0:
	if m3.a[0].endswith(b'\r\n'):			if m3.a[0].endswith(b'\r\n'):
	lines.extend(group_lines)			lines.extend(group_lines)
	return lines			return lines


	@attr.s			@attr.s
	class MergeInput(object):			class MergeInput(object):
	fctx = attr.ib()			fctx = attr.ib()
	label = attr.ib(default=None)			label = attr.ib(default=None)
				# If the "detail" part is set, then that is rendered after the label and
				# separated by a ':'. The label is padded to make the ':' aligned among all
				# merge inputs.
				label_detail = attr.ib(default=None)


	def simplemerge(ui, local, base, other, **opts):			def simplemerge(ui, local, base, other, **opts):
	"""Performs the simplemerge algorithm.			"""Performs the simplemerge algorithm.

	The merged result is written into `localctx`.			The merged result is written into `localctx`.
	"""			"""

tests/test-merge-tools.t

	> --config merge-tools.true.mergemarkers=detailed \			> --config merge-tools.true.mergemarkers=detailed \
	> --config merge-tools.true.mergemarkertemplate='tooltmpl {short(node)}' \			> --config merge-tools.true.mergemarkertemplate='tooltmpl {short(node)}' \
	> --config ui.mergemarkertemplate='uitmpl {rev}' \			> --config ui.mergemarkertemplate='uitmpl {rev}' \
	> --config ui.mergemarkers=basic \			> --config ui.mergemarkers=basic \
	> merge -r 2			> merge -r 2
	merging f			merging f
	arg: "ll:working copy: tooltmpl ef83787e2614"			arg: "ll:working copy: tooltmpl ef83787e2614"
	arg: "lo:"			arg: "lo:"
	arg: "merge rev: tooltmpl 0185f4e0cf02"			arg: "merge rev: tooltmpl 0185f4e0cf02"
	arg: "lb:base: /f~base." (glob)			arg: "lb:base: /f~base." (glob)
	0 files updated, 1 files merged, 0 files removed, 0 files unresolved			0 files updated, 1 files merged, 0 files removed, 0 files unresolved
	(branch merge, don't forget to commit)			(branch merge, don't forget to commit)
	$ rm -f 'printargs_merge_tool'			$ rm -f 'printargs_merge_tool'

	The merge tool still gets labellocal and labelother as 'basic' even when			The merge tool still gets labellocal and labelother as 'basic' even when
	premerge=keep is used and has 'detailed' markers:			premerge=keep is used and has 'detailed' markers:

	> --config merge-tools.true.premerge=keep \			> --config merge-tools.true.premerge=keep \
	> --config merge-tools.true.mergemarkers=detailed \			> --config merge-tools.true.mergemarkers=detailed \
	> --config merge-tools.true.mergemarkertemplate='tooltmpl {short(node)}' \			> --config merge-tools.true.mergemarkertemplate='tooltmpl {short(node)}' \
	> --config ui.mergemarkertemplate='uitmpl {rev}' \			> --config ui.mergemarkertemplate='uitmpl {rev}' \
	> --config ui.mergemarkers=detailed \			> --config ui.mergemarkers=detailed \
	> merge -r 2			> merge -r 2
	merging f			merging f
	labellocal: "working copy: tooltmpl ef83787e2614"			labellocal: "working copy: tooltmpl ef83787e2614"
	labelother: "merge rev: tooltmpl 0185f4e0cf02"			labelother: "merge rev: tooltmpl 0185f4e0cf02"
	output (arg): "$TESTTMP/repo/f"			output (arg): "$TESTTMP/repo/f"
	output (contents):			output (contents):
	<<<<<<< working copy: tooltmpl ef83787e2614			<<<<<<< working copy: tooltmpl ef83787e2614
	revision 1			revision 1
	=======			=======
	revision 2			revision 2
	>>>>>>> merge rev: tooltmpl 0185f4e0cf02			>>>>>>> merge rev: tooltmpl 0185f4e0cf02
	space			space