This is an archive of the discontinued Mercurial Phabricator instance.

Differential D6712

config: remove pycompat.bytestr() for defaultvalue
ClosedPublic

Authored by navaneeth.suresh on Aug 3 2019, 2:51 AM.
Tags
None
Subscribers
yuja, pulkit, durin42 and 2 others

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 edited the summary of this revision. (Show Details)Aug 3 2019, 2:55 AM
marmoute added a subscriber: marmoute.Aug 7 2019, 4:20 AM

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.

durin42 resigned from this revision.Aug 7 2019, 8:09 AM
durin42 added a subscriber: durin42.

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.
pulkit added a subscriber: pulkit.Aug 7 2019, 10:19 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?

pulkit added a comment.Aug 7 2019, 10:24 AM

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
navaneeth.suresh added a comment.Aug 7 2019, 11:20 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
yuja added a subscriber: yuja.Aug 7 2019, 7:46 PM

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

PathPackages
Mmercurial/commands.py (2 lines)
Mtests/test-config.t (4 lines)

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