This is an archive of the discontinued Mercurial Phabricator instance.

templater: extract shortest() logic from template function
ClosedPublic

Authored by martinvonz on Sep 15 2017, 2:23 PM.

Download Raw Diff

Details

Reviewers

quark
indygreg
yuja

Group Reviewers

hg-reviewers

Commits

rHG448725a2ef73: templater: extract shortest() logic from template function
rHG9a137d15d146: templater: extract shortest() logic from template function

Summary

It can be useful for extensions to be able to produce the shortest
unambiguous hash (including the in-tree "show" extension). That logic
is currently inside the shortest() template function. Let's move it
out of the templater. I've put it on revlog since it's closely related
to revlog._partialmatch. We may also want a convenience method on
context, but I'll leave that for a later patch.

Diff Detail

Repository

rHG Mercurial

Lint

Lint Skipped

Unit

Unit Tests Skipped

Event Timeline

martinvonz created this revision.Sep 15 2017, 2:23 PM

Herald added a reviewer: hg-reviewers. · View Herald TranscriptSep 15 2017, 2:23 PM

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

It might make sense to be a revlog method if we don't really care about the "cleanness" of revlog class. I slightly prefer that.

martinvonz edited the summary of this revision. (Show Details)Sep 15 2017, 7:37 PM

martinvonz updated this revision to Diff 1860.

Herald added a reviewer: indygreg. · View Herald TranscriptSep 15 2017, 7:37 PM

In D724#12091, @quark wrote:

It might make sense to be a revlog method if we don't really care about the "cleanness" of revlog class. I slightly prefer that.

Heh, that's what I decided to do. I apparently forgot to send it after tests had run, but now it's there. PTAL.

Queued, thanks.

I slightly prefer moving shortest() to scmutil as it seems not a core function
of revlog, but that's just my preference.

This revision is now accepted and ready to land.Sep 16 2017, 7:49 PM

Closed by commit rHG9a137d15d146: templater: extract shortest() logic from template function (authored by martinvonz). · Explain WhySep 16 2017, 7:50 PM

This revision was automatically updated to reflect the committed changes.

Oops. What I originally thought was shortest may have alternative implementation if the changelog is not backed by revlog (in the far far future). So it feels the current implementation (using some revlog internals) belongs to revlog itself.

\What I originally thought was shortest may have alternative implementation if the changelog is not backed by revlog (in the far far future).

Okay, got it. Even if it's backed by revlog, shortest can be implemented as a one-pass
lookup of prefix tree.

Revision Contents
Changeset List

			Path	Packages
M			mercurial/revlog.py (38 lines)
M			mercurial/templater.py (36 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	1859		Sep 15 2017, 2:23 PM	★	★
Diff 2	1860		Sep 15 2017, 7:37 PM	★	★
Diff 3	1862	rHG9a137d15d146d9af9b71a68dfea9f2931c3603d1	Sep 15 2017, 3:01 AM	★	★

Diff 1860

mercurial/revlog.py

	if n is not None:			if n is not None:
	return n			return n
	n = self._partialmatch(id)			n = self._partialmatch(id)
	if n:			if n:
	return n			return n

	raise LookupError(id, self.indexfile, _('no match found'))			raise LookupError(id, self.indexfile, _('no match found'))

				def shortest(self, hexnode, minlength=1):
				"""Find the shortest unambiguous prefix that matches hexnode."""
				def isvalid(test):
				try:
				if self._partialmatch(test) is None:
				return False

				try:
				i = int(test)
				# if we are a pure int, then starting with zero will not be
				# confused as a rev; or, obviously, if the int is larger
				# than the value of the tip rev
				if test[0] == '0' or i > len(self):
				return True
				return False
				except ValueError:
				return True
				except error.RevlogError:
				return False
				except error.WdirUnsupported:
				# single 'ff...' match
				return True

				shortest = hexnode
				startlength = max(6, minlength)
				length = startlength
				while True:
				test = hexnode[:length]
				if isvalid(test):
				shortest = test
				if length == minlength or length > startlength:
				return shortest
				length -= 1
				else:
				length += 1
				if len(shortest) <= length:
				return shortest

	def cmp(self, node, text):			def cmp(self, node, text):
	"""compare text with a given file revision			"""compare text with a given file revision

	returns True if text is different than what is stored.			returns True if text is different than what is stored.
	"""			"""
	p1, p2 = self.parents(node)			p1, p2 = self.parents(node)
	return hash(text, p1, p2) != node			return hash(text, p1, p2) != node

mercurial/templater.py

	minlength = evalinteger(context, mapping, args[1],			minlength = evalinteger(context, mapping, args[1],
	# i18n: "shortest" is a keyword			# i18n: "shortest" is a keyword
	_("shortest() expects an integer minlength"))			_("shortest() expects an integer minlength"))

	# _partialmatch() of filtered changelog could take O(len(repo)) time,			# _partialmatch() of filtered changelog could take O(len(repo)) time,
	# which would be unacceptably slow. so we look for hash collision in			# which would be unacceptably slow. so we look for hash collision in
	# unfiltered space, which means some hashes may be slightly longer.			# unfiltered space, which means some hashes may be slightly longer.
	cl = mapping['ctx']._repo.unfiltered().changelog			cl = mapping['ctx']._repo.unfiltered().changelog
	def isvalid(test):			return cl.shortest(node, minlength)
	try:
	if cl._partialmatch(test) is None:
	return False

	try:
	i = int(test)
	# if we are a pure int, then starting with zero will not be
	# confused as a rev; or, obviously, if the int is larger than
	# the value of the tip rev
	if test[0] == '0' or i > len(cl):
	return True
	return False
	except ValueError:
	return True
	except error.RevlogError:
	return False
	except error.WdirUnsupported:
	# single 'ff...' match
	return True

	shortest = node
	startlength = max(6, minlength)
	length = startlength
	while True:
	test = node[:length]
	if isvalid(test):
	shortest = test
	if length == minlength or length > startlength:
	return shortest
	length -= 1
	else:
	length += 1
	if len(shortest) <= length:
	return shortest

	@templatefunc('strip(text[, chars])')			@templatefunc('strip(text[, chars])')
	def strip(context, mapping, args):			def strip(context, mapping, args):
	"""Strip characters from a string. By default,			"""Strip characters from a string. By default,
	strips all leading and trailing whitespace."""			strips all leading and trailing whitespace."""
	if not (1 <= len(args) <= 2):			if not (1 <= len(args) <= 2):
	# i18n: "strip" is a keyword			# i18n: "strip" is a keyword
	raise error.ParseError(_("strip expects one or two arguments"))			raise error.ParseError(_("strip expects one or two arguments"))