This is an archive of the discontinued Mercurial Phabricator instance.

config: remove pycompat.bytestr() for defaultvalue
ClosedPublic

Authored by navaneeth.suresh on Aug 3 2019, 2:51 AM.

Download Raw Diff

Details

Reviewers

pulkit
durin42

Group Reviewers

hg-reviewers

Commits

rHG049b2ac3252e: config: remove pycompat.bytestr() for defaultvalue

Summary

This is a follow-up patch to 51a2e3102db2. This removes
pycompat.bytestr to preserve None in commands.config().

Diff Detail

Repository

rHG Mercurial

Lint

Lint Skipped

Unit

Unit Tests Skipped

Event Timeline

navaneeth.suresh created this revision.Aug 3 2019, 2:51 AM

Herald added a reviewer: hg-reviewers. · View Herald TranscriptAug 3 2019, 2:51 AM

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

navaneeth.suresh edited the summary of this revision. (Show Details)Aug 3 2019, 2:55 AM

navaneeth.suresh updated this revision to Diff 16144.Aug 6 2019, 1:03 PM

For some reasons, my previous comment seems to have never made it to phabricator:

You description says the changesets does three things. So it should be three different changesets. Can you split them ?

I don't understand why we are adding a warning here. The API usage seems valid.

I agree with marmoute on all points.

navaneeth.suresh edited the summary of this revision. (Show Details)Aug 7 2019, 9:44 AM

navaneeth.suresh retitled this revision from config: fix defaultvalue template keyword to config: fix defaultvalue template keyword (patch 1 of 2).

navaneeth.suresh updated this revision to Diff 16148.

navaneeth.suresh added a child revision: D6720: config: fix fm.data() handling of defaultvalue.Aug 7 2019, 9:51 AM

The pycompat.bytestr() was there for py3 compatibility, however removing it should be fine in this case. Can you make sure test-config.t passes with this patch on Python 3?

This patch needs a better description.

navaneeth.suresh retitled this revision from config: fix defaultvalue template keyword (patch 1 of 2) to config: remove pycompat.bytestr() for defaultvalue.Aug 7 2019, 10:32 AM

In D6712#98424, @pulkit wrote:

The pycompat.bytestr() was there for py3 compatibility, however removing it should be fine in this case. Can you make sure test-config.t passes with this patch on Python 3?

Yes, test-config.t passes with this patch on py3.

pulkit accepted this revision.Aug 7 2019, 5:08 PM

This revision is now accepted and ready to land.Aug 7 2019, 5:08 PM

navaneeth.suresh added a commit: rHG049b2ac3252e: config: remove pycompat.bytestr() for defaultvalue.Aug 7 2019, 5:31 PM

Closed by commit rHG049b2ac3252e: config: remove pycompat.bytestr() for defaultvalue (authored by navaneeth.suresh). · Explain Why

This revision was automatically updated to reflect the committed changes.

The pycompat.bytestr() was there for py3 compatibility,

Not really for the "value" variable which may be unprintable object, but
I think it's okay to assume defaultvalues are None/bool/int/float/bytes type.

Revision Contents
Changeset List

			Path	Packages
