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

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

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 (2 lines)
M			tests/test-config.t (4 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	★	★

	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 16151

mercurial/commands.py

	uniquesel = (len(selentries) == 1 and not selsections)			uniquesel = (len(selentries) == 1 and not selsections)
	selsections = set(selsections)			selsections = set(selsections)
	selentries = set(selentries)			selentries = set(selentries)

	matched = False			matched = False
	for section, name, value in ui.walkconfig(untrusted=untrusted):			for section, name, value in ui.walkconfig(untrusted=untrusted):
	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)

tests/test-config.t

	"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
	},			},
	{			{
	> 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