M			mercurial/commands.py (4 lines)
M			mercurial/ui.py (5 lines)
M			tests/test-config.t (12 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	16118		Aug 3 2019, 2:51 AM	★	★
Diff 2	16144		Aug 6 2019, 1:03 PM	★	★
Diff 3	16148		Aug 7 2019, 9:44 AM	★	★
Diff 4	16151	rHG049b2ac3252e15a5bbfb5a4bec0ec88e0884603e	Aug 3 2019, 2:44 AM	★	★

Commit	Parents	Author	Summary	Date
		Navaneeth Suresh		Aug 3 2019, 2:44 AM

	Status	Author	Revision
	Closed	navaneeth.suresh	D6720 config: fix fm.data() handling of defaultvalue
	Closed	navaneeth.suresh	D6712 config: remove pycompat.bytestr() for defaultvalue

Diff 16144

mercurial/commands.py


	matched = False			matched = False
	walkconfig = ui.walkconfig(untrusted=untrusted)			walkconfig = ui.walkconfig(untrusted=untrusted)
	if opts.get('registered'):			if opts.get('registered'):
	walkconfig = ui.walkallconfig(untrusted=untrusted)			walkconfig = ui.walkallconfig(untrusted=untrusted)
	for section, name, value in walkconfig:			for section, name, value in walkconfig:
	source = ui.configsource(section, name, untrusted)			source = ui.configsource(section, name, untrusted)
	value = pycompat.bytestr(value)			value = pycompat.bytestr(value)
	defaultvalue = pycompat.bytestr(ui.configdefault(section, name))			defaultvalue = ui.configdefault(section, name)
	if fm.isplain():			if fm.isplain():
	source = source or 'none'			source = source or 'none'
	value = value.replace('\n', '\\n')			value = value.replace('\n', '\\n')
	entryname = section + '.' + name			entryname = section + '.' + name
	if values and not (section in selsections or entryname in selentries):			if values and not (section in selsections or entryname in selentries):
	continue			continue
	fm.startitem()			fm.startitem()
	fm.condwrite(ui.debugflag, 'source', '%s: ', source)			fm.condwrite(ui.debugflag, 'source', '%s: ', source)
	if uniquesel:
	fm.data(name=entryname, defaultvalue=defaultvalue)			fm.data(name=entryname, defaultvalue=defaultvalue)
				if uniquesel:
	fm.write('value', '%s\n', value)			fm.write('value', '%s\n', value)
	else:			else:
	fm.write('name value', '%s=%s\n', entryname, value)			fm.write('name value', '%s=%s\n', entryname, value)
	matched = True			matched = True
	fm.end()			fm.end()
	if matched:			if matched:
	return 0			return 0
	return 1			return 1

mercurial/ui.py

	"""returns the default value of the config item"""			"""returns the default value of the config item"""
	item = self._knownconfig.get(section, {}).get(name)			item = self._knownconfig.get(section, {}).get(name)
	itemdefault = None			itemdefault = None
	if item is not None:			if item is not None:
	if callable(item.default):			if callable(item.default):
	itemdefault = item.default()			itemdefault = item.default()
	else:			else:
	itemdefault = item.default			itemdefault = item.default
				if item.default is configitems.dynamicdefault:
				self.warn(_("config item requires an explicit default value: "
				"'%s.%s'\n" % (section, name)))
	return itemdefault			return itemdefault

	def hasconfig(self, section, name, untrusted=False):			def hasconfig(self, section, name, untrusted=False):
	return self._data(untrusted).hasitem(section, name)			return self._data(untrusted).hasitem(section, name)

	def has_section(self, section, untrusted=False):			def has_section(self, section, untrusted=False):
	'''tell whether section exists in config.'''			'''tell whether section exists in config.'''
	return section in self._data(untrusted)			return section in self._data(untrusted)

tests/test-config.t


	$ hg showconfig Section			$ hg showconfig Section
	Section.KeY=Case Sensitive			Section.KeY=Case Sensitive
	Section.key=lower case			Section.key=lower case

	$ hg showconfig Section -Tjson			$ hg showconfig Section -Tjson
	[			[
	{			{
				"defaultvalue": null,
	"name": "Section.KeY",			"name": "Section.KeY",
	"source": ".hgrc:", (glob)			"source": ".hgrc:", (glob)
	"value": "Case Sensitive"			"value": "Case Sensitive"
	},			},
	{			{
				"defaultvalue": null,
	"name": "Section.key",			"name": "Section.key",
	"source": ".hgrc:", (glob)			"source": ".hgrc:", (glob)
	"value": "lower case"			"value": "lower case"
	}			}
	]			]
	$ hg showconfig Section.KeY -Tjson			$ hg showconfig Section.KeY -Tjson
	[			[
	{			{
	"defaultvalue": "None",			"defaultvalue": null,
	"name": "Section.KeY",			"name": "Section.KeY",
	"source": ".hgrc:", (glob)			"source": ".hgrc:", (glob)
	"value": "Case Sensitive"			"value": "Case Sensitive"
	}			}
	]			]
	$ hg showconfig -Tjson \| tail -7			$ hg showconfig -Tjson \| tail -7
	},
	{			{
				"defaultvalue": null,
	"name": "*", (glob)			"name": "*", (glob)
	"source": "*", (glob)			"source": "*", (glob)
	"value": "*" (glob)			"value": "*" (glob)
	}			}
	]			]

	Test empty config source:			Test empty config source:

	$ cat <<EOF > emptysource.py			$ cat <<EOF > emptysource.py
	> def reposetup(ui, repo):			> def reposetup(ui, repo):
	> ui.setconfig(b'empty', b'source', b'value')			> ui.setconfig(b'empty', b'source', b'value')
	> EOF			> EOF
	$ cp .hg/hgrc .hg/hgrc.orig			$ cp .hg/hgrc .hg/hgrc.orig
	$ cat <<EOF >> .hg/hgrc			$ cat <<EOF >> .hg/hgrc
	> [extensions]			> [extensions]
	> emptysource = `pwd`/emptysource.py			> emptysource = `pwd`/emptysource.py
	> EOF			> EOF

	$ hg config --debug empty.source			$ hg config --debug empty.source
	read config from: * (glob)			read config from: * (glob)
	none: value			none: value
	$ hg config empty.source -Tjson			$ hg config empty.source -Tjson
	[			[
	{			{
	"defaultvalue": "None",			"defaultvalue": null,
	"name": "empty.source",			"name": "empty.source",
	"source": "",			"source": "",
	"value": "value"			"value": "value"
	}			}
	]			]

	$ cp .hg/hgrc.orig .hg/hgrc			$ cp .hg/hgrc.orig .hg/hgrc


	$ HGEDITOR=false hg config --edit			$ HGEDITOR=false hg config --edit
	abort: edit failed: false exited with status 1			abort: edit failed: false exited with status 1
	[255]			[255]

	config affected by environment variables			config affected by environment variables

	$ EDITOR=e1 VISUAL=e2 hg config --debug \| grep 'ui\.editor'			$ EDITOR=e1 VISUAL=e2 hg config --debug \| grep 'ui\.editor'
				config item requires an explicit default value: 'ui.editor'
	$VISUAL: ui.editor=e2			$VISUAL: ui.editor=e2

	$ VISUAL=e2 hg config --debug --config ui.editor=e3 \| grep 'ui\.editor'			$ VISUAL=e2 hg config --debug --config ui.editor=e3 \| grep 'ui\.editor'
				config item requires an explicit default value: 'ui.editor'
	--config: ui.editor=e3			--config: ui.editor=e3

	$ PAGER=p1 hg config --debug \| grep 'pager\.pager'			$ PAGER=p1 hg config --debug \| grep 'pager\.pager'
				config item requires an explicit default value: 'pager.pager'
	$PAGER: pager.pager=p1			$PAGER: pager.pager=p1

	$ PAGER=p1 hg config --debug --config pager.pager=p2 \| grep 'pager\.pager'			$ PAGER=p1 hg config --debug --config pager.pager=p2 \| grep 'pager\.pager'
				config item requires an explicit default value: 'pager.pager'
	--config: pager.pager=p2			--config: pager.pager=p2

	verify that aliases are evaluated as well			verify that aliases are evaluated as well

	$ hg init aliastest			$ hg init aliastest
	$ cd aliastest			$ cd aliastest
	$ cat > .hg/hgrc << EOF			$ cat > .hg/hgrc << EOF
	> [ui]			> [ui